gustavoharff commented on code in PR #16559:
URL: https://github.com/apache/grails-core/pull/16559#discussion_r4258630903
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java:
##########
@@ -305,20 +305,27 @@ 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);
+ fromsByProvider.registerAlias(associationName, associationRoot);
Review Comment:
It is needed, for a case the specs didn't cover: a path through the
association after a nested block, when an outer context has already realized an
alias of the same name. In
```groovy
Pet.createCriteria().list {
eq('face.name', 'Santa')
owner {
face { isNotNull('id') }
eq('face.name', 'Homer')
}
}
```
the top-level `face.name` realizes `face` as the pet's face. Inside `owner`,
`resolveFromAlias` takes the alias realized by the outer context (`getRealized`
walks the parents) before it looks at the join tracker of this context, so
without the line `face.name` resolves to the pet's face and the query returns
nothing; with it, it resolves to the owner's face and returns Lucky.
You're right that it changes nothing at the top level: a block of the
queried class always has a definition, and the old code realized it through
`resolveFromAlias`. It only matters for nested blocks. I added this case to
`HibernateCriteriaBuilderNestedJoinTypeSpec` and `PredicateGeneratorSpec` (both
fail without the line) and a comment on the line in 1b7aba9882.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java:
##########
@@ -305,20 +305,27 @@ 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);
+ fromsByProvider.registerAlias(associationName, associationRoot);
}
// Create a nested context for this association
JpaQueryContext nestedContext = new JpaQueryContext(fromsByProvider,
null, associationRoot);
+ // The join type of an association block nested in this one, such as
the LEFT of face in
+ // owner(LEFT) { face(LEFT) { ... } }, is recorded on this block's
criteria. It is registered as a join type,
+ // not as an alias, so a criterion on the property of the same name,
such as isNotEmpty('pets') next to
+ // pets { ... }, still resolves to the property rather than to the
join.
+
associationCriteria.getJoinTypes().forEach(nestedContext::registerJoinType);
Review Comment:
Agreed. I narrowed it to a criterion that comes before the block, here and
in the Javadoc of `JpaQueryContext.registerJoinType`, which made the same
claim. I'll open a follow-up issue for the resolver returning a join for a
collection-valued path.
--
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]
