jdaugherty commented on code in PR #16573:
URL: https://github.com/apache/grails-core/pull/16573#discussion_r4413143669
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java:
##########
@@ -306,20 +305,11 @@ private Predicate handleAssociationCriteria(
JpaQueryContext fromsByProvider,
DetachedAssociationCriteria<?> associationCriteria) {
String associationName = associationCriteria.getAssociationPath();
- From<?, ?> associationRoot = fromsByProvider.getFrom(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);
- }
- }
+ From<?, ?> associationRoot =
fromsByProvider.resolveAssociation(associationName);
// Create a nested context for this association
- JpaQueryContext nestedContext = new JpaQueryContext(fromsByProvider,
null, associationRoot);
+ JpaQueryContext nestedContext = new
QueryAliasRegistrar(associationCriteria, null)
+ .createContext(fromsByProvider, associationRoot);
Review Comment:
`createAlias` inside a junction makes the junction always true. GORM core's
`createAlias` adds the alias's `DetachedAssociationCriteria` through `add()`,
so inside `or { }` it lands in the disjunction (the `HibernateAlias` criterion
goes to the top-level list). That association criteria has no criteria of its
own, so the `return` two lines below yields `criteriaBuilder.and()` with no
predicates, a `true` that short-circuits the `or`.
With `Team { String name; Like favorite }` (`favorite` nullable), five teams
of which three have a favorite:
```groovy
Team.withCriteria {
or {
createAlias('favorite', 'f', 1)
isNull('f.id')
eq('f.label', 'likeB')
}
}
```
| | result |
|---|---|
| Hibernate 5 (7.0.x) | the four expected teams: two without a favorite, two
whose favorite is `likeB` |
| 8.0.x (87fdc9f3f8) | three teams: the two with `likeB` and the one with
`likeA`; the two without a favorite are dropped |
| this head | all five teams |
Returning `null` from this method when `criteriaList` is empty, after
`createContext` so that an unused nested alias still joins, gives the Hibernate
5 result; with that change the full module suite still passes (4396 tests, 0
failures, 18 skipped). An `or { }` holding only a `likes { }` block keeps its
join either way, since `resolveAssociation` already ran above. Please add that
and a test for the `or { createAlias(..., 1); isNull(...); eq(...) }` shape,
with `count()` and `totalCount` if you like, since the reference page's own
example is this `or` block with the alias one line outside it.
##########
grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java:
##########
@@ -308,49 +284,14 @@ private void assignOrderBy(CriteriaQuery<?> cq,
JpaQueryContext context) {
private boolean isCountOnly() {
List<Query.Projection> projectionList =
projections.getProjectionList();
return !projectionList.isEmpty() && projectionList.stream()
- .allMatch(projection -> projection instanceof
Query.CountProjection ||
- projection instanceof Query.CountDistinctProjection);
- }
-
- private void discoverAliases(List<Query.Criterion> criteria,
JpaQueryContext context) {
- if (criteria == null) return;
- for (Query.Criterion criterion : criteria) {
- if (criterion instanceof HibernateAlias ha) {
- // If the alias is already defined in parent and materialized,
just link it
- if (!context.hasAlias(ha.alias())) {
- context.registerAlias(ha.alias(), ha);
- }
- } else if (criterion instanceof DetachedAssociationCriteria<?>
dac) {
- if (dac.getAlias() != null) {
- context.registerAlias(dac.getAlias(), new
HibernateAlias(dac.getAssociationPath(), dac.getAlias()));
- }
- discoverAliases(dac.getCriteria(), context);
- } else if (criterion instanceof Query.PropertyNameCriterion pnc) {
- String propertyName = pnc.getProperty();
- if (propertyName.contains(".")) {
- String alias = propertyName.substring(0,
propertyName.indexOf("."));
- // Only register if not already known in this or parent
context
- if (!context.hasAlias(alias)) {
- context.registerAlias(alias, new HibernateAlias(alias,
alias));
- }
- }
- } else if (criterion instanceof Query.Junction junction) {
- discoverAliases(junction.getCriteria(), context);
- }
- }
- }
-
- private void registerDetachedJoins(JpaQueryContext context) {
- detachedCriteria.getJoinTypes().forEach((path, joinType) ->
- context.registerAlias(path, new HibernateAlias(path, path,
joinType))
- );
+ .allMatch(projection -> projection instanceof
Query.CountProjection || projection instanceof Query.CountDistinctProjection);
Review Comment:
Still an unrelated hunk: 8.0.x has this lambda on two lines (`... instanceof
Query.CountProjection ||` and `projection instanceof
Query.CountDistinctProjection);` on the next), and this joins them into one
140-character line. Restoring the two lines leaves `isCountOnly()` untouched by
the PR, which is what matrei and sbglasius asked for.
##########
grails-doc/src/en/ref/Domain Classes/createCriteria.adoc:
##########
@@ -75,10 +75,42 @@ The `sort` and `order` arguments accepted alongside the
pagination values are ch
=== Description
+Criteria queries are a type-safe, advanced way to query that uses a Groovy
builder to construct potentially complex queries. It is a much better
alternative to using a StringBuilder to dynamically construct an HQL query.
Refer to the criteria sections in the GORM for
link:{hibernate7GuideFromRef}index.html#criteria[Hibernate 7] or
link:{hibernate5GuideFromRef}index.html#criteria[Hibernate 5] guide for usage
instructions.
-Criteria queries are a type-safe, advanced way to query that uses a Groovy
builder to construct potentially complex queries. It is a much better
alternative to using a StringBuilder to dynamically construct an HQL query.
Refer to the user guide section on
link:{hibernate5GuideFromRef}index.html#criteria[Criteria] for usage
instructions.
+==== Association aliases
-Method reference:
+With Hibernate, `createAlias('association', 'alias')` creates an inner join
and lets you refer to the association as `alias.property`.
+
+In both Hibernate 5 and Hibernate 7, the three-argument overload accepts `0`
for an inner join, `1` for a left outer join, or `2` for a right outer join.
Hibernate 5 also supports `4` for a full join. Hibernate 7 does not support
full joins through this overload: any value other than `1` or `2`, including
`4`, produces an inner join.
+
+For example, to include books with no author as well as books whose author has
a name:
+
+[source,groovy]
+----
+def books = Book.createCriteria().list {
+ createAlias('author', 'a', 1) // left outer join
+ or {
+ isNull('author')
+ isNotNull('a.name')
+ }
+}
+----
+
+The alias uses the requested join type. A left join alone does not guarantee
that unmatched rows remain in the result: the restrictions must also allow the
missing association, as the `or` block does above.
+
+In Hibernate 7, every explicitly requested alias of an entity association
creates a join even when no restriction, projection, or order references it.
Declaring a left alias and an inner alias for the same association therefore
excludes rows with no associated entity, even if the inner alias is otherwise
unused.
+
+An alias declared inside an association block is relative to that association,
not the outer domain class. In Hibernate 7, aliases declared in a `list { ...
}` closure do not carry over to a subsequent `list { ... }` call on the same
criteria builder.
Review Comment:
This sentence has no qualifier while the paragraph above it has one, so it
reads as a rule for both Hibernate versions, and on Hibernate 5 it is the other
way round: the path is resolved from the domain class being queried. With the
domain above plus `Tag tag` on both `Team` and `Like`:
| query | Hibernate 5 (7.0.x) | this head |
|---|---|---|
| `likes { createAlias('tag', 't') }; eq('t.name', 'a')` | the teams whose
own `tag` is `a` and that have a like | `ConfigurationException: Cannot use
comparison criteria on non-existent property [t.name]` |
| `likes { createAlias('tag', 't'); eq('t.name', 'a') }` | `QueryException:
could not resolve property: likes_alias1` | the teams with a like tagged `a` |
So the first shape is the one thing in this area that worked on 7.x and now
fails, with a clear error rather than a different result, which I think is
fine. Please start the sentence with "In Hibernate 7," and add that Hibernate 5
resolves the alias path from the domain class being queried, so a reader
upgrading knows which query to move the `createAlias` out of the block.
--
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]
