From c08b1b9bf1a77c558bd4213abfc6513eaecaa0fa Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Fri, 31 May 2024 09:39:41 +0200 Subject: [PATCH] HHH-18202 Fix group/order by fk rendering handling nested paths Introduced generalized `MetadataKey`-based resolutions with caching in `BaseSqmToSqlAstConverter` --- .../query/sqm/internal/SqmPathVisitor.java | 89 +++++++++++++++++++ .../hibernate/query/sqm/internal/SqmUtil.java | 35 +++++++- .../sqm/sql/BaseSqmToSqlAstConverter.java | 37 ++++++++ .../sqm/sql/FakeSqmToSqlAstConverter.java | 1 + .../query/sqm/sql/SqmToSqlAstConverter.java | 9 ++ .../EntityValuedPathInterpretation.java | 2 +- .../query/sqm/tree/select/SqmQuerySpec.java | 33 ++++++- 7 files changed, 200 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmPathVisitor.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmPathVisitor.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmPathVisitor.java new file mode 100644 index 0000000000..c7a8165eef --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmPathVisitor.java @@ -0,0 +1,89 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.query.sqm.internal; + +import java.util.function.Consumer; + +import org.hibernate.metamodel.model.domain.DiscriminatorSqmPath; +import org.hibernate.query.sqm.spi.BaseSemanticQueryWalker; +import org.hibernate.query.sqm.tree.domain.NonAggregatedCompositeSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmAnyValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmEntityValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmPath; +import org.hibernate.query.sqm.tree.domain.SqmPluralValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; +import org.hibernate.query.sqm.tree.from.SqmAttributeJoin; + +/** + * Generic {@link org.hibernate.query.sqm.SemanticQueryWalker} that applies the provided + * {@link Consumer} to all {@link SqmPath paths} encountered during visitation. + * + * @author Marco Belladelli + */ +public class SqmPathVisitor extends BaseSemanticQueryWalker { + private final Consumer> pathConsumer; + + public SqmPathVisitor(Consumer> pathConsumer) { + this.pathConsumer = pathConsumer; + } + + @Override + public Object visitBasicValuedPath(SqmBasicValuedSimplePath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitEmbeddableValuedPath(SqmEmbeddedValuedSimplePath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitEntityValuedPath(SqmEntityValuedSimplePath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitAnyValuedValuedPath(SqmAnyValuedSimplePath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitQualifiedAttributeJoin(SqmAttributeJoin path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitTreatedPath(SqmTreatedPath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitDiscriminatorPath(DiscriminatorSqmPath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitPluralValuedPath(SqmPluralValuedSimplePath path) { + pathConsumer.accept( path ); + return path; + } + + @Override + public Object visitNonAggregatedCompositeValuedPath(NonAggregatedCompositeSimplePath path) { + pathConsumer.accept( path ); + return path; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java index 1a49a89c80..58ff6b089e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java @@ -54,6 +54,7 @@ import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter; import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef; +import org.hibernate.query.sqm.tree.expression.SqmExpression; import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper; import org.hibernate.query.sqm.tree.expression.SqmParameter; import org.hibernate.query.sqm.tree.expression.SqmTuple; @@ -62,7 +63,9 @@ import org.hibernate.query.sqm.tree.from.SqmFrom; import org.hibernate.query.sqm.tree.from.SqmJoin; import org.hibernate.query.sqm.tree.from.SqmQualifiedJoin; import org.hibernate.query.sqm.tree.from.SqmRoot; +import org.hibernate.query.sqm.tree.select.SqmOrderByClause; import org.hibernate.query.sqm.tree.select.SqmQueryPart; +import org.hibernate.query.sqm.tree.select.SqmQuerySpec; import org.hibernate.query.sqm.tree.select.SqmSelectStatement; import org.hibernate.query.sqm.tree.select.SqmSelectableNode; import org.hibernate.query.sqm.tree.select.SqmSelection; @@ -167,7 +170,8 @@ public class SqmUtil { if ( association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() ) && ( clause == Clause.GROUP || clause == Clause.ORDER || !isFkOptimizationAllowed( sqmPath.getLhs() ) - || queryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) ) { + || queryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState ) + || queryPart.getFirstQuerySpec().orderByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState ) ) ) { return association.getAssociatedEntityMappingType(); } } @@ -210,6 +214,35 @@ public class SqmUtil { return false; } + public static List getGroupByNavigablePaths(SqmQuerySpec querySpec) { + final List> expressions = querySpec.getGroupByClauseExpressions(); + if ( expressions.isEmpty() ) { + return Collections.emptyList(); + } + + final List navigablePaths = new ArrayList<>( expressions.size() ); + final SqmPathVisitor pathVisitor = new SqmPathVisitor( path -> navigablePaths.add( path.getNavigablePath() ) ); + for ( SqmExpression expression : expressions ) { + expression.accept( pathVisitor ); + } + return navigablePaths; + } + + public static List getOrderByNavigablePaths(SqmQuerySpec querySpec) { + final SqmOrderByClause order = querySpec.getOrderByClause(); + if ( order == null || order.getSortSpecifications().isEmpty() ) { + return Collections.emptyList(); + } + + final List sortSpecifications = order.getSortSpecifications(); + final List navigablePaths = new ArrayList<>( sortSpecifications.size() ); + final SqmPathVisitor pathVisitor = new SqmPathVisitor( path -> navigablePaths.add( path.getNavigablePath() ) ); + for ( SqmSortSpecification sortSpec : sortSpecifications ) { + sortSpec.getSortExpression().accept( pathVisitor ); + } + return navigablePaths; + } + public static SqmAttributeJoin findCompatibleFetchJoin( SqmFrom sqmFrom, SqmPathSource pathSource, 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 c153860bdd..7f75fbc63e 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 @@ -524,6 +524,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base private boolean negativeAdjustment; private final Set visitedAssociationKeys = new HashSet<>(); + private final HashMap, Object> metadata = new HashMap<>(); private final MappingMetamodel domainModel; public BaseSqmToSqlAstConverter( @@ -8614,6 +8615,42 @@ public abstract class BaseSqmToSqlAstConverter extends Base orderByFragments.add( new AbstractMap.SimpleEntry<>( orderByFragment, tableGroup ) ); } + @Override + public M resolveMetadata(S source, Function producer ) { + //noinspection unchecked + return (M) metadata.computeIfAbsent( new MetadataKey<>( source, producer ), k -> producer.apply( source ) ); + } + + static class MetadataKey { + private final S source; + private final Function producer; + + public MetadataKey(S source, Function producer) { + this.source = source; + this.producer = producer; + } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + if ( o == null || getClass() != o.getClass() ) { + return false; + } + + final MetadataKey that = (MetadataKey) o; + return source.equals( that.source ) && producer.equals( that.producer ); + } + + @Override + public int hashCode() { + int result = source.hashCode(); + result = 31 * result + producer.hashCode(); + return result; + } + } + @Override public boolean isResolvingCircularFetch() { return resolvingCircularFetch; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java index ada844cddc..f30c26ad07 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java @@ -7,6 +7,7 @@ package org.hibernate.query.sqm.sql; import java.util.List; +import java.util.function.Function; import java.util.function.Supplier; import org.hibernate.LockMode; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java index 72f6da54b8..f5fc0caea3 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java @@ -7,6 +7,7 @@ package org.hibernate.query.sqm.sql; import java.util.List; +import java.util.function.Function; import java.util.function.Supplier; import org.hibernate.internal.util.collections.Stack; @@ -23,6 +24,8 @@ import org.hibernate.sql.ast.tree.expression.Expression; import org.hibernate.sql.ast.tree.expression.QueryTransformer; import org.hibernate.sql.ast.tree.predicate.Predicate; +import jakarta.annotation.Nullable; + /** * Specialized SemanticQueryWalker (SQM visitor) for producing SQL AST. * @@ -58,4 +61,10 @@ public interface SqmToSqlAstConverter extends SemanticQueryWalker, SqlAs Predicate visitNestedTopLevelPredicate(SqmPredicate predicate); + /** + * Resolve a generic metadata object from the provided source, using the specified producer. + */ + default M resolveMetadata(S source, Function producer) { + return producer.apply( source ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java index d6b4b900b9..0eb0826f87 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java @@ -267,7 +267,7 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta if ( currentClause == Clause.GROUP || currentClause == Clause.ORDER ) { assert sqlAstCreationState.getCurrentSqmQueryPart().isSimpleQueryPart(); final SqmQuerySpec querySpec = sqlAstCreationState.getCurrentSqmQueryPart().getFirstQuerySpec(); - if ( currentClause == Clause.ORDER && !querySpec.groupByClauseContains( navigablePath ) ) { + if ( currentClause == Clause.ORDER && !querySpec.groupByClauseContains( navigablePath, sqlAstCreationState ) ) { // We must ensure that the order by expression be expanded but only if the group by // contained the same expression, and that was expanded as well expandToAllColumns = false; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmQuerySpec.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmQuerySpec.java index 6a8044bb28..35d6a47f70 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmQuerySpec.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmQuerySpec.java @@ -12,6 +12,7 @@ import java.util.HashSet; import java.util.List; import java.util.Set; +import org.hibernate.Internal; import org.hibernate.internal.util.collections.CollectionHelper; import org.hibernate.metamodel.mapping.CollectionPart; import org.hibernate.metamodel.model.domain.EmbeddableDomainType; @@ -25,11 +26,12 @@ import org.hibernate.query.criteria.JpaSelection; import org.hibernate.query.sqm.FetchClauseType; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SemanticQueryWalker; +import org.hibernate.query.sqm.internal.SqmUtil; +import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.SqmCopyContext; import org.hibernate.query.sqm.tree.SqmNode; import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmEntityValuedSimplePath; -import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef; import org.hibernate.query.sqm.tree.expression.SqmExpression; import org.hibernate.query.sqm.tree.from.SqmAttributeJoin; @@ -627,9 +629,32 @@ public class SqmQuerySpec extends SqmQueryPart super.appendHqlString( sb ); } - public boolean groupByClauseContains(NavigablePath path) { - for ( final SqmExpression expression : groupByClauseExpressions ) { - if ( expression instanceof SqmPath && ( (SqmPath) expression ).getNavigablePath().isParentOrEqual( path ) ) { + @Internal + public boolean groupByClauseContains(NavigablePath navigablePath, SqmToSqlAstConverter sqlAstConverter) { + if ( groupByClauseExpressions.isEmpty() ) { + return false; + } + return navigablePathsContain( sqlAstConverter.resolveMetadata( + this, + SqmUtil::getGroupByNavigablePaths + ), navigablePath ); + } + + @Internal + public boolean orderByClauseContains(NavigablePath navigablePath, SqmToSqlAstConverter sqlAstConverter) { + final SqmOrderByClause orderByClause = getOrderByClause(); + if ( orderByClause == null || orderByClause.getSortSpecifications().isEmpty() ) { + return false; + } + return navigablePathsContain( sqlAstConverter.resolveMetadata( + this, + SqmUtil::getOrderByNavigablePaths + ), navigablePath ); + } + + private boolean navigablePathsContain(List navigablePaths, NavigablePath navigablePath) { + for ( NavigablePath path : navigablePaths ) { + if ( path.isParentOrEqual( navigablePath ) ) { return true; } }