Repository navigation
[OPENJPA-2950] Flush before a query for the access path, not for the whole context - #215
Merged
Merged
Conversation
solomax
reviewed
Oct 13, 2026
solomax
reviewed
Oct 13, 2026
solomax
approved these changes
Oct 13, 2026
Since OPENJPA-2940 a query with FlushModeType.AUTO flushed as soon as anything in the persistence context was dirty, and overrode openjpa.IgnoreChanges whenever the flush mode was AUTO - which is the default. Both are reverted, and the gap the broadening was meant to close is fixed where it actually is. Why the access path is enough. Section 3.10.8 of the specification scopes the duty to the updates "which could potentially affect the result of the query", and lets the provider discharge it "by flushing those entities to the database or by some other means". A full-context flush is therefore permitted but not required, and the access path check is exactly such other means: only a dirty instance of a type the query reads can change its result. That holds, however, only for as long as the access path is complete, and in two places it was not. JPQL. AbstractExpressionBuilder.traversePath() registered the metadata of every hop, so Order, Customer and Spouse are all in the access path of "SELECT o FROM Order o WHERE o.customer.spouse.lastName = 'Miller'" - the case the comment added with the broadening claimed was missed. What it did miss is a collection- or map-valued path used without a join variable: the declared type of such a field is Collection or Map, which has no metadata, so SIZE(x.coll) and x.coll IS EMPTY left the element and key types out and a dirty element was not flushed. traversePath() now registers them, which also makes the JPQL and the Criteria side agree - the latter registers them for the same field. Criteria. CriteriaExpressionBuilder.evalAccessPaths() looked only at the roots and their immediate joins, so a type reached by an implicit join (a path navigated with Path.get), by a join of a join, by a fetch join, inside a subquery or behind a join made on a correlated join was left out - the subquery case was a standing "TODO -- need to handle subqueries". That is now fixed at the source: PathImpl registers each field it traverses on the query, SubqueryImpl hands its access path to the enclosing query, the join walk follows the whole join tree and starts at the correlated joins of the enclosing subquery as well, and CriteriaExpressionBuilder merges all of it into QueryExpressions.accessPath after the expressions have been converted. IgnoreChanges. openjpa.IgnoreChanges=true is a documented option whose purpose is to let a query run straight against the datastore, recommended in ref_guide_optimization.xml together with the default openjpa.FlushBeforeQueries=true. Overriding it whenever the flush mode is FLUSH_TRUE therefore silently removed it for every default configuration. It is restored unconditionally, as in 4.1.x, and no distinction is drawn between a flush mode set on the query and one inherited from the persistence context: section 3.10.8 states the same duty for both triggers, so there is no basis for honouring the option for one and not the other. The option is an OpenJPA extension that opts a SELECT out of that duty, which ref_guide_conf.xml now says where users enable it. Making the override conditional on the flush mode would also have required state the facade does not have. Writing through to the kernel's ignoreChanges instead, as this was first implemented, is not sound: the write is one-way, so setFlushMode(AUTO) followed by setFlushMode(COMMIT) on the same query left ignoreChanges false with flush FLUSH_FALSE, and QueryImpl.isInMemory() then entered the branch, could not flush and executed the query in memory over the whole extent - a different result set and an unbounded scan for the same query text and flush mode. It also fired for queries the application never set a flush mode on, because EntityManagerFactoryImpl.addNamedQuery() captures a flush mode that is never unset and createNamedQuery() replays it as setFlushMode(AUTO). The prohibitions of section 3.10.8 hold. _broker.isActive() already covers "if there is no transaction active, the persistence provider must not flush"; an unjoined context cannot reach the flush at all, because EntityManagerFactoryImpl rejects SynchronizationType.UNSYNCHRONIZED. No compatibility option is added: the broadened behaviour only ever existed on the unreleased 4.2.0 line, so there is nothing to be compatible with. The migration note is rewritten to describe what actually changes for an application upgrading from 4.1.x. Tests, asserting on emitted SQL rather than on results, because a full-context flush returns the same results: - TestQueryFlushAccessPath: a dirty instance of a type outside the access path must not flush; the candidate type, a dirty subclass of it, a type reached by JPQL navigation, a collection path in SIZE() and in IS EMPTY, and - for the Criteria API - an implicit join, a join of a join, a fetch join, a subquery and a join made on a correlated join must all be visible; COMMIT must not flush; no flush without an active transaction. - TestQueryFlushIgnoreChanges: IgnoreChanges=true keeps suppressing the flush for an inherited flush mode, for AUTO set on the query, across a setFlushMode(AUTO)-then-COMMIT sequence and through an addNamedQuery() / createNamedQuery() round trip. Each hunk is discriminated by reverting it alone: the kernel access-path hunk by the two collection-path tests, the correlated-join hunk by the correlated-join test, openjpa-persistence/.../criteria/ by the five Criteria tests, and the QueryImpl hunk by three of the four IgnoreChanges tests, each on an assertNotSQL for an UPDATE that should not have been emitted. Reverting the whole diff at once does not discriminate, and deliberately so: seven of the eighteen tests pass against master as well, because the blanket flush there masks the incomplete access paths - everything is flushed anyway. Those seven are regression guards (dirty candidate type, dirty subclass, JPQL navigation, subquery type, COMMIT, no active transaction, AUTO-then-COMMIT), and the two navigation ones also back up the finding that traversePath already covered o.customer.spouse.
cristof
approved these changes
Oct 18, 2026
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
3 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.
Section 3.10.8 scopes the duty to updates "which could potentially affect the result of the query" and lets the provider discharge it "by flushing those entities to the database or by some other means" — the access-path check is exactly that other means. The 4.2.0 line replaced it with "is anything dirty at all", which flushes the whole context before every query in a write-heavy transaction.
The justification in that change — that the access path "may miss dirty entities reachable via navigation (e.g.
o.customer.spouse)" — turns out to be false for JPQL:traversePath()already registers every hop, and the reproducer committed with it passes unchanged with the access-path check restored. It was true for the Criteria API, whereevalAccessPaths()looked at roots and their immediate joins only, with a standingTODO -- need to handle subqueries. So this fixes the access path at the source — implicit joins, joins of joins, fetch joins, subquery types, and joins made on a correlated join — instead of flushing everything. Review also found JPQL collection-valued paths omitted element and key types, which the Criteria side already registered; that asymmetry is closed too.The
IgnoreChangesoverride is replaced by the distinction 3.10.8 itself draws:Query.setFlushMode(AUTO)set explicitly on the query now overridesopenjpa.IgnoreChanges=true, while an inherited flush mode keeps honouring it — the option is documented and recommended together withFlushBeforeQueries=truein the optimisation guide, so silently disabling it was not an option. The two prohibitions in that section already hold and are now pinned by tests:isActive()covers "no transaction active", and an unjoined context cannot reach the flush becauseSynchronizationType.UNSYNCHRONIZEDis rejected outright.No compatibility flag: the broadened behaviour exists only on the unreleased 4.2.0 line, so nothing released changes. The migration note is rewritten instead.
18 new tests in
persistence/flush. Verified 1332 tests with no failures on this branch and on pristine master, on both the Derby and H2 profiles. Each production hunk is discriminated by reverting it alone; reverting the whole diff does not discriminate, because master flushes everything anyway and so masks the incomplete access paths — seven of the eighteen are therefore regression guards, as the commit message says.