From 93e74362d5a9930daadc6cf876c8762674a73e7d Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Tue, 6 Aug 2024 13:45:55 +0200 Subject: [PATCH] HHH-18436 Apply plural attribute ordering when creating the join --- .../ast/internal/LoaderSelectBuilder.java | 26 ------------------- .../internal/LoaderSqlAstCreationState.java | 10 +++++++ .../internal/PluralAttributeMappingImpl.java | 10 +++++++ .../sqm/sql/BaseSqmToSqlAstConverter.java | 19 +++++--------- .../sql/ast/spi/SqlAstCreationState.java | 5 ++++ 5 files changed, 32 insertions(+), 38 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index 0899b4b3c3..e25ec9a52b 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -962,19 +962,6 @@ public class LoaderSelectBuilder { 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 @@ public class LoaderSelectBuilder { 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 diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java index ae03685bcf..f7377d4dcd 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java @@ -24,6 +24,7 @@ import org.hibernate.graph.spi.AppliedGraph; 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.SqlAstProcessingState; 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 class 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; diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java index 62d3b3313a..5b8958589e 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java @@ -737,6 +737,16 @@ public class PluralAttributeMappingImpl 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 ), diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index 950935888f..b68b2fc88c 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -8555,14 +8555,6 @@ public abstract class BaseSqmToSqlAstConverter extends Base @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 @@ public abstract class BaseSqmToSqlAstConverter extends Base } } - 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 diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlAstCreationState.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlAstCreationState.java index 8d3bfe0d23..4ae6d801e2 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlAstCreationState.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/SqlAstCreationState.java @@ -9,6 +9,7 @@ package org.hibernate.sql.ast.spi; 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 @@ public interface SqlAstCreationState { default boolean supportsEntityNameUsage() { return false; } + + @Internal + default void applyOrdering(TableGroup tableGroup, OrderByFragment orderByFragment) { + } }