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 22801790e9..6efc1f93ae 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 @@ -762,6 +762,11 @@ public abstract class BaseSqmToSqlAstConverter extends Base return currentSqmQueryPart; } + @Override + public SqmStatement getCurrentSqmStatement() { + return currentSqmStatement; + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // Statements @@ -891,16 +896,16 @@ public abstract class BaseSqmToSqlAstConverter extends Base throw new SemanticException( "user-defined version types not supported for increment option" ); } + currentClauseStack.push( Clause.SET ); final EntityVersionMapping versionMapping = persister.getVersionMapping(); final List targetColumnReferences = BasicValuedPathInterpretation.from( (SqmBasicValuedSimplePath) sqmStatement .getRoot() .get( versionMapping.getPartName() ), this, - this, - jpaQueryComplianceEnabled, - Clause.SET + jpaQueryComplianceEnabled ).getColumnReferences(); + currentClauseStack.pop(); assert targetColumnReferences.size() == 1; final ColumnReference versionColumn = targetColumnReferences.get( 0 ); @@ -1344,9 +1349,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base (SqmBasicValuedSimplePath) sqmStatement.getTarget() .get( versionAttributeName ), this, - this, - jpaQueryComplianceEnabled, - getCurrentClauseStack().getCurrent() + jpaQueryComplianceEnabled ); final List targetColumnReferences = versionPath.getColumnReferences(); assert targetColumnReferences.size() == 1; @@ -4151,9 +4154,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base () -> BasicValuedPathInterpretation.from( sqmPath, this, - this, - jpaQueryComplianceEnabled, - getCurrentClauseStack().getCurrent() + jpaQueryComplianceEnabled ) ); Expression result = path; @@ -4254,9 +4255,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base () -> EmbeddableValuedPathInterpretation.from( sqmPath, this, - this, - jpaQueryComplianceEnabled, - getCurrentClauseStack().getCurrent() + jpaQueryComplianceEnabled ) ), sqmPath 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 37a8a2fa78..5b7de88ffa 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 @@ -14,6 +14,7 @@ import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.internal.util.collections.Stack; import org.hibernate.metamodel.mapping.MappingModelExpressible; import org.hibernate.query.sqm.spi.BaseSemanticQueryWalker; +import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.SqmVisitableNode; import org.hibernate.query.sqm.tree.expression.SqmExpression; import org.hibernate.query.sqm.tree.expression.SqmParameter; @@ -92,6 +93,11 @@ public class FakeSqmToSqlAstConverter extends BaseSemanticQueryWalker implements return null; } + @Override + public SqmStatement getCurrentSqmStatement() { + return null; + } + @Override public void registerQueryTransformer(QueryTransformer transformer) { } 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 378fef24be..d90962db08 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 @@ -12,6 +12,7 @@ import java.util.function.Supplier; import org.hibernate.internal.util.collections.Stack; import org.hibernate.metamodel.mapping.MappingModelExpressible; import org.hibernate.query.sqm.SemanticQueryWalker; +import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.SqmVisitableNode; import org.hibernate.query.sqm.tree.expression.SqmExpression; import org.hibernate.query.sqm.tree.expression.SqmParameter; @@ -33,6 +34,8 @@ public interface SqmToSqlAstConverter extends SemanticQueryWalker, SqlAs SqmQueryPart getCurrentSqmQueryPart(); + SqmStatement getCurrentSqmStatement(); + void registerQueryTransformer(QueryTransformer transformer); /** 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 3f1515bf1c..b4bdeba3a2 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 @@ -18,19 +18,20 @@ import org.hibernate.metamodel.mapping.MappingType; import org.hibernate.metamodel.mapping.ModelPart; import org.hibernate.metamodel.mapping.ModelPartContainer; import org.hibernate.metamodel.model.domain.EntityDomainType; +import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; +import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.from.SqmFrom; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.query.sqm.tree.domain.SqmPath; +import org.hibernate.query.sqm.tree.select.SqmSelectStatement; import org.hibernate.spi.NavigablePath; import org.hibernate.query.SemanticException; -import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.StrictJpaComplianceViolation; import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstWalker; import org.hibernate.sql.ast.spi.FromClauseAccess; -import org.hibernate.sql.ast.spi.SqlAstCreationState; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.Expression; import org.hibernate.sql.ast.tree.expression.SqlSelectionExpression; @@ -47,10 +48,8 @@ public class BasicValuedPathInterpretation extends AbstractSqmPathInterpretat */ public static BasicValuedPathInterpretation from( SqmBasicValuedSimplePath sqmPath, - SqlAstCreationState sqlAstCreationState, - SemanticQueryWalker sqmWalker, - boolean jpaQueryComplianceEnabled, - Clause currentClause) { + SqmToSqlAstConverter sqlAstCreationState, + boolean jpaQueryComplianceEnabled) { final FromClauseAccess fromClauseAccess = sqlAstCreationState.getFromClauseAccess(); final TableGroup tableGroup = fromClauseAccess.getTableGroup( sqmPath.getNavigablePath().getParent() ); @@ -89,10 +88,15 @@ 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. + // 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 SqmStatement sqmStatement = sqlAstCreationState.getCurrentSqmStatement(); if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING ) && sqmPath.getLhs() instanceof SqmFrom - && modelPartContainer.getPartMappingType() instanceof ManagedMappingType ) { + && modelPartContainer.getPartMappingType() instanceof ManagedMappingType + && sqmStatement instanceof SqmSelectStatement + && ( (SqmSelectStatement) sqmStatement ).getQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) { 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 94acf9cd2d..256e2f6a05 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 @@ -16,9 +16,10 @@ import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.ManagedMappingType; import org.hibernate.metamodel.mapping.ModelPartContainer; import org.hibernate.metamodel.model.domain.EntityDomainType; +import org.hibernate.query.sqm.tree.SqmStatement; import org.hibernate.query.sqm.tree.from.SqmFrom; +import org.hibernate.query.sqm.tree.select.SqmSelectStatement; import org.hibernate.spi.NavigablePath; -import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; @@ -41,15 +42,13 @@ public class EmbeddableValuedPathInterpretation extends AbstractSqmPathInterp */ public static Expression from( SqmEmbeddedValuedSimplePath sqmPath, - SqmToSqlAstConverter converter, - SemanticQueryWalker sqmWalker, - boolean jpaQueryComplianceEnabled, - Clause currentClause) { - TableGroup tableGroup = converter.getFromClauseAccess().findTableGroup( sqmPath.getLhs().getNavigablePath() ); + SqmToSqlAstConverter sqlAstCreationState, + boolean jpaQueryComplianceEnabled) { + TableGroup tableGroup = sqlAstCreationState.getFromClauseAccess().findTableGroup( sqmPath.getLhs().getNavigablePath() ); EntityMappingType treatTarget = null; if ( jpaQueryComplianceEnabled ) { - final MappingMetamodel mappingMetamodel = converter.getCreationContext() + final MappingMetamodel mappingMetamodel = sqlAstCreationState.getCreationContext() .getSessionFactory() .getRuntimeMetamodels() .getMappingMetamodel(); @@ -69,10 +68,15 @@ public class EmbeddableValuedPathInterpretation extends AbstractSqmPathInterp final ModelPartContainer modelPart = 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. + // 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 SqmStatement sqmStatement = sqlAstCreationState.getCurrentSqmStatement(); if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING ) && sqmPath.getLhs() instanceof SqmFrom - && modelPart.getPartMappingType() instanceof ManagedMappingType ) { + && modelPart.getPartMappingType() instanceof ManagedMappingType + && sqmStatement instanceof SqmSelectStatement + && ( (SqmSelectStatement) sqmStatement ).getQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) { mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart( sqmPath.getReferencedPathSource().getPathName(), treatTarget @@ -88,9 +92,9 @@ public class EmbeddableValuedPathInterpretation extends AbstractSqmPathInterp return new EmbeddableValuedPathInterpretation<>( mapping.toSqlExpression( tableGroup, - converter.getCurrentClauseStack().getCurrent(), - converter, - converter + currentClause, + sqlAstCreationState, + sqlAstCreationState ), sqmPath.getNavigablePath(), mapping, 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 f693f6b571..6082765459 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 @@ -27,7 +27,6 @@ import org.hibernate.query.derived.AnonymousTupleEntityValuedModelPart; import org.hibernate.query.sqm.sql.SqmToSqlAstConverter; import org.hibernate.query.sqm.tree.domain.SqmEntityValuedSimplePath; import org.hibernate.query.sqm.tree.domain.SqmPath; -import org.hibernate.query.sqm.tree.expression.SqmExpression; import org.hibernate.query.sqm.tree.select.SqmDynamicInstantiation; import org.hibernate.query.sqm.tree.select.SqmDynamicInstantiationArgument; import org.hibernate.query.sqm.tree.select.SqmQuerySpec; @@ -259,7 +258,7 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); if ( currentClause == Clause.GROUP || currentClause == Clause.ORDER ) { final SqmQuerySpec querySpec = (SqmQuerySpec) sqlAstCreationState.getCurrentSqmQueryPart(); - if ( currentClause == Clause.ORDER && !groupByClauseContains( navigablePath, querySpec ) ) { + if ( currentClause == Clause.ORDER && !querySpec.groupByClauseContains( navigablePath ) ) { // 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; @@ -373,15 +372,6 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta return false; } - private static boolean groupByClauseContains(NavigablePath path, SqmQuerySpec sqmQuerySpec) { - for ( SqmExpression expression : sqmQuerySpec.getGroupByClauseExpressions() ) { - if ( expression instanceof SqmPath && ( (SqmPath) expression ).getNavigablePath() == path ) { - return true; - } - } - return false; - } - private final Expression sqlExpression; public EntityValuedPathInterpretation( 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 05e46bf437..b3d838ed69 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 @@ -28,6 +28,7 @@ import org.hibernate.query.sqm.SemanticQueryWalker; import org.hibernate.query.sqm.tree.SqmCopyContext; import org.hibernate.query.sqm.tree.SqmNode; import org.hibernate.query.sqm.tree.domain.SqmEntityValuedSimplePath; +import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.domain.SqmTreatedPath; import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef; import org.hibernate.query.sqm.tree.expression.SqmExpression; @@ -42,6 +43,7 @@ import org.hibernate.query.sqm.tree.from.SqmRoot; import org.hibernate.query.sqm.tree.predicate.SqmPredicate; import org.hibernate.query.sqm.tree.predicate.SqmWhereClause; import org.hibernate.query.sqm.tree.predicate.SqmWhereClauseContainer; +import org.hibernate.spi.NavigablePath; import jakarta.persistence.criteria.Expression; import jakarta.persistence.criteria.Predicate; @@ -698,4 +700,13 @@ public class SqmQuerySpec extends SqmQueryPart appendJoins( sqmTreat, sb ); } } + + public boolean groupByClauseContains(NavigablePath path) { + for ( SqmExpression expression : groupByClauseExpressions ) { + if ( expression instanceof SqmPath && ( (SqmPath) expression ).getNavigablePath() == path ) { + return true; + } + } + return false; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java new file mode 100644 index 0000000000..766f9ca506 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java @@ -0,0 +1,60 @@ +/* + * 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 . + */ +package org.hibernate.orm.test.jpa.ql; + +import java.util.Set; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.jdbc.SQLStatementInspector; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.JoinTable; +import jakarta.persistence.OneToMany; + +@TestForIssue(jiraKey = "HHH-16691") +@DomainModel( + annotatedClasses = { + JoinTableOptimizationTest.Document.class, JoinTableOptimizationTest.Person.class + }) +@SessionFactory(useCollectingStatementInspector = true) +public class JoinTableOptimizationTest { + + @Test + public void testOnlyCollectionTableJoined(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.id from Document d left join d.people p where p.id is not null" ).list(); + statementInspector.assertExecutedCount( 1 ); + // Assert only the collection table is joined + statementInspector.assertNumberOfJoins( 0, 1 ); + } + ); + } + + @Entity(name = "Document") + public static class Document { + @Id + Long id; + String name; + @OneToMany + @JoinTable(name = "people") + Set people; + } + @Entity(name = "Person") + public static class Person { + @Id + Long id; + String name; + } +}