[OPENJPA-2987] Restore externalized-parameter assertions - #152
Open
rzo1 wants to merge 1 commit into
Open
Conversation
The three "CanDetectExternalized..." tests had been reduced to assertNotNull(getResultList()), which can never fail, after JPA 3.2 made getResultList() return a mutable ArrayList copy instead of the internal ResultList the assertions relied on. Externalized-parameter detection is still in place (PreparedQueryImpl excludes such queries from the prepared query cache), so the original assertions are restored by executing the kernel query directly, which still yields the ResultList carrying the QueryExpressions user object.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up on #144 (comment) (OPENJPA-2987).
Answer to the review question
Externalized-parameter detection was not removed.
PreparedQueryImpl.extractSelectExecutor()still callsisUsingExternalizedParameter(...)and excludes such queries from the prepared query cache. With trace logging enabled:What broke was only the test's access to the expressions: JPA 3.2 requires a mutable
getResultList(), soorg.apache.openjpa.persistence.QueryImpl.getResultList()now returnsnew ArrayList<>(delegate)and the(ResultList) resultcast in the oldgetExpressions()helper no longer works. The cache itself is unaffected, sincepostExecute()still receives the rawResultList.Change
TestExternalizedParametergets its original assertions back:getExpressions(...)helper restoredassertFalse(...)/assertTrue(isUsingExternalizedParameter(exps[0]))restored in place of the no-opassertNotNull(getResultList())ResultListis obtained by executing the kernel query directly (((QueryImpl<?>) em.createQuery(jpql)).getDelegate().execute(params)) instead of through the JPA facadeTest
mvn -pl openjpa-persistence-jdbc -Dtest=TestExternalizedParameter test-> Tests run: 3, Failures: 0, Errors: 0.Note
While looking for a cache-level assertion instead:
PreparedQueryCacheImpl.isExcluded(id)only matches user-supplied exclusion patterns. Per-query exclusions are stored in_uncachablesand are only observable viaisCachable(id) == FALSE, with no accessor for the exclusion reason. Since everyBookquery is uncachable anyway (eager@ManyToMany-> multiple SQL statements), that route cannot distinguish an externalized-parameter exclusion from any other, so the kernel-level assertion is the one that actually discriminates.