[matrei]

Description

Fixes #15589.

Sorting a where query by a property of an association failed with a QueryException when the query also had criteria on that association:

Team.where {
    club.name == 'Manchester United'
}.list(sort: 'club.name')
// org.hibernate.QueryException: could not resolve property: club.name of: grails.gorm.tests.Team

The criteria join club under a generated alias, such as club_alias0, and AbstractHibernateQuery records the join under both the association path and the alias. order() found the join, but added the order with the association path, which Hibernate cannot resolve against the root entity. The sort only worked with an explicit alias defined in the where query, because then the sort property already started with the alias. The order is now added with the alias of the join, as club_alias0.name.

The same sort already worked when the query had no criteria on the association, and the Hibernate 7 version of WhereQueryWithAssociationSortSpec on 8.0.x already expects it to work.

A sort by a path through more than one association, such as team.club.name, failed whether or not the where query joined the nested association, because order() split the path at its first dot only. order() now walks the path one association at a time. It reuses a join the criteria registered under the full path, such as team.club, and joins any other association on the path for the sort, registering that join like any other. A sort through an explicit alias, such as t.club.name, works the same way. Because the sort join is registered instead of created anew for every order, a sort by two properties of the same association no longer fails with duplicate association path.

Sorting by an explicit alias produces the same order as before, and so does sorting by the association's id, such as club.id, in a query with criteria on club, which now goes through the join instead of the foreign key.

Sorting no longer leaves out rows

An association that was joined only for a sort was joined with an inner join, so the sort left out the instances whose association is null. Team.list(sort: 'club.name') did not return the teams without a club, Team.findAllWhere([name: 'A team without a club'], [sort: 'club.name']) did not find such a team at all, and the totalCount of a paged list did not count them. A sort should not change which rows a query returns, so the association is now joined with a left join. This applies to where queries, detached criteria and dynamic finders, and to list, find and findAll with an example, findWhere, findAllWhere and a criteria query that is passed pagination arguments.

• A where query that has criteria on the association still joins it for those criteria, with an inner join by default, and the sort uses that join.
• A join type that a where query requests for the association, as fetch: [club: JoinType.INNER] does, is used for the sort as well. Before, the sort ignored a requested join type and always created an inner join.
• Every join of a where query is made when the query is executed, not when the sort, the criteria or the projection that needs it is added. A sort stored on a where query, as in Team.where { ... }.sort('club.name'), is applied before the arguments of list are read and before criteria passed to list are added. A join type that fetch requests there is therefore used for a stored sort as well, and criteria passed to list join the association with their own join type, as they do without the sort. Those criteria can also join it through an alias, as in createAlias('club', 'c'), which failed after a stored sort, and the sort can be by that alias.
• order in a criteria query can sort by a property of an association too, as in Team.createCriteria().list { order('club.name') }, also by a path through more than one association and inside the criteria of an association. It failed with could not resolve property, and with a Query.Order it sorted by a property of the queried class instead. It joins with a left join, and uses a join that the criteria make for the association. The orders are added once the criteria are built, so an order also uses a join type that the criteria or the fetch argument of list request for its association, and does not keep a collection from being fetched that they ask to fetch, whether the request comes before or after the order. An order by the identifier of an association, such as club.id, makes no join and reads the foreign key, as it did before this PR, so it can still be combined with a projection that groups by the same path or is distinct on it. Inside the criteria of an association, a Query.Order for a single property is resolved against that association too, as an order by a property name is. An order by the identifier of a hasOne, as order('contract.id'), is joined with a left join as well: Hibernate has no foreign key to read it from, and joined it with an inner join, also before this PR.
• A sort by a path through more than one association keeps the rows too. team.club.id returns a player whose team has no club, as it did before this PR, when only team was joined and club.id was read from its foreign key.
• A sort does not change what a query fetches either. Hibernate initializes a collection from the rows of a left join that has no condition of its own, so the left join of a sort by a property of a collection, such as players.name, fetched the collection, and a row limit left elements out of it: with max: 2, a team came back with two of its three players. A sort now joins a collection without fetching it, unless the query asks for the collection to be fetched, and the collection is loaded when it is read, as it was before this PR. A collection that the query asks to fetch is fetched with and without the sort. Before this PR, the inner join of the sort took the place of the fetch. When the query fetches another collection, a sort by a collection fetches its collection as well: Hibernate gives a join of a collection that it does not fetch the column aliases of the next collection that it fetches, and fills that one with the elements of the other when both hold elements of the same class. Hibernate does not support fetching more than one bag, a collection declared as a Collection, in a query, so when both are bags the query throws an UnsupportedOperationException that names both collections, instead of leaving out the instances whose sorted bag is empty, as 7.0.x did. The condition that keeps a collection from being fetched is part of the where clause, not of the join condition, which Hibernate combines with the filter of a collection that has a join table, such as the tenant filter of discriminator based multi-tenancy, into SQL that no database accepts.

This changes results on 7.0.x, so section 22 of the upgrade notes describes it, including how to leave the instances out as before. #16863 makes the same change for Hibernate 7 on 8.0.x, where the two versions then return the same rows for these sorts. Apart from this, only queries that failed before behave differently.

A requested join type is applied without a sort or criteria

A where query, a detached criteria, a dynamic finder or a criteria query can request a join type for an association, with fetch: [club: JoinType.INNER] or join('club', JoinType.INNER). In a where query, a detached criteria and a dynamic finder, the type was only used when criteria, or with this PR a sort, joined the association. Otherwise the association was fetched with the outer join of its fetch mode: Team.where { ... }.list(fetch: [club: JoinType.INNER]) returned the teams without a club, and the same query with sort: 'club.name' did not, so a sort still changed the rows of such a query. A requested join type other than LEFT is now applied when the query is executed, whether or not anything else in the query uses the association.

• JoinType.INNER leaves out the instances whose association is null, from list, get, find, count and the totalCount of a paged list, and still fetches the association.
• JoinType.RIGHT leaves out the same instances and fetches the association. As a right join does, it also has a row for each associated instance that none of the queried instances refers to: list returns null for such a row and count counts it, unless the criteria of the query exclude it.
• An inner or a right join requested for a collection keeps the left fetch join, because Hibernate fetches a collection only through a left join, and restricts the query with a subquery that joins the collection. The collection stays fetched, and the count is of instances, not of joined rows. A right join of a collection therefore returns the same instances as an inner join. This also holds when the query is sorted by the collection. The joins of the subquery apply the filters that are enabled for the session, which a check that the collection is not empty does not: with discriminator based multi-tenancy, a shelf whose books all belong to another tenant is left out.
• A requested path through more than one association, such as fetch: ['team.club': JoinType.INNER], was ignored without criteria on the path. Every association on it is now joined and fetched. A path through a collection, as in fetch: ['players.contract': JoinType.INNER], restricts the query in a subquery, because a join through a collection returns an instance once for each element: count and the totalCount of a paged list count each instance once. The collection on the path is fetched with outer joins and holds all of its elements, also the players without a contract: the join type decides which teams the query returns and never leaves elements out of a fetched collection, which would leave a partial collection in the session. A where query and a dynamic finder return each team once, and a criteria query and a query by example once for each fetched player, as for any fetch of a collection. A JoinType.LEFT for such a path fetches the path too.
• A query used as a subquery, and a query with a property projection, apply the requested join type too. A subquery applies it once its criteria have been added, as a query that is executed itself does, so an association that the criteria of the subquery join keeps that join.
• A fetch mode that is requested for the association afterwards replaces the join type: .join('club', JoinType.INNER).list(fetch: [club: 'lazy']) returns the teams without a club, as it did before this PR. The same holds for join('club') or select('club') after join('club', JoinType.INNER), on a where query and in a criteria query, and for the fetch argument of a criteria query. The join that criteria on the association make keeps the requested join type, as it did before this PR, so Team.where { club.name == 'Arsenal' || name == 'Unattached' }.join('club', JoinType.LEFT).list(fetch: [club: 'join']) returns the team without a club.
• A requested join type is used for the join that the criteria or a projection of a where query make, whether join or the fetch argument of list requests it. The fetch argument is read after the criteria have been applied, so it was not used for such a join: Team.where { club.name == 'Chelsea' || name == 'Unattached First Team' }.list(fetch: [club: JoinType.LEFT]) left out the team without a club.
• A join type is requested for a nested association by its path. join('team.club', JoinType.LEFT) is used for the join that criteria on team.club make, which was an inner join whatever was requested, and a join type requested for club is no longer used for team.club. Criteria on a nested association can also be used more than once, which failed with duplicate association path.
• The criteria builder accepted a join type and never used it: join('club', JoinType.INNER) set a fetch mode like the one-argument join, and a JoinType in the fetch argument of list matched no fetch mode, so the association was not fetched at all. Both now join and fetch with the requested type. They are applied once the criteria are built, so an association that the criteria have a block for keeps the join of that block, as does one that the criteria join through an alias, as in createAlias('t.club', 'c').
• A where query that has criteria on a collection and requests JoinType.LEFT for it uses that join type for the join of the criteria, and no longer fetches the collection through that join, whose rows only have the elements that the criteria match. With join('players', JoinType.LEFT) the collection was initialized with those elements only, and with the fetch argument it would have been since this PR uses the argument for the join of criteria. The collection is now loaded when it is read, with all of its elements.
• updateAll and deleteAll apply a requested join type too, so they update or delete the instances that list returns. Team.where { ... }.join('club', JoinType.INNER) listed and counted the teams with a club, but deleteAll on it deleted the teams without one as well. An update and a delete cannot join, so each requested join is applied to them as a restriction: a subquery that selects from the entity of the first association on the path and joins the rest of it. MySQL, and MariaDB before version 10.3, do not allow an update or a delete to select from the table that it modifies. On these databases a join that leads back to the queried class, such as the parent of a category, is rejected with an UnsupportedOperationException that says to restrict the query with criteria instead. The check follows the dialect: a MySQL dialect, or a MariaDB dialect older than MariaDB103Dialect. deleteAll selects the identifiers first when the path starts with a collection that has a join table, because Hibernate deletes the join rows before the instances, with the same restriction, which then finds no instance. A one-to-one whose foreign key the other class has is matched through the property that refers back.
• list, findWhere, findAllWhere, and find and findAll with an example, apply a JoinType in their fetch argument too, before a sort by the association, which uses that join. They ignored it, and since a sort joins with a left join, a sort by the association no longer left out the instances without it either. list fetches the association with the requested join type, returns each instance once when it fetches a collection, and applies RIGHT like INNER, as Hibernate does not support right joins in criteria queries. The 'join' fetch mode of list is unchanged, which belongs to the open question about fetch in list() above.
• A requested join type applies to a collection of a basic type, such as hasMany = [tags: String], too. It was ignored.
• A right join requested for a collection that criteria on the collection join is made as an inner join, as for a collection that nothing else joins. It returned null for each element that no instance has.
• The associations before a collection on a requested path, such as team in team.players.contract, are joined and fetched, and so is the collection.
• A listener of the query events can add criteria on an association to find and findAll with an example, findWhere and findAllWhere. The joins that those criteria need are made after the event; they were left to a query that is not executed, and the query failed.

This also changes results on 7.0.x. Section 23 of the upgrade notes describes it, and the fetching guide now documents the join types.

A criteria query that is passed pagination arguments can now also be sorted by a path through more than one association. Player.createCriteria().list(sort: 'team.club.name', max: 10) { ... } failed when totalCount was read, and with duplicate association path when the criteria used team. The sort now reuses a join that the criteria have for an association path, and creates any other from the root criteria with its whole path, which is how the count recreates it. A join that the criteria make through an alias, as in createAlias('t.club', 'c'), is reused as well. It can also be sorted through an alias that the criteria give an association, as sort: 't.club.name' with createAlias('t.club', 'c'), and by a map of sort properties, which failed with a ClassCastException. An association after the first one on a sort path, or on a requested join path, is joined through the alias of the join before it, so that Hibernate does not read the path through an alias of the criteria that has the name of its first association.

list with a map of sort properties, such as Team.list(sort: ['club.name': 'asc', name: 'desc']), sorted by the last entry only, because each entry replaced the orders of the query. It now sorts by every entry. Hibernate 7 on 8.0.x already does. The same holds for a default sort that the mapping declares for more than one property, of which list applied the last one only. An association is joined once for all the sort properties that use it: list joined it once for each, and find and findAll with an example, findWhere and findAllWhere failed with duplicate association path.

The "Sorting with Associations" section of the where query docs said that this sort fails and that an explicit alias is required. It now shows sorting by the association path, notes that the path can cross more than one association, that sorting does not change which rows a query returns, and that a sort by an alias that the query does not define is ignored.

Tests

WhereQueryWithAssociationSortSpec asserted the QueryException. It now checks that a where query with criteria on club can be sorted by club.name:

• ascending and descending;
• with a sort map;
• with chained where calls and fetch: [club: 'join'].

Without the change, these cases fail with the QueryException. It also checks that the order is as before when sorting by club.id, by an explicit alias, and by club.name in a query without criteria on club.

For paths through more than one association, it checks sorting Player by team.club.name with criteria on team.club, with criteria on team only, and without criteria on either, and by t.club.name through an explicit alias of team. It also checks a sort by both club.name and club.id. These cases failed before the change.

For the left join, it checks with a team that has no club that sorts by club.name, club.id, team.club.name and team.club.id keep the team or its player, also through an explicit alias, with criteria on team, and in the totalCount of a paged list. It also checks that a join type requested with fetch is used, and that a criterion club != null leaves the team out. Team.club is nullable in the fixture for that.

For a sort stored on the query with sort, it checks that the team without a club is kept, that a join type requested with fetch in list or find is used, also for team.club.name and in the totalCount of a paged list, and that criteria passed to list return the same rows with and without the sort. Seven of these cases fail without the join being made when the query is executed.

The new AssociationSortSpec checks the same for list, findAll with an example, findWhere, findAllWhere, a criteria query with pagination arguments, and a dynamic finder. It also checks a paged criteria query sorted by team.club.name, with and without criteria on team and with criteria that join team.club through an alias, which failed before, that list applies every property of a default sort, a sort by two properties of one association, and order in a criteria query, including the sequence of several orders, an order that comes before the criteria or the alias of its association, an order by the identifier of an association together with a projection on it, and a Query.Order inside the criteria of an association.

Both specs check a sort by a property of a collection as well: that it keeps a team without players, and that it does not fetch the collection, with a row limit and for a single result, so that no player is left out of it, unless fetch asks for the collection.

56 of the 71 cases of AssociationSortSpec, and 24 of the 59 cases of WhereQueryWithAssociationSortSpec, fail on c0c5a80, the commit before the left join.

The new AssociationJoinTypeSpec checks a join type requested without a sort or criteria on the association: 'join', LEFT, INNER and RIGHT with list, including that the association is fetched, the same rows with and without a sort, count, the totalCount of a paged list, find, get, a dynamic finder, team.club, the players collection, including that an inner and a right join fetch it, also when the query is sorted by it, a path through the collection, which must not leave elements out of it, a fetch mode requested after a join type, a subquery, also one that has criteria on the association it requests a join type for, and a projection. It checks the same for a criteria query, with join and with the fetch argument of list, including count, get, the totalCount of a paged list, a sort, a collection, a nested path and an association the criteria have a block for or join through an alias. It also checks that a join type that fetch requests is used for the join of criteria and of a projection, a join type for a nested association, a fetch mode requested after a join type, and that a path through a collection returns and counts each instance once. For a criteria query that is ordered by an association or a collection, it checks that a join type and a fetch that join or the fetch argument request are applied, before and after the order. It also checks that a left join requested for a collection that a where query has criteria on does not fetch the collection, and that updateAll and deleteAll update or delete the instances that list returns, for an association, a collection, a path through either, and an association whose key the associated class has. 101 of its 135 cases fail without the changes. 28 cases of the two specs fail on 9c59ac4, where an order in a criteria query joined its association before the rest of the criteria was known, and nine fail on e8f7036, where updateAll and deleteAll did not apply a requested join type.

The new MultiTenancyRequestedJoinSpec checks, with discriminator based multi-tenancy, that an inner or a right join requested for a collection leaves out an instance whose elements all belong to another tenant, from a where query, a criteria query and count. Six of its seven cases fail on c5b0edb. DetachedCriteriaJoinTypeSpec in grails-datamapping-core checks that a later join or select replaces a recorded join type and keeps it as one that the fetch mode replaced, also in a copy of the criteria.

The new RequestedJoinBulkOperationDialectSpec checks, with the MySQL and MariaDB dialects, that updateAll and deleteAll reject an inner join requested for an association to the queried class, that the MariaDB 10.3 and H2 dialects apply it, that a join to another class is applied on MySQL, and that criteria on the association restrict the update there. Four of its nine cases fail on cc58a3b.

A second review, with each finding run against 7.0.x, added cases for: deleteAll and updateAll through a collection that has a join table and through the side of a one-to-one that the other class has the foreign key of; a sort by a collection that has a join table and multi-tenant elements, with each kind of query; a sort by a collection while the query fetches another collection of the same class, and the exception when both are bags; a fetch mode requested after a join type, with criteria on the association; a paged criteria query sorted through an alias, by a map, and by a nested path when an alias has the name of its first association; a join type in the fetch argument of list, findWhere, findAllWhere, and find and findAll with an example, with and without a sort; a collection of a basic type; the associations before a collection on a path; a right join of a collection that criteria join; order('contract.id') on a hasOne; and a projection into an embedded component of an association. The new PreQueryListenerAssociationCriteriaSpec checks criteria on an association that a listener adds. 50 of these cases fail on 1ab83ab. WhereQueryWithAssociationSortSpec also sorts players whose order differs by club name, club id, team name and player name, so that a sort through the wrong join cannot pass.

The new FetchJoinTypeSpec defines the behavior of a requested join type as a chart: for players, players.contract and club with INNER, RIGHT and LEFT, which teams the query returns, what it fetches, that a fetched collection holds all of its elements, whether a team is returned once or once for each fetched player, and the totalCount of a paged list. It checks every row with eight kinds of query: a where query with the fetch argument and with join, a dynamic finder, a criteria query with join and with the fetch argument, findAll with an example, list and a paged list. 28 of its 64 cases fail on 5e6dc8f. AssociationSortSpec has the same kind of chart for a sort by a set or a bag while the query fetches nothing, a set or a bag.

These pass with the change:

• all tests of grails-data-hibernate5-core, including the data TCK, and of grails-datamapping-core;
• codeStyle for grails-data-hibernate5-core and grails-datamapping-core.

[jdaugherty]

Verified locally on 0676c67 with :grails-data-hibernate5-core:test. The spec passes, and probes beyond it pass as well: a paged list(max: 2) with totalCount, max plus offset descending, .order('club.name', 'desc') on the detached criteria, a disjunction that uses the association (club.name == 'Arsenal' || name == '...'), a multi-key sort map mixing club.name and name, an explicit alias in the where query combined with a sort by club.name, and ignoreCase: false. The paged count works because HibernateQuery.clone() copies the criterion entries and the subcriteria but not the orders, so the count query carries no order by.

The change is right: getOrCreateAlias registers the join under both the association path and the alias, so resolving the order through criteriaAndAlias.alias covers the generated alias, the explicit alias and the association id in one place, and the detachedCriteria branch closes a latent NPE the old code had for a subquery-backed query. I also confirmed that a sort by an alias the query does not define is silently ignored (the else branch finds no such property), as the docs now say.

One thing to point out: the sort resolves a single association level only. A nested path such as team.club.name still throws, whether or not the where query joins it, while the new docs sentence reads as if any association path works. Details inline on order(). The other inline note is a merge-up heads-up for the Hibernate 7 docs on 8.0.x.

[jdaugherty]

Merge-up note: on 8.0.x the Hibernate 7 copy of this guide (grails-data-hibernate7/docs/src/docs/asciidoc/querying/whereQueries.adoc, section "Query Aliases and Sorting") still shows the owner.lastName sort as failing because of a dynamic alias, while the Hibernate 7 WhereQueryWithAssociationSortSpec there already expects it to work. The merge-up carries only this file, so that section stays stale unless it is aligned in the merge-up or a follow-up.

[matrei]

Thanks. That section was never updated for Hibernate 7. I'll align it in a separate PR against 8.0.x, after checking the Hibernate 7 behavior a bit beyond what its spec covers.

[jdaugherty]

This resolves one level only: property is split at the first dot, so a sort by a nested path such as team.club.name (Player -> Team -> Club) still throws a QueryException, whether or not the where query joins team.club. It threw before this change too, so it is not a regression, but the new docs sentence ("To sort by a property of an associated entity, use the association path") reads as if any depth works. Either extend this to nested paths, or state the one-level scope in the docs and leave nested paths to a follow-up.

[matrei]

Right, it's one level only, as before. I've stated that in the docs now and would leave nested paths for a follow-up. Supporting them means walking the path, reusing the joins that the criteria register under the full path (such as team.club), and adding inner joins for the rest, which I'd rather not do on 7.0.x.

Explicit aliases don't help with nested paths either: the where DSL doesn't recognize def c = team.club, or def t = team; def c = t.club, as an alias, so a query that uses c doesn't compile.

[jdaugherty]

I'm actually going to fix this ...

[jdaugherty]

Pushed c0c5a80 to this branch. It reverts b494caa and makes order() walk the sort path one association at a time: a join the criteria registered under the full path, such as team.club, is reused, and each association not joined yet gets an inner join, so team.club.name sorts whether or not the where query joins it, and t.club.name works through an explicit alias of team. A sort by two properties of the same association no longer raises duplicate association path either, since the sort join is registered instead of created for every order. WhereQueryWithAssociationSortSpec covers the nested and duplicate cases (all six failed before the change), and the docs sentence now says the path can cross more than one association.

[jdaugherty]

@matrei I think it's worth doing this after looking at it. Take a look at the change and see what you think.

[matrei]

Thanks, that looks good to me, and it brings Hibernate 5 in line with Hibernate 7, which already sorts by nested paths. I ran the grails-data-hibernate5-core tests on c0c5a80 locally, since CI didn't run them after I removed run ci, and they all pass.

Comparing a few queries before and after the change, the results are the same except for one: with fetch: [owner: JoinType.LEFT] on a nullable association, a sort by owner.name now keeps the rows without an owner, because the sort join uses the requested join type instead of an inner join. I think that's right, but since it changes results on 7.0.x, I'll mention it in the PR description.

[matrei]

I reviewed c0c5a80 and it looks good to me. Walking the path one association at a time and going through getOrCreateAlias reuses the joins the where query made. It also registers the sort's own join, which fixes the duplicate association path error for two sorts on the same association.

Two minor points, neither blocking:

• No test covers the LEFT fetch join now being used for the sort.
• The docs could say that a sort joins an association with an inner join unless fetch asks for another type, so a sort by owner.name leaves out pets without an owner. That was already the case before this change.

[jamesfredley]

Sorting by an association path reuses a join the where query already made and orders by that join's alias. A path through more than one association works the same way. Not approving. A sort by a nested identifier, such as team.club.id, inner-joins the last association and drops rows whose association is null. Those rows were returned before, because the identifier was ordered through the foreign key.

[jdaugherty]

Pushed 5e71443 for the feedback on c0c5a80.

@jamesfredley: a sort by a nested identifier keeps its rows again. team.club.id is sorted by the foreign key on team, as it was on 7.0.x, and club is joined only when there is no such foreign key. Details are in the thread.

@matrei: both points are in the same commit.

• WhereQueryWithAssociationSortSpec now covers the join type of a sort join: an inner join by default, which leaves out a team without a club, and a left join with fetch: [club: JoinType.LEFT], which keeps it. Team.club is nullable in the fixture for that.
• The docs say that an association is joined with an inner join unless the query requests another type, show the fetch example, and mention the nested identifier.

[jdaugherty]

Follow-up to my review above: 2154153 and 1b70845 replace the approach of 5e71443.

A sort should not change which rows a query returns, so an association that is joined only for a sort is now joined with a left join. That keeps the rows for every association on a sort path, so the foreign-key ordering for a nested identifier from 5e71443 is removed again.

@matrei: this goes beyond the two points in your review and changes results on 7.0.x, so I have updated the PR description and added section 22 to the upgrade notes.

• Where queries, detached criteria and dynamic finders (2154153): without a requested join type the sort join was an inner join, and a sort by club.name or club.id left out the teams without a club, from the list and from the totalCount of a paged list. They are kept now. A join type requested for the association, such as fetch: [club: JoinType.INNER], is used for the sort, and an association the query has criteria on is still joined by the criteria. calculateProjectionPropertyName in the same class already left-joins the associations it joins for a projection, for the same reason.
• list, find and findAll with an example, findWhere, findAllWhere and a criteria query with pagination arguments (1b70845) had the same inner join in GrailsHibernateQueryUtils and GrailsHibernateUtil. Team.list(sort: 'club.name') left out the teams without a club, and findAllWhere with that sort did not find such a team at all. They use a left join now too.
• The where query docs say that sorting does not change which rows a query returns. WhereQueryWithAssociationSortSpec and the new AssociationSortSpec cover it, and nine cases of each fail on c0c5a80.

@jamesfredley: the details for team.club.id are in the thread.

[jdaugherty]

@jamesfredley @matrei @borinquenkid, a question that came up while testing this PR and that the PR does not change: what should the fetch argument of the static list() do when the association is null?

With four teams, one of which has no club (Team.club is nullable):

Query	7.0.x (Hibernate 5)	8.0.x (Hibernate 7)
Team.list(fetch: [club: 'join'])	3 teams, club not fetched	3 teams, club fetched
Team.where { ... }.list(fetch: [club: 'join'])	4 teams, fetched	4 teams, fetched
Team.findAllByNameLike('%First%', [fetch: [club: 'join']])	4 teams, fetched	4 teams, fetched
Team.createCriteria().list(fetch: [club: 'join'], max: 10) { ... }	4 teams, fetched	4 teams, not fetched
Team.findAllWhere([name: 'MU First Team'], [fetch: [club: 'join']])	found, fetched	found, not fetched

"Fetched" is Hibernate.isInitialized(team.club) after clearing the session. fetch: [club: 'eager'] behaves like 'join' in list().

So in list() the argument leaves out the team without a club, on both versions:

• On 7.0.x, GrailsHibernateQueryUtils.populateArgumentsForCriteria turns it into queryRoot.join('club'). That is an inner join and not a fetch, so it only filters.
• On 8.0.x, HqlListQueryBuilder appends join fetch e.club, an inner fetch join.

It also means that Team.list(sort: 'club.name', fetch: [club: 'join']) still leaves the team out after this PR, because of fetch and not because of the sort.

The options I see:

1. A left fetch join in list(), which is what the where queries and dynamic finders do. It changes which rows list() returns, like the sort change in this PR, and on 7.0.x list() starts to fetch the association.
2. A left join without the fetch on 7.0.x, so that the argument no longer filters and still does not fetch.
3. Leave it as it is and document it.

What do you think should happen, and on which branch? The two rows where Hibernate 7 does not fetch look like a separate gap there.

[jamesfredley]

A sort by a nested identifier such as team.club.id now left-joins that association and keeps the rows whose association is null. A paged criteria count can sort by a nested path, and each entry of a sort map is kept. Not approving. A sort stored on the query is applied before the list arguments, so an inner join requested there does not replace the left join the sort already created.

[jdaugherty]

One more commit since the review above. The head is 1ab83ab.

updateAll and deleteAll apply a requested join type with a subquery (cc58a3b). For a path that leads back to the queried class, such as the parent of a category, that subquery selects from the table that the update or the delete modifies. MySQL, and MariaDB before version 10.3, do not allow that, so the statement failed there with an error of the database. With 1ab83ab, updateAll and deleteAll throw an UnsupportedOperationException on these databases. It names the association and says to restrict the query with criteria instead, as in Category.where { parent != null }. Other databases run the statement.

RequestedJoinBulkOperationDialectSpec covers this with the MySQL and MariaDB dialects, and four of its nine cases fail on cc58a3b. The fetching guide and section 23 of the upgrade notes describe it.

[jamesfredley]

Not approved. A listener that adds an association restriction to findAll is ignored, and the query fails to resolve the alias. Sorting an example query by an association still returns a row whose association is null when an inner join was requested. A criteria list that asks to fetch a collection it sorts by now fetches it.

[jdaugherty]

Pushed two commits for the review of 1ab83ab. The head is 0c81f00.

@jamesfredley: both points are fixed in 688440e, and the details are in the threads. A listener's criteria on an association now get their joins after the PreQueryEvent, and find and findAll with an example, findWhere, findAllWhere and list apply a JoinType in their fetch argument, before a sort by the association, which uses that join.

A second review of the branch, with each finding run against 7.0.x, found seven more regressions. The same commit fixes them:

• deleteAll with an inner join of a collection that has a join table deleted no instance and removed the join rows of the instances it should have deleted. Hibernate deletes the join rows first, with the same restriction. The identifiers are now selected first.
• updateAll and deleteAll matched the side of a one-to-one that the other class has the foreign key of, as Nose.face with static belongsTo = [face: Face], by comparing the two primary keys, and changed other instances than list returns.
• A sort by a collection that has a join table and multi-tenant elements produced SQL that no database accepts: Hibernate combines the condition that keeps the collection from being fetched with the tenant filter of the join table. The condition is now part of the where clause.
• A sort by players.name in a query that fetched reserves, a collection of the same class, filled reserves with the players. The sort now fetches its collection as well when the query fetches another one.
• join('club', JoinType.LEFT) followed by fetch: [club: 'join'] lost the left join for the criteria on club, as on 7.0.x it did not.
• A paged criteria query sorted through an alias, as sort: 't.club.name', lost its sort.
• A nested sort path was resolved through an alias of the criteria that has the name of the first association on the path.

0c81f00 fixes older issues that the review found in what this branch changes or documents: order('contract.id') on a hasOne inner-joined, a paged criteria query with a map of sort properties failed, a projection into an embedded component of an association failed, a right join of a collection that criteria join returned null rows, a join type for a collection of a basic type was ignored, and the associations before a collection on a requested path were not fetched. It also corrects two statements in the docs.

50 of the new cases fail on 1ab83ab. The whole test suites of grails-data-hibernate5-core and grails-datamapping-core pass, and so does codeStyle for both.

@matrei: both commits change results on 7.0.x in a few more places, and sections 22 and 23 of the upgrade notes describe each of them. grails-datamapping-core gains getSupersededJoinTypes() on the detached criteria, so that a join type that a later fetch mode replaced still reaches the query. The PR description is updated.

[jamesfredley]

Not approved. Sorting by an empty bag drops the root row when another bag is fetched. A list that asks to fetch a collection with an inner join returns the row without initializing that collection.

[jdaugherty]

Pushed two commits for the review of 0c81f00. The head is 1313011.

@jamesfredley:

• 5e6dc8f: a sort by a bag in a query that fetches another bag now throws an UnsupportedOperationException that names both collections, instead of leaving out the squad. Keeping the squad would need the sort to fetch regulars as well, which Hibernate does not do for two bags in one query, and a join that does not fetch it lets Hibernate fill substitutes with the regulars. A sort by a bag while no other bag is fetched keeps a squad whose bag is empty.
• 1313011: list fetches the association that its fetch argument requests a join type for, with that join type, so Team.list(fetch: [players: JoinType.INNER]) returns the teams that have players and initializes players in the same query. The commit also settles what a join type means for a path through a collection: it decides which instances the query returns, and the collection on the path is fetched with all of its elements. fetch: ['players.contract': JoinType.INNER] returns the teams with a player who has a contract, each with all of its players. A fetch that left out the players without a contract would leave that partial collection in the session. This applies to every kind of query.

The new FetchJoinTypeSpec checks this as a chart of fetch arguments, the teams that they return and what they fetch, for eight kinds of query; 28 of its 64 cases fail on 5e6dc8f. AssociationSortSpec has the same kind of chart for a sort by a set or a bag while the query fetches nothing, a set or a bag. The whole test suite of grails-data-hibernate5-core passes, and so does codeStyle.

@matrei: both commits change results on 7.0.x. Sections 22 and 23 of the upgrade notes describe them, the fetching and where query guides are updated, and so is the PR description.

[jamesfredley]

A list that asks to fetch a collection with an inner join now initializes it. Not approving. Fetching one bag while sorting by another bag throws, and the page total counts the joined rows instead of the distinct roots.

[jamesfredley]

Input: an author with one book and one award, listed with a fetch of awards and a sort by books.title, where both collections are bags. The query throws even though only one bag was asked to be fetched. Keep the author row and the requested fetch, and leave the sorted bag as a non-fetching join.

[jdaugherty]

The exception stays, for a reason that one book and one award cannot show. A join that does not fetch books still returns a row for each book, and awards is fetched from those rows. A bag cannot tell a repeated row from a repeated element, so an author with two books and two awards gets awards as [A3, A3, A4, A4]. 7.0.x does this in a where query and a dynamic finder, and this branch did it in list, the one kind of query without the check. Hibernate's PersistentBag.readFrom says the same ("if we load this collection from a cartesian product the multiplicity would be broken"), and it is what MultipleBagFetchException guards against.

0c312b0 adds the check to list. The message and the docs now give this reason, and say to declare both collections as a Set or a List, or not to fetch the other one. 8c4df83 applies it when either collection is a bag, since one bag is enough: with books as a Set and awards still a bag, the same author gets four awards as well. A sort by an association to a single instance, and a query that fetches the collection that it sorts by, are not rejected. AssociationSortSpec covers list, a paged list and a join type in the fetch argument, and has the combinations as a chart with two elements in each collection, since one element cannot show a repeated row.

[borinquenkid]

As discussed, I pushed b4e50b4 on top of 1313011: a sort by an alias the query does not define now throws IllegalArgumentException: Invalid sort property on Hibernate 5, as it does on Hibernate 7 (#16858), instead of being ignored. It covers applyOrder and the two addOrderPossiblyNested overloads, a property that exists but is not an association is still ignored, and the where-query guide sentence is changed. UndefinedAliasSortSpec has the cases; the sort specs of this PR pass with it.

[borinquenkid]

@jdaugherty on the fetch of the static list() with an association that can be null: I would say every instance, with the association fetched where there is one. A where query, a dynamic finder and a criteria query already return all four teams on both versions, so list() was the odd one out. A fetch mode should never filter; JoinType.INNER stays the explicit way to ask for that. Hibernate 5 is fixed on this PR in 1e47513, and Hibernate 7 in #16898. FetchJoinTypeSpec and ListFetchJoinNullSpec cover a nullable association and an empty collection for list, a paged list, a where query and a dynamic finder. Tell me if you meant something else.

[jamesfredley]

Not approving. A paged list that names its root alias throws when the total is read, and fetching one collection while filtering another collection of the same type fills the fetched collection with the wrong rows.

[jdaugherty]

Pushed six commits for the reviews of 1313011 and 1e47513. The head is 8c4df83.

@jamesfredley:

• 9f2c781: the count of a paged list keeps the root alias of the query. See the thread on HibernateQuery.
• f7ee8a3: a filtered collection no longer fills a collection that the query fetches. See the thread on AbstractHibernateQuery.
• 0c312b0 and 8c4df83: the exception stays and gives its reason. It now also applies to list, and to a sort by one collection next to a fetch of another when either of them is a bag. See the thread on GrailsHibernateUtil.
• 8df78b0, for the page total in the review of 1313011: a total now counts what the list pages through. A list that fetches a collection returns each instance once, so its total counts instances, also when a sort joins another collection. A list that only sorts by a collection returns a row for each element, and max and offset page through those rows, so its total keeps counting them: two squads with three players have 3 rows, and a total of 2 would put the last row out of reach. That is the same for list, a where query and a criteria query, and 7.0.x counts those rows too.

@borinquenkid: deb2784 adds b4e50b4 and 1e47513 to sections 22 and 23 of the upgrade notes.

AssociationSortSpec and AssociationJoinTypeSpec cover the changes, and 21 of their cases fail on 1e47513. The whole test suite of grails-data-hibernate5-core passes, and so does codeStyle.

[jdaugherty]

The exception stays, for a reason that one book and one award cannot show. A join that does not fetch books still returns a row for each book, and awards is fetched from those rows. A bag cannot tell a repeated row from a repeated element, so an author with two books and two awards gets awards as [A3, A3, A4, A4]. 7.0.x does this in a where query and a dynamic finder, and this branch did it in list, the one kind of query without the check. Hibernate's PersistentBag.readFrom says the same ("if we load this collection from a cartesian product the multiplicity would be broken"), and it is what MultipleBagFetchException guards against.

0c312b0 adds the check to list. The message and the docs now give this reason, and say to declare both collections as a Set or a List, or not to fetch the other one. 8c4df83 applies it when either collection is a bag, since one bag is enough: with books as a Set and awards still a bag, the same author gets four awards as well. A sort by an association to a single instance, and a query that fetches the collection that it sorts by, are not rejected. AssociationSortSpec covers list, a paged list and a join type in the fetch argument, and has the combinations as a chart with two elements in each collection, since one element cannot show a repeated row.

[testlens-app]

🚨 All tests passed but jobs failed 🚨

Failed Jobs without Test Failures

❌ CI / Spring Security Config Functional Tests (annotation)
❌ CI / Spring Security Config Functional Tests (misc)
❌ CI / Spring Security Config Functional Tests (putWithParams)

🏷️ Commit: 8c4df83
▶️ Tests: 49138 executed
🟡 Checks: 45/52 completed

Rerun Controls

Click the checkbox to trigger a rerun:

🔲 Rerun jobs

---

Learn more about TestLens at testlens.app/docs.