HHH-18436 Apply plural attribute ordering when creating the join

Author: mbellade

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java

@@ -962,19 +962,6 @@ else if ( fetchDepth > maximumFetchDepth + 1 ) {
 						creationState
 				);
 
-				if ( fetch.getTiming() == FetchTiming.IMMEDIATE && joined ) {
-					if ( isFetchablePluralAttributeMapping ) {
-						final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable;
-						final QuerySpec querySpec = creationState.getInflightQueryPart().getFirstQuerySpec();
-						applyOrdering(
-								querySpec,
-								fetchablePath,
-								pluralAttributeMapping,
-								creationState
-						);
-					}
-				}
-
 				fetches.add( fetch );
 			}
 			finally {
@@ -1009,19 +996,6 @@ else if ( fetchable instanceof PluralAttributeMapping ) {
 		return true;
 	}
 
-	private void applyOrdering(
-			QuerySpec querySpec,
-			NavigablePath navigablePath,
-			PluralAttributeMapping pluralAttributeMapping,
-			LoaderSqlAstCreationState sqlAstCreationState) {
-		assert pluralAttributeMapping.getAttributeName().equals( navigablePath.getLocalName() );
-
-		final TableGroup tableGroup = sqlAstCreationState.getFromClauseAccess().getTableGroup( navigablePath );
-		assert tableGroup != null;
-
-		applyOrdering( querySpec, tableGroup, pluralAttributeMapping, sqlAstCreationState );
-	}
-
 	private SelectStatement generateSelect(SubselectFetch subselect) {
 
 		// todo (6.0) : we could even convert this to a join by piecing together

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java

@@ -24,6 +24,7 @@
 import org.hibernate.metamodel.mapping.AssociationKey;
 import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
 import org.hibernate.metamodel.mapping.ModelPart;
+import org.hibernate.metamodel.mapping.ordering.OrderByFragment;
 import org.hibernate.query.spi.Limit;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
 import org.hibernate.spi.NavigablePath;
@@ -41,8 +42,10 @@
 import org.hibernate.sql.ast.spi.SqlAstQueryPartProcessingState;
 import org.hibernate.sql.ast.spi.SqlExpressionResolver;
 import org.hibernate.sql.ast.tree.from.FromClause;
+import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.predicate.Predicate;
 import org.hibernate.sql.ast.tree.select.QueryPart;
+import org.hibernate.sql.ast.tree.select.QuerySpec;
 import org.hibernate.sql.results.graph.DomainResultCreationState;
 import org.hibernate.sql.results.graph.FetchParent;
 import org.hibernate.sql.results.graph.internal.ImmutableFetchList;
@@ -94,6 +97,13 @@ public LoaderSqlAstCreationState(
 		);
 	}
 
+	@Override
+	public void applyOrdering(TableGroup tableGroup, OrderByFragment orderByFragment) {
+		final QuerySpec querySpec = getInflightQueryPart().getFirstQuerySpec();
+		assert querySpec.isRoot() : "Illegal attempt to apply order-by fragment to a non-root query spec";
+		orderByFragment.apply( querySpec, tableGroup, this );
+	}
+
 	@Override
 	public SqlAstCreationContext getCreationContext() {
 		return sf;

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java

@@ -737,6 +737,16 @@ public TableGroupJoin createTableGroupJoin(
 				creationState
 		);
 
+		if ( fetched ) {
+			if ( orderByFragment != null ) {
+				creationState.applyOrdering( tableGroup, orderByFragment );
+			}
+
+			if ( manyToManyOrderByFragment != null ) {
+				creationState.applyOrdering( tableGroup, manyToManyOrderByFragment );
+			}
+		}
+
 		final TableGroupJoin tableGroupJoin = new TableGroupJoin(
 				navigablePath,
 				determineSqlJoinType( lhs, requestedJoinType, fetched ),

hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java

@@ -8555,14 +8555,6 @@ public <R> R withNestedFetchParent(FetchParent fetchParent, Function<FetchParent
 
 	@Override
 	public ImmutableFetchList visitFetches(FetchParent fetchParent) {
-		if ( fetchParent instanceof EagerCollectionFetch && currentQuerySpec().isRoot() ) {
-			final EagerCollectionFetch collectionFetch = (EagerCollectionFetch) fetchParent;
-			final PluralAttributeMapping pluralAttributeMapping = collectionFetch.getFetchedMapping();
-			final NavigablePath fetchablePath = collectionFetch.getNavigablePath();
-			final TableGroup tableGroup = getFromClauseIndex().getTableGroup( fetchablePath );
-			assert tableGroup.getModelPart() == pluralAttributeMapping;
-			applyOrdering( tableGroup, pluralAttributeMapping );
-		}
 		final FetchableContainer referencedMappingContainer = fetchParent.getReferencedMappingContainer();
 		final int keySize = referencedMappingContainer.getNumberOfKeyFetchables();
 		final int size = referencedMappingContainer.getNumberOfFetchables();
@@ -8649,11 +8641,14 @@ private void applyOrdering(TableGroup tableGroup, PluralAttributeMapping pluralA
 		}
 	}
 
-	private void applyOrdering(TableGroup tableGroup, OrderByFragment orderByFragment) {
-		if ( orderByFragments == null ) {
-			orderByFragments = new ArrayList<>();
+	@Override
+	public void applyOrdering(TableGroup tableGroup, OrderByFragment orderByFragment) {
+		if ( currentQuerySpec().isRoot() ) {
+			if ( orderByFragments == null ) {
+				orderByFragments = new ArrayList<>();
+			}
+			orderByFragments.add( new AbstractMap.SimpleEntry<>( orderByFragment, tableGroup ) );
 		}
-		orderByFragments.add( new AbstractMap.SimpleEntry<>( orderByFragment, tableGroup ) );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlAstCreationState.java

@@ -9,6 +9,7 @@
 import org.hibernate.Internal;
 import org.hibernate.LockMode;
 import org.hibernate.engine.spi.LoadQueryInfluencers;
+import org.hibernate.metamodel.mapping.ordering.OrderByFragment;
 import org.hibernate.persister.entity.EntityNameUse;
 import org.hibernate.sql.ast.tree.from.TableGroup;
 
@@ -49,4 +50,8 @@ default void registerEntityNameUsage(
 	default boolean supportsEntityNameUsage() {
 		return false;
 	}
+
+	@Internal
+	default void applyOrdering(TableGroup tableGroup, OrderByFragment orderByFragment) {
+	}
 }