HHH-16691 Avoid join table joins for SqmPath in some more scenarios

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

@@ -762,6 +762,11 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 		return currentSqmQueryPart;
 	}
 
+	@Override
+	public SqmStatement<?> getCurrentSqmStatement() {
+		return currentSqmStatement;
+	}
+
 	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 	// Statements
 
@@ -891,16 +896,16 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> 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<ColumnReference> targetColumnReferences = BasicValuedPathInterpretation.from(
 				(SqmBasicValuedSimplePath<?>) sqmStatement
 						.getRoot()
 						.get( versionMapping.getPartName() ),
 				this,
-				this,
+				jpaQueryComplianceEnabled
-				jpaQueryComplianceEnabled,
-				Clause.SET
 		).getColumnReferences();
+		currentClauseStack.pop();
 		assert targetColumnReferences.size() == 1;
 
 		final ColumnReference versionColumn = targetColumnReferences.get( 0 );
@@ -1344,9 +1349,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 					(SqmBasicValuedSimplePath<?>) sqmStatement.getTarget()
 							.get( versionAttributeName ),
 					this,
-					this,
+					jpaQueryComplianceEnabled
-					jpaQueryComplianceEnabled,
-					getCurrentClauseStack().getCurrent()
 			);
 			final List<ColumnReference> targetColumnReferences = versionPath.getColumnReferences();
 			assert targetColumnReferences.size() == 1;
@@ -4151,9 +4154,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				() -> BasicValuedPathInterpretation.from(
 						sqmPath,
 						this,
-						this,
+						jpaQueryComplianceEnabled
-						jpaQueryComplianceEnabled,
-						getCurrentClauseStack().getCurrent()
 				)
 		);
 		Expression result = path;
@@ -4254,9 +4255,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 						() -> EmbeddableValuedPathInterpretation.from(
 								sqmPath,
 								this,
-								this,
+								jpaQueryComplianceEnabled
-								jpaQueryComplianceEnabled,
-								getCurrentClauseStack().getCurrent()
 						)
 				),
 				sqmPath

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) {
 	}

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<Object>, SqlAs
 
 	SqmQueryPart<?> getCurrentSqmQueryPart();
 
+	SqmStatement<?> getCurrentSqmStatement();
+
 	void registerQueryTransformer(QueryTransformer transformer);
 
 	/**

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<T> extends AbstractSqmPathInterpretat
 	 */
 	public static <T> BasicValuedPathInterpretation<T> from(
 			SqmBasicValuedSimplePath<T> sqmPath,
-			SqlAstCreationState sqlAstCreationState,
+			SqmToSqlAstConverter sqlAstCreationState,
-			SemanticQueryWalker<?> sqmWalker,
+			boolean jpaQueryComplianceEnabled) {
-			boolean jpaQueryComplianceEnabled,
-			Clause currentClause) {
 		final FromClauseAccess fromClauseAccess = sqlAstCreationState.getFromClauseAccess();
 		final TableGroup tableGroup = fromClauseAccess.getTableGroup( sqmPath.getNavigablePath().getParent() );
 
@@ -89,10 +88,15 @@ public class BasicValuedPathInterpretation<T> 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

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<T> extends AbstractSqmPathInterp
 	 */
 	public static <T> Expression from(
 			SqmEmbeddedValuedSimplePath<T> sqmPath,
-			SqmToSqlAstConverter converter,
+			SqmToSqlAstConverter sqlAstCreationState,
-			SemanticQueryWalker<?> sqmWalker,
+			boolean jpaQueryComplianceEnabled) {
-			boolean jpaQueryComplianceEnabled,
+		TableGroup tableGroup = sqlAstCreationState.getFromClauseAccess().findTableGroup( sqmPath.getLhs().getNavigablePath() );
-			Clause currentClause) {
-		TableGroup tableGroup = converter.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<T> 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<T> extends AbstractSqmPathInterp
 		return new EmbeddableValuedPathInterpretation<>(
 				mapping.toSqlExpression(
 						tableGroup,
-						converter.getCurrentClauseStack().getCurrent(),
+						currentClause,
-						converter,
+						sqlAstCreationState,
-						converter
+						sqlAstCreationState
 				),
 				sqmPath.getNavigablePath(),
 				mapping,

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<T> 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<T> 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(

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<T> extends SqmQueryPart<T>
 			appendJoins( sqmTreat, sb );
 		}
 	}
+
+	public boolean groupByClauseContains(NavigablePath path) {
+		for ( SqmExpression<?> expression : groupByClauseExpressions ) {
+			if ( expression instanceof SqmPath && ( (SqmPath<?>) expression ).getNavigablePath() == path ) {
+				return true;
+			}
+		}
+		return false;
+	}
 }

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 <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+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<Person> people;
+	}
+	@Entity(name = "Person")
+	public static class Person {
+		@Id
+		Long id;
+		String name;
+	}
+}