gustavoharff commented on code in PR #16559:
URL: https://github.com/apache/grails-core/pull/16559#discussion_r4371799717


##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java:
##########
@@ -305,20 +305,31 @@ private Predicate handleAssociationCriteria(
         JpaQueryContext fromsByProvider,
         DetachedAssociationCriteria<?> associationCriteria) {
         String associationName = associationCriteria.getAssociationPath();
-        From<?, ?> associationRoot = fromsByProvider.getFrom(associationName);
+        // The block joins from the entity its context queries, with the join 
type recorded for it there. A join that
+        // an outer block or an outer query made for an association of the 
same name, such as face in
+        // face { ... } owner { face { ... } } or owner in a subquery, is 
another table, so only the joins of this
+        // context are reused.
+        From<?, ?> associationRoot = 
fromsByProvider.getLocalFrom(associationName);
         if (associationRoot == null) {
-            // Check if we already have it in our parent or alias map
-            Expression<?> expr = 
fromsByProvider.getFullyQualifiedExpression(associationName);
-            if (expr instanceof From<?, ?> from) {
-                associationRoot = from;
-            } else {
-                associationRoot = 
fromsByProvider.getRoot().join(associationName);
-                fromsByProvider.addFrom(associationName, associationRoot);
-            }
+            HibernateAlias definition = 
fromsByProvider.getLocalAliasDefinition(associationName);
+            associationRoot = definition != null ?
+                    fromsByProvider.getRoot().join(definition.path(), 
definition.joinType()) :
+                    fromsByProvider.getRoot().join(associationName, 
fromsByProvider.getLocalJoinType(associationName));
+            fromsByProvider.addFrom(associationName, associationRoot);
+            // A later path through the association, such as face.name in
+            // eq('face.name', 'Santa') owner { face { ... } eq('face.name', 
'Homer') }, is resolved against the aliases
+            // realized by the outer contexts before the joins of this one, so 
the join is realized as an alias of this
+            // context too.
+            fromsByProvider.registerAlias(associationName, associationRoot);

Review Comment:
   Thanks, you're right. I reproduced it: with an `owner { ... }` block in the 
outer criteria, a subquery projecting `owner.firstName` came out correlated 
(`select o2_0.first_name from pet p2_0, person o2_0 where 
o2_0.id=p1_0.owner_id`), both for a `DetachedCriteria` and for a subquery 
inside a nested block. Neither is correlated on 8.0.x, so the alias 
registration regressed it.
   
   In 1b9594c751 the association block no longer realizes its join as an alias. 
It only records the join in the join tracker of its context. 
`ExpressionResolver` now resolves a path to a join made from the context's own 
root before it looks at the aliases realized by the outer contexts. That keeps 
`face.name` (and `face`) after `face { ... }` inside `owner { ... }` on the 
owner's face, which is the case the registration was added for, without 
publishing anything to a subquery. Explicit aliases still correlate as before.
   
   Regressions added:
   
   - `HibernateCriteriaBuilderNestedJoinTypeSpec`: your example with a 
`DetachedCriteria`, and a subquery inside a nested block projecting a path 
through that block's association.
   - `PredicateGeneratorSpec`: an outer association block and a subquery 
projecting the same path.
   - `ExpressionResolverSpec`: a join from the context's own root winning over 
an alias the parent realized, and a subquery path not taken through a parent 
join that no alias defines.
   
   One case still correlates, and it does on 8.0.x too: a top-level block in 
`createCriteria`. Every block there records a join type, and 
`registerDetachedJoins` turns it into an alias definition named after the path, 
which a subquery then sees through `getDefinition` and the parent 
`JoinTracker`. Fixing it means separating those automatic definitions from 
explicit aliases, so I opened #16762 for it rather than widening this PR.
   
   I also merged 8.0.x, which brings #16758, the fix for the collection 
criterion next to its block that @jdaugherty pointed out.
   



-- 
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