From b115ab7f42c5c54cd6d1a140eddc6e649a0ed14b Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 25 Sep 2023 10:53:22 +0200 Subject: [PATCH] HHH-17225 Always use target table column for right / full joins --- .../hibernate/query/sqm/internal/SqmUtil.java | 32 +++++++++++++++++++ .../BasicValuedPathInterpretation.java | 19 ++++------- .../EmbeddableValuedPathInterpretation.java | 27 ++++++---------- 3 files changed, 48 insertions(+), 30 deletions(-) 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 d64eb73eb8..98c0f85f35 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 @@ -29,7 +29,9 @@ import org.hibernate.metamodel.mapping.EntityIdentifierMapping; import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.ForeignKeyDescriptor; import org.hibernate.metamodel.mapping.JdbcMapping; +import org.hibernate.metamodel.mapping.ManagedMappingType; import org.hibernate.metamodel.mapping.MappingModelExpressible; +import org.hibernate.metamodel.mapping.ModelPartContainer; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.query.IllegalQueryOperationException; import org.hibernate.query.IllegalSelectQueryException; @@ -42,7 +44,9 @@ import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SqmQuerySource; import org.hibernate.query.sqm.spi.JdbcParameterBySqmParameterAccess; import org.hibernate.query.sqm.spi.SqmParameterMappingModelResolutionAccess; +import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.SqmDmlStatement; +import org.hibernate.query.sqm.tree.SqmJoinType; import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter; @@ -50,11 +54,14 @@ import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef; import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper; import org.hibernate.query.sqm.tree.expression.SqmParameter; import org.hibernate.query.sqm.tree.from.SqmFrom; +import org.hibernate.query.sqm.tree.from.SqmJoin; import org.hibernate.query.sqm.tree.from.SqmRoot; +import org.hibernate.query.sqm.tree.select.SqmQueryPart; import org.hibernate.query.sqm.tree.select.SqmSelectStatement; import org.hibernate.query.sqm.tree.select.SqmSelectableNode; import org.hibernate.query.sqm.tree.select.SqmSortSpecification; import org.hibernate.spi.NavigablePath; +import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlTreeCreationException; import org.hibernate.sql.ast.tree.expression.JdbcParameter; import org.hibernate.sql.ast.tree.from.TableGroup; @@ -120,6 +127,31 @@ public class SqmUtil { ); } + public static boolean needsTargetTableMapping( + SqmPath sqmPath, + ModelPartContainer modelPartContainer, + SqmToSqlAstConverter sqlAstCreationState) { + final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); + return ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING ) + && modelPartContainer.getPartMappingType() != modelPartContainer + && sqmPath.getLhs() instanceof SqmFrom + && modelPartContainer.getPartMappingType() instanceof ManagedMappingType + && ( groupByClauseContains( sqlAstCreationState.getCurrentSqmQueryPart(), sqmPath.getNavigablePath() ) + || isNonOptimizableJoin( sqmPath.getLhs() ) ); + } + + private static boolean groupByClauseContains(SqmQueryPart sqmQueryPart, NavigablePath path) { + return sqmQueryPart.isSimpleQueryPart() && sqmQueryPart.getFirstQuerySpec().groupByClauseContains( path ); + } + + private static boolean isNonOptimizableJoin(SqmPath sqmPath) { + if ( sqmPath instanceof SqmJoin ) { + final SqmJoinType sqmJoinType = ( (SqmJoin) sqmPath ).getSqmJoinType(); + return sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT; + } + return false; + } + public static Map, Map, List>> generateJdbcParamsXref( DomainParameterXref domainParameterXref, JdbcParameterBySqmParameterAccess jdbcParameterBySqmParameterAccess) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/BasicValuedPathInterpretation.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/BasicValuedPathInterpretation.java index 917a5ef78d..47e9cd81b5 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/BasicValuedPathInterpretation.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/BasicValuedPathInterpretation.java @@ -25,10 +25,7 @@ import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; -import org.hibernate.query.sqm.tree.from.SqmFrom; -import org.hibernate.query.sqm.tree.select.SqmQueryPart; import org.hibernate.spi.NavigablePath; -import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.Expression; @@ -37,6 +34,8 @@ import org.hibernate.sql.ast.tree.from.TableGroup; import org.hibernate.sql.ast.tree.from.TableReference; import org.hibernate.sql.ast.tree.update.Assignable; +import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping; + /** * @author Steve Ebersole */ @@ -83,16 +82,10 @@ public class BasicValuedPathInterpretation extends AbstractSqmPathInterpretat } final BasicValuedModelPart mapping; - // In the select, group by, order by and having clause we have to make sure we render the column of the target table, - // never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined - // and if this basic path is part of the group by clause - final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); - final SqmQueryPart sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart(); - if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING ) - && lhs instanceof SqmFrom - && modelPartContainer.getPartMappingType() instanceof ManagedMappingType - && sqmQueryPart.isSimpleQueryPart() - && sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) { + if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) { + // In the select, group by, order by and having clause we have to make sure we render + // the column of the target table, never the FK column, if the lhs is a join type that + // requires it (right, full) or if this path is contained in group by clause mapping = (BasicValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedPathInterpretation.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedPathInterpretation.java index 69a6ac302b..18f5bbc6d0 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedPathInterpretation.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EmbeddableValuedPathInterpretation.java @@ -20,10 +20,7 @@ import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; -import org.hibernate.query.sqm.tree.from.SqmFrom; -import org.hibernate.query.sqm.tree.select.SqmQueryPart; import org.hibernate.spi.NavigablePath; -import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.Expression; @@ -32,6 +29,8 @@ import org.hibernate.sql.ast.tree.expression.SqlTupleContainer; import org.hibernate.sql.ast.tree.from.TableGroup; import org.hibernate.sql.ast.tree.update.Assignable; +import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping; + /** * @author Steve Ebersole */ @@ -65,25 +64,19 @@ public class EmbeddableValuedPathInterpretation extends AbstractSqmPathInterp } } - final ModelPartContainer modelPart = tableGroup.getModelPart(); + final ModelPartContainer modelPartContainer = tableGroup.getModelPart(); final EmbeddableValuedModelPart mapping; - // In the select, group by, order by and having clause we have to make sure we render the column of the target table, - // never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined - // and if this basic path is part of the group by clause - final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); - final SqmQueryPart sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart(); - if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING ) - && lhs instanceof SqmFrom - && modelPart.getPartMappingType() instanceof ManagedMappingType - && sqmQueryPart.isSimpleQueryPart() - && sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) { - mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart( + if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) { + // In the select, group by, order by and having clause we have to make sure we render + // the column of the target table, never the FK column, if the lhs is a join type that + // requires it (right, full) or if this path is contained in group by clause + mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget ); } else { - mapping = (EmbeddableValuedModelPart) modelPart.findSubPart( + mapping = (EmbeddableValuedModelPart) modelPartContainer.findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget ); @@ -92,7 +85,7 @@ public class EmbeddableValuedPathInterpretation extends AbstractSqmPathInterp return new EmbeddableValuedPathInterpretation<>( mapping.toSqlExpression( tableGroup, - currentClause, + sqlAstCreationState.getCurrentClauseStack().getCurrent(), sqlAstCreationState, sqlAstCreationState ),