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


##########
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());
+        } else {
+            Path<?> path = getPath(joinTracker.getRoot(), def.path());
+            boolean elementCollection = path != null && path.getModel() 
instanceof PluralAttribute<?, ?, ?> attribute &&
+                    attribute.getPersistentAttributeType() == 
PersistentAttributeType.ELEMENT_COLLECTION;
+            boolean sharesPath = !elementCollection && 
def.equals(aliasRegistry.getAssociationAlias(def.path()));
+            From<?, ?> join = sharesPath ? joinTracker.getOwnJoin(def.path()) 
: null;
+            if (join == null) {
+                join = joinTracker.getRoot().join(def.path(), def.joinType());
+                if (sharesPath) {
+                    joinTracker.addJoin(def.path(), join);
+                }
             }
+            aliased = join;
         }
+        aliasRegistry.realize(alias, aliased);
+        return subPath != null ? getPath((From<?, ?>) aliased, subPath) : 
aliased;
+    }
 
-        return null;
+    /**
+     * Resolves an association path independently of named aliases, even when 
an alias has the same name.
+     */
+    public From<?, ?> resolveAssociation(String path, JoinType joinType) {
+        From<?, ?> join = joinTracker.getOwnJoin(path);
+        if (join == null) {
+            HibernateAlias associationAlias = 
aliasRegistry.getAssociationAlias(path);

Review Comment:
   > Every explicitly requested alias has to be materialized, even when no 
restriction, projection, or order names it.
   
   @jamesfredley, that holds for entity associations, and your `favorite` pair 
is fixed and covered at 64ac919cb4. For a collection of basic values it changes 
results that 8.0.x returns, so in 
https://github.com/apache/grails-core/pull/16573#discussion_r4360519138 I've 
asked to exempt basic-collection aliases. Why:
   
   1. 8.0.x never joined an unused basic-collection alias, and the queries that 
depend on that return correct rows there. With `hasMany = [schools: String]`, 
at 64ac919cb4:
      - `createAlias('schools', 's')` alone: 8.0.x returns `[grace, hank, 
none]`, this head `[grace, grace, hank]`.
      - `createAlias('schools', 's'); 'in'('schools', ['Princeton'])`: 8.0.x 
returns `[grace, hank]` and `count()` 2, this head `[grace, grace, hank]` and 
3. That is the duplicate-row symptom #16562 fixes, and 
`BasicCollectionInQuerySpec` was rewritten to expect the duplicate.
   2. For entity associations, materializing changes no 8.0.x result, so the 
exemption does not bring back the bug in this thread. On 8.0.x a single unused 
entity alias already joined through its association criteria (an unused inner 
`createAlias('favorite', 'a')` excludes the team without a favorite), and the 
LEFT/INNER pair above threw `PathElementException`.
   3. Verified locally (not pushed): with materialization limited to entity 
aliases, `unreferenced to-one aliases retain the inner join ...` and 
`unreferenced mixed aliases survive counts and pagination` still pass, 
including their assertion that the INNER join reaches the SQL. The full 
`:grails-data-hibernate7-core:test` has one failure: the rewritten 
`BasicCollectionInQuerySpec` row, which goes back to `['[email protected]', 
'[email protected]']`.
   
   > Before this change the association used the last join type.
   
   That applies to entity associations only. A basic-collection alias has no 
association criteria, so 8.0.x had no join for it to use when nothing 
referenced the alias.
   



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