HHH-16189 wrong order by and group by generated

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

@@ -886,7 +886,9 @@ public void addVersionedAssignment(Consumer<Assignment> assignmentConsumer, SqmU
 						.get( versionMapping.getPartName() ),
 				this,
 				this,
-				jpaQueryComplianceEnabled ).getColumnReferences();
+				jpaQueryComplianceEnabled,
+				Clause.SET
+		).getColumnReferences();
 		assert targetColumnReferences.size() == 1;
 
 		final ColumnReference versionColumn = targetColumnReferences.get( 0 );
@@ -1331,7 +1333,8 @@ else if ( localName.equals( versionAttributeName ) ) {
 							.get( versionAttributeName ),
 					this,
 					this,
-					jpaQueryComplianceEnabled
+					jpaQueryComplianceEnabled,
+					getCurrentClauseStack().getCurrent()
 			);
 			final List<ColumnReference> targetColumnReferences = versionPath.getColumnReferences();
 			assert targetColumnReferences.size() == 1;
@@ -3960,7 +3963,8 @@ public Expression visitBasicValuedPath(SqmBasicValuedSimplePath<?> sqmPath) {
 						sqmPath,
 						this,
 						this,
-						jpaQueryComplianceEnabled
+						jpaQueryComplianceEnabled,
+						getCurrentClauseStack().getCurrent()
 				)
 		);
 		Expression result = path;
@@ -4058,7 +4062,13 @@ public Expression visitEmbeddableValuedPath(SqmEmbeddedValuedSimplePath<?> sqmPa
 		return withTreatRestriction(
 				prepareReusablePath(
 						sqmPath,
-						() -> EmbeddableValuedPathInterpretation.from( sqmPath, this, this, jpaQueryComplianceEnabled )
+						() -> EmbeddableValuedPathInterpretation.from(
+								sqmPath,
+								this,
+								this,
+								jpaQueryComplianceEnabled,
+								getCurrentClauseStack().getCurrent()
+						)
 				),
 				sqmPath
 		);

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

@@ -13,18 +13,21 @@
 import org.hibernate.metamodel.MappingMetamodel;
 import org.hibernate.metamodel.mapping.BasicValuedModelPart;
 import org.hibernate.metamodel.mapping.EntityMappingType;
+import org.hibernate.metamodel.mapping.ManagedMappingType;
 import org.hibernate.metamodel.mapping.ModelPart;
+import org.hibernate.metamodel.mapping.ModelPartContainer;
 import org.hibernate.metamodel.model.domain.EntityDomainType;
+import org.hibernate.query.sqm.tree.from.SqmFrom;
 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.spi.SqlExpressionResolver;
 import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.ast.tree.expression.Expression;
 import org.hibernate.sql.ast.tree.expression.SqlSelectionExpression;
@@ -43,7 +46,8 @@ public static <T> BasicValuedPathInterpretation<T> from(
 			SqmBasicValuedSimplePath<T> sqmPath,
 			SqlAstCreationState sqlAstCreationState,
 			SemanticQueryWalker sqmWalker,
-			boolean jpaQueryComplianceEnabled) {
+			boolean jpaQueryComplianceEnabled,
+			Clause currentClause) {
 		final FromClauseAccess fromClauseAccess = sqlAstCreationState.getFromClauseAccess();
 		final TableGroup tableGroup = fromClauseAccess.getTableGroup( sqmPath.getNavigablePath().getParent() );
 
@@ -68,10 +72,24 @@ else if ( sqmPath.getLhs().getNodeType() instanceof EntityDomainType ) {
 			}
 		}
 
-		final BasicValuedModelPart mapping = (BasicValuedModelPart) tableGroup.getModelPart().findSubPart(
-				sqmPath.getReferencedPathSource().getPathName(),
-				treatTarget
-		);
+		final ModelPartContainer modelPart = tableGroup.getModelPart();
+		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.
+		if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
+				&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
+				&& modelPart.getPartMappingType() instanceof ManagedMappingType ) {
+			mapping = (BasicValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart(
+					sqmPath.getReferencedPathSource().getPathName(),
+					treatTarget
+			);
+		}
+		else {
+			mapping = (BasicValuedModelPart) modelPart.findSubPart(
+					sqmPath.getReferencedPathSource().getPathName(),
+					treatTarget
+			);
+		}
 
 		if ( mapping == null ) {
 			if ( jpaQueryComplianceEnabled ) {

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

@@ -13,12 +13,16 @@
 import org.hibernate.metamodel.MappingMetamodel;
 import org.hibernate.metamodel.mapping.EmbeddableValuedModelPart;
 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.from.SqmFrom;
 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;
+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;
@@ -39,7 +43,8 @@ public static <T> Expression from(
 			SqmEmbeddedValuedSimplePath<T> sqmPath,
 			SqmToSqlAstConverter converter,
 			SemanticQueryWalker<?> sqmWalker,
-			boolean jpaQueryComplianceEnabled) {
+			boolean jpaQueryComplianceEnabled,
+			Clause currentClause) {
 		TableGroup tableGroup = converter.getFromClauseAccess().findTableGroup( sqmPath.getLhs().getNavigablePath() );
 
 		EntityMappingType treatTarget = null;
@@ -61,10 +66,24 @@ else if ( sqmPath.getLhs().getNodeType() instanceof EntityDomainType ) {
 			}
 		}
 
-		final EmbeddableValuedModelPart mapping = (EmbeddableValuedModelPart) tableGroup.getModelPart().findSubPart(
-				sqmPath.getReferencedPathSource().getPathName(),
-				treatTarget
-		);
+		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.
+		if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
+				&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
+				&& modelPart.getPartMappingType() instanceof ManagedMappingType ) {
+			mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart(
+					sqmPath.getReferencedPathSource().getPathName(),
+					treatTarget
+			);
+		}
+		else {
+			mapping = (EmbeddableValuedModelPart) modelPart.findSubPart(
+					sqmPath.getReferencedPathSource().getPathName(),
+					treatTarget
+			);
+		}
 
 		return new EmbeddableValuedPathInterpretation<>(
 				mapping.toSqlExpression(

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java

@@ -65,6 +65,20 @@ public void testMapKeyJoinIsOmitted(SessionFactoryScope scope) {
 		);
 	}
 
+	@Test
+	public void testMapKeyDeReferenceDoesNotCauseJoin(SessionFactoryScope scope) {
+		SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
+		statementInspector.clear();
+		scope.inTransaction(
+				s -> {
+					s.createQuery( "select c from MapOwner as o left join o.contents c where key(c).id is not null" ).list();
+					statementInspector.assertExecutedCount( 1 );
+					// Assert 2 joins, collection table and collection element
+					statementInspector.assertNumberOfJoins( 0, 2 );
+				}
+		);
+	}
+
 	@Test
 	public void testMapKeyJoinIsReused(SessionFactoryScope scope) {
 		SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();

hibernate-core/src/test/java/org/hibernate/orm/test/orderby/EmbeddedIdOrderByAndAggregateFunctionTest.java

@@ -0,0 +1,137 @@
+package org.hibernate.orm.test.orderby;
+
+import org.hibernate.dialect.H2Dialect;
+import org.hibernate.dialect.PostgreSQLDialect;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.RequiresDialect;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.Column;
+import jakarta.persistence.Embeddable;
+import jakarta.persistence.EmbeddedId;
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.ManyToOne;
+
+@DomainModel(
+		annotatedClasses = {
+				EmbeddedIdOrderByAndAggregateFunctionTest.Parent.class,
+				EmbeddedIdOrderByAndAggregateFunctionTest.Child.class,
+		}
+)
+@SessionFactory
+@TestForIssue(jiraKey = "HHH-16189")
+public class EmbeddedIdOrderByAndAggregateFunctionTest {
+
+	@Test
+	@RequiresDialect(PostgreSQLDialect.class)
+	@RequiresDialect(H2Dialect.class)
+	public void testSelectWithOrderAndGroupBy(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					session.createQuery(
+									"select c.id,c.name, sum(p.number) " +
+											"from Parent p " +
+											"inner join p.child c " +
+											"group by c.id " +
+											"order by c.name",
+									Object[].class
+							)
+							.getResultList();
+				}
+		);
+	}
+
+	@Test
+	public void testSelectWithOrderAndGroupBy2(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					session.createQuery(
+									"select c.id, sum(p.number) " +
+											"from Parent p " +
+											"inner join p.child c " +
+											"group by c.id " +
+											"order by c.id",
+									Object[].class
+							)
+							.getResultList();
+				}
+		);
+	}
+
+	@Test
+	public void testSelectWithOrderAndGroupBy3(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					session.createQuery(
+									"select c.id.id1, sum(p.number) " +
+											"from Parent p " +
+											"inner join p.child c " +
+											"group by c.id.id1 " +
+											"order by c.id.id1",
+									Object[].class
+							)
+							.getResultList();
+				}
+		);
+	}
+
+	@Test
+	public void testSelectWithHaving(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					session.createQuery(
+									"select c.id, sum(p.number) " +
+											"from Parent p " +
+											"inner join p.child c " +
+											"group by c.id " +
+											"having c.id > :childId",
+									Object[].class
+							)
+							.setParameter( "childId", new ChildId( 1l, 2l ) )
+							.getResultList();
+				}
+		);
+	}
+
+	@Entity(name = "Parent")
+	public static class Parent {
+
+		@Id
+		private String code;
+
+		@ManyToOne
+		private Child child;
+
+		@Column(name = "NUMBER_COLUMN")
+		int number;
+	}
+
+	@Entity(name = "Child")
+	public static class Child {
+
+		@EmbeddedId
+		private ChildId id;
+
+		private String name;
+	}
+
+	@Embeddable
+	public static class ChildId {
+		private long id1;
+		private long id2;
+
+		public ChildId() {
+		}
+
+		public ChildId(long id1, long id2) {
+			this.id1 = id1;
+			this.id2 = id2;
+		}
+	}
+
+}