Repository navigation
Hibernate 7: fix subquery correlation with top-level association blocks - #16773
Open
borinquenkid wants to merge 1 commit into
Open
borinquenkid wants to merge 1 commit into
borinquenkid wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Alias resolution still has correctness issues, and several regression tests do not distinguish correct from incorrect correlation.
5 open issues
Review effort: Balanced
Comment on lines
+119
to
+120
| if (!aliasRegistry.hasLocalDefinition(alias) && context != null && context.getParent() != null) { | ||
| Expression<?> parentExpr = context.getParent().getFullyQualifiedExpression(alias); |
|
|
||
| public boolean hasAlias(String alias) { | ||
| return aliasRegistry.isDefined(alias) || aliasRegistry.hasRealized(alias) || (parent != null && parent.hasAlias(alias)); | ||
| return aliasRegistry.isDefined(alias) || aliasRegistry.hasRealized(alias); |
Comment on lines
+68
to
+74
| when: | ||
| def results = SubqueryCorrPet.createCriteria().list { | ||
| owner { | ||
| eq('firstName', 'Ned') | ||
| } | ||
| 'in'('name', new DetachedCriteria(SubqueryCorrPet).build { | ||
| eq('owner.firstName', 'Ned') |
Comment on lines
+87
to
+95
| when: | ||
| def results = SubqueryCorrPet.createCriteria().list { | ||
| owner { | ||
| eq('firstName', 'Ned') | ||
| } | ||
| 'in'('name', new DetachedCriteria(SubqueryCorrPet).build { | ||
| owner { | ||
| eq('firstName', 'Ned') | ||
| } |
Comment on lines
+114
to
+121
| property('name') | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| then: | ||
| results.size() == 2 | ||
| results*.name.sort() == ["Homer", "Santa's Little Helper"] |
jamesfredley
left a comment
jamesfredley
left a comment
Contributor
There was a problem hiding this comment.
Not approving. A subquery that correlates through a nested association loses the outer join, and an alias declared inside an association block is materialized on the wrong root.
jamesfredley
left a comment
jamesfredley
left a comment
Contributor
There was a problem hiding this comment.
The nested createAlias is fixed. The alias stays on the association that declared it. That thread is resolved. Not approving. A comparison through an inherited join is still dropped, and two sibling blocks that join the same name share one join.
jamesfredley
left a comment
jamesfredley
left a comment
Contributor
There was a problem hiding this comment.
An inherited comparison is kept, and homeAddress { country { } } no longer shares its join with workAddress { country { } }. Not approving. The same two blocks written as eq('country.name', ...) still both constrain the home country, so a USA home and a Canadian workplace match nothing. 8.0.x can keep this bugfix.
| public void registerAlias(String alias, Expression<?> expression) { | ||
| aliasRegistry.realize(alias, expression); | ||
| if (!isSubquery && parent != null) { | ||
| parent.registerAlias(alias, expression); |
Contributor
There was a problem hiding this comment.
homeAddress { eq('country.name', 'USA') } then workAddress { eq('country.name', 'Canada') } should return the person with that pair of addresses. registerAlias publishes the implicit country join to the shared parent, so the second predicate reuses the home country and the person is excluded. Keep that implicit path on the association that declared it.
borinquenkid
force-pushed
the
fix/h7-subquery-alias-correlation-8.0.x
branch
from
October 30, 2026 01:54
61fc4ed to
89207e3
Compare
Member
Author
|
Updated and rebased on The issue with dotted association properties in sibling blocks (such as In public void registerAlias(String alias, Expression<?> expression) {
aliasRegistry.realize(alias, expression);
if (!isSubquery && parent != null && aliasRegistry.isExplicit(alias)) {
parent.registerAlias(alias, expression);
}
}Only explicit user-declared aliases propagate upward, ensuring implicit association navigation aliases remain scoped to their declaring block whether written as nested association closures or dotted property expressions. |
jamesfredley
left a comment
jamesfredley
left a comment
Contributor
There was a problem hiding this comment.
eq('country.name', ...) on two sibling blocks no longer constrains the first country. Not approving. Resolving an implicit join marks that join explicit, so a later subquery correlates through it.
| if (def != null) { | ||
| return def.explicit(); | ||
| } | ||
| if (realizedExpressions.containsKey(alias)) { |
Contributor
There was a problem hiding this comment.
An outer DetachedCriteria on the pet does owner { eq('firstName', 'Ned'); eqProperty('firstName', 'owner.firstName') }, then an in('name', ...) subquery that should project its own owner's first name. The comparison realizes the tracked owner join with no definition, isExplicit() becomes true, and the subquery projects the outer Ned instead of joining its owner. The pet that should match is missing. Keep sibling isolation, and keep an implicit join implicit when it is realized.
…ery correlation to explicit aliases Differentiates explicit user-declared aliases from implicit joins, restricts query context alias propagation strictly to explicit aliases, ensures implicit joins realized during translation do not mark aliases explicit in AliasRegistry, and prevents subqueries from correlating through outer implicit paths while preserving outer joins and independent joins across sibling association blocks.
borinquenkid
force-pushed
the
fix/h7-subquery-alias-correlation-8.0.x
branch
from
October 31, 2026 04:56
89207e3 to
39c1816
Compare
Member
Author
|
Updated with clean alias classification and scope isolation. Architectural Context & Resolution1. Explicit User Aliases vs. Implicit Navigation Joins
2. Why Realization Never Confers Explicit Status
3. ExpressionResolver Isolation
4. Test Coverage
|
🚨 TestLens detected 13 failed tests 🚨Failed Jobs without Test Failures❌ CI / Build Grails Forge (Java 21, indy=true) Test SummaryTip Inspect all details with the TestLens CLI: CI / Build Grails Forge (Java 21, indy=false) > :grails-forge-core:test [grails-forge]
CI / Build Grails Forge (Java 25, indy=false) > :test-core:test [grails-forge]
CI / Build Grails-Core (Ubuntu JDK 21) > :grails-web-url-mappings:test
CI / Build Grails-Core (Ubuntu JDK 25) > :grails-openapi:test
CI / Build Grails-Core (Ubuntu JDK 25) > :grails-web-url-mappings:test
CI / Build Grails-Core (macOS JDK 21) > :grails-openapi:test
CI / Build Grails-Core Rerunning all Tasks (Ubuntu JDK 21 shard 1) > :grails-openapi:test
CI / Build Grails-Core Rerunning all Tasks (Ubuntu JDK 21 shard 2) > :grails-web-url-mappings:test
Groovy Snapshot Canary Build / Build Grails (shard 2) > :grails-web-url-mappings:test
🏷️ Commit: 39c1816 Test Failures (first 10 of 13)LinkGeneratorSpec > cache key should use identity of resource value (:grails-web-url-mappings:test in Groovy Snapshot Canary Build / Build Grails (shard 2))
LinkGeneratorSpec > resource links should use ident and allow controller override (:grails-web-url-mappings:test in Groovy Snapshot Canary Build / Build Grails (shard 2))
GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation (:grails-forge-core:test [grails-forge] in CI / Build Grails Forge (Java 21, indy=false))AsciidoctorBuildSpec > an app with asciidoctor builds its guide, without the configuration cache the plugin does not support (:test-core:test [grails-forge] in CI / Build Grails Forge (Java 25, indy=false))LinkGeneratorSpec > cache key should use identity of resource value (:grails-web-url-mappings:test in CI / Build Grails-Core (Ubuntu JDK 21))
LinkGeneratorSpec > resource links should use ident and allow controller override (:grails-web-url-mappings:test in CI / Build Grails-Core (Ubuntu JDK 21))
LinkGeneratorSpec > cache key should use identity of resource value (:grails-web-url-mappings:test in CI / Build Grails-Core (Ubuntu JDK 25))
LinkGeneratorSpec > resource links should use ident and allow controller override (:grails-web-url-mappings:test in CI / Build Grails-Core (Ubuntu JDK 25))
PersistentEntitySchemaSpec > describes the resource as a plain type when the application has no mapping context (:grails-openapi:test in CI / Build Grails-Core (Ubuntu JDK 25))PersistentEntitySchemaSpec > describes the resource as a plain type when the application has no mapping context (:grails-openapi:test in CI / Build Grails-Core (macOS JDK 21))Rerun ControlsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app/docs. |
This branch has not been deployed
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
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.
Fixes #16762
Problem
In Hibernate 7 criteria queries, top-level association blocks (such as
owner { eq('firstName', 'Ned') }) and un-aliased association joins record the join path indetachedCriteria.getJoinTypes(). During JPA criteria translation,JpaCriteriaQueryCreatorregistered these join paths as aliases inAliasRegistryand tracked the materialized joins inJoinTracker. When an inner subquery was created viaJpaQueryContext.forSubquery(parentContext, ...)and projected a property through an association (such asproperty('owner.firstName')), the subquery context delegated to the parent query'sAliasRegistryandJoinTracker, resolving the projection path against the parent query's physical join rather than creating an independent join on the subquery's root entity. This caused the subquery to unintentionally correlate with the outer query.Additionally, when an explicit alias was declared on an outer query and referenced inside a subquery,
ExpressionResolverwas attempting to materialize the join on the subquery's root rather than delegating materialization to the parent context that owns the alias.Solution
explicitflag toHibernateAlias.AliasRegistryparent delegation (isDefined,getDefinition,getRealized,hasRealized) so that child subquery contexts only inherit explicit aliases from the parent query.ExpressionResolverto resolve and materialize parent-defined explicit aliases on the parent context that owns them.JpaQueryContextso that subquery contexts do not delegate physical joins to the parent'sJoinTracker.SubqueryAssociationCorrelationSpeccovering projections, criteria, nested association blocks, and correlated outer aliases, as well as unit tests inAliasRegistrySpecandJpaQueryContextSpec.