jdaugherty commented on code in PR #16573:
URL: https://github.com/apache/grails-core/pull/16573#discussion_r4352592429


##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/ExpressionResolver.java:
##########
@@ -112,7 +112,13 @@ private Expression<?> resolveFromAlias(String alias, 
String subPath) {
         if (aliasRegistry.isDefined(alias)) {
             HibernateAlias def = aliasRegistry.getDefinition(alias);
             if (def != null && def.path() != null) {
-                aliased = joinTracker.getRoot().join(def.path(), 
def.joinType());
+                // The association criteria and its alias must refer to the 
same physical join,
+                // regardless of whether a predicate, projection or order 
resolves it first.
+                aliased = joinTracker.getJoin(def.path());

Review Comment:
   Verified at 644adf856e: the `inList('f.label', namesWithFavoriteY)` query 
returns `[x, y, z]`, and the regression test is in.
   
   The association-block shape from the end of my comment (`favorite { 
eq('label', 'y') }` inside the subquery, `inList('favorite.label', ...)` 
outside) is now fixed too, through `resolveAssociation` in 
`PredicateGenerator`: the merge-base returns `[x, y]`, this head `[x, y, z]`. 
Nothing covers it yet, so please add it to `CreateAliasJoinSpec` as well.



##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/ExpressionResolver.java:
##########
@@ -101,25 +104,45 @@ private Expression<?> resolveFromAlias(String alias, 
String subPath) {
             return aliased; // It's a non-From expression (projection alias) 
or subPath is null
         }
 
-        // 2. Check if already joined in THIS or PARENT JoinTracker 
(correlated joins)
-        aliased = joinTracker.getJoin(alias);
-        if (aliased != null) {
-            aliasRegistry.realize(alias, aliased);
-            return subPath != null ? getPath((From<?, ?>) aliased, subPath) : 
aliased;
+        HibernateAlias def = aliasRegistry.getDefinition(alias);
+        if (def == null || def.path() == null) {
+            return null;
         }
-
-        // 3. If defined but not joined, materialize it on the CURRENT root
-        if (aliasRegistry.isDefined(alias)) {
-            HibernateAlias def = aliasRegistry.getDefinition(alias);
-            if (def != null && def.path() != null) {
-                aliased = joinTracker.getRoot().join(def.path(), 
def.joinType());
-                aliasRegistry.realize(alias, aliased);
-                joinTracker.addJoin(alias, (From<?, ?>) aliased);
-                return subPath != null ? getPath((From<?, ?>) aliased, 
subPath) : aliased;
+        if (!def.explicit()) {
+            aliased = resolveAssociation(def.path(), def.joinType());

Review Comment:
   A subquery over another entity can no longer use the outer query's 
association path to correlate. With `Team { String name; Like favorite }` and 
`Like { String label }`, and teams A (favorite `first`) and B (favorite 
`second`):
   
   ```groovy
   Team.withCriteria {
       isNotNull('favorite.label')
       exists(new DetachedCriteria(Like).build {
           eq('label', 'second')
           eqProperty('favorite.label', 'label')
       }.id())
       projections { property('name') }
   }
   ```
   
   On the merge-base 0e8fd03e56 this returns `[B]`. At this head it throws 
`PathElementException: Could not resolve attribute 'favorite' of 'Like'`. 
`QueryAliasRegistrar.discover` now re-declares the inherited `favorite` path in 
the subquery, and this branch joins it from the subquery's root, which has no 
such attribute. The same query with `createAlias('favorite', 'f')` and 
`isNotNull('f.label')` in the outer query fails the same way, because 
`createAlias` also calls `join('favorite', ...)` and `registerDetachedJoins` 
turns that into an implicit outer alias named `favorite`.
   
   A path the current root does not have can only mean the enclosing query's 
join. This change made both variants return `[B]` in a local probe (not pushed):
   
   ```java
   if (!def.explicit()) {
       From<?, ?> enclosing = getPath(joinTracker.getRoot(), def.path()) == 
null ? joinTracker.getJoin(def.path()) : null;
       aliased = enclosing != null ? enclosing : resolveAssociation(def.path(), 
def.joinType());
   }
   ```
   
   With it, the 597 tests of `CreateAliasJoinSpec`, `SubqueryAliasSpec`, 
`ExistsCrossJoinSpec`, the TCK `DetachedCriteriaSpec` and the 
`detachedcriteria`, `hibernatequery` and `org.grails.orm.hibernate.query` spec 
packages still pass, and a plain `eq('favorite.label', 'second')` in a subquery 
over `Team` still joins from the subquery's own root. Please add both variants 
to `CreateAliasJoinSpec`. The sentence at `createCriteria.adoc:84` then needs a 
qualifier: the subquery's own root is used when its entity has that association.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to