[
https://issues.apache.org/jira/browse/OPENJPA-2980?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126139#comment-18126139
]
ASF subversion and git services commented on OPENJPA-2980:
----------------------------------------------------------
Commit 00064aa235e2702e6dc57c5612bc55d2c4e084da in openjpa's branch
refs/heads/tck-mysql from Richard Zowalla
[ https://gitbox.apache.org/repos/asf?p=openjpa.git;h=00064aa23 ]
[OPENJPA-2980] Replay a criteria query registered by addNamedQuery as a
criteria query
addNamedQuery() re-labelled a criteria query as JPQL and stored the CQL
rendering of the CriteriaQuery as the query string. That rendering is not
guaranteed to be parseable JPQL: string literals are not escaped and
subqueries are not parenthesized, so createNamedQuery() either failed to
parse the query or recreated a different one.
Keep the compiled criteria form in the QueryMetaData instead and replay it
in createNamedQuery(), so a criteria query is recreated as a criteria query.
For that to survive, ExpressionStoreQuery now retains the parsed form handed
to setQuery() and reuses it when the query has no string to parse again,
e.g. after setResultClass() invalidated the compilation.
The query is compiled before its metadata is captured: the candidate and the
result type of a query are only known once it has been compiled, and a criteria
query has no query string they could be recovered from afterwards, so capturing
them from an uncompiled query left both of them null and the registered query
was invisible to getNamedQueries(Class) - unlike a JPQL one, which the facade
compiles before addNamedQuery() sees it. The identifier of the query, the CQL
rendering of its tree, is rendered there as well, while the registering thread
is still the only one holding the tree; createNamedQuery() reuses it instead of
rendering the shared tree on every replay.
A named query is an EntityManagerFactory-wide artefact, so the retained
criteria tree is now replayed by every EntityManager. Translating a criteria
tree to a kernel expression tree mutates the tree - the alias, variable and
value maps are re-populated and a subquery records the kernel subquery it was
translated to - so concurrent replays corrupted each other, yielding wrong
results, malformed SQL or a ConcurrentModificationException. The tree itself
is not a usable monitor for that: a snapshot deliberately shares those
mutable maps with the original query and with every other snapshot, and the
captive query of a subquery shares the parameter map of the query it belongs
to, so locking on the object being translated still lets two snapshots of one
tree race. Each tree therefore carries a TranslationLock, created by the root
query and handed to every snapshot of it and to every subquery delegate, and
the translation is serialized on that lock. A set operation translates two
independent trees and takes both their locks, in a fixed order so that two set
operations over the same operands cannot deadlock.
The replayed query is identified by the CQL rendering of its tree, which is
not a query string it can be parsed from, so the prepared query cache must not
hold it: it only excluded LANG_SQL and LANG_METHODQL, and a criteria query
that had been cached under that rendering was rebuilt by parsing the rendering
as JPQL as soon as something made a later execution abandon the cached entry,
e.g. setting a lock mode. Name the criteria language in QueryLanguages and
exclude it from the cache.
Excluding it from the cache only covers the write side. The rendering of a
simple criteria query is valid JPQL, so an unrelated JPQL query of exactly that
text can be cached under the identifier of the criteria query, and the facade
looked the cache up by that identifier alone: a replay that abandoned the cached
entry - with a lock mode, or with a null parameter value - then rebuilt the
criteria query by parsing its own identifier as JPQL, which silently discarded
the criteria tree or failed outright with "declared parameters were not given
values". Rebuilding from the identifier now happens for JPQL queries only.
Also narrow the exception handling in addNamedQuery(): getFlushMode() and
getMaxResults() do not fail for any query a facade query can wrap, and only
getLockMode() is specified to throw IllegalStateException for non-SELECT or
native queries, so genuine failures are no longer swallowed.
Update the 4.2.0 migration notes, which still described the removed
"stored as JPQL text" behaviour.
> addNamedQuery relabels criteria query as JPQL
> ---------------------------------------------
>
> Key: OPENJPA-2980
> URL: https://issues.apache.org/jira/browse/OPENJPA-2980
> Project: OpenJPA
> Issue Type: Sub-task
> Components: jpa
> Affects Versions: 4.2.0
> Reporter: Maxim Solodovnik
> Assignee: Richard Zowalla
> Priority: Major
> Fix For: 4.2.0
>
>
> Discussion thread:
> https://github.com/apache/openjpa/pull/144#discussion_r3683005009
> **(medium)** `addNamedQuery` re-labels a criteria query as JPQL using
> `queryImpl.getQueryString()`, which for criteria queries is the CQL/toString
> rendering - OpenJPA has never guaranteed that string is parseable JPQL
> (parameter rendering, literals, treated paths). Has this been exercised
> beyond simple queries? The three `catch (Exception) { // ignore }` blocks
> below (lines 512-540) would also hide genuine failures - they could be
> narrowed to IllegalStateException where the spec defines it.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
