HHH-1615 Test and fix for group by entity

hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java

@@ -44,6 +44,7 @@
 import org.hibernate.sql.ast.SqlAstTranslatorFactory;
 import org.hibernate.sql.ast.spi.StandardSqlAstTranslatorFactory;
 import org.hibernate.sql.ast.tree.Statement;
+import org.hibernate.sql.ast.tree.expression.SqlTupleContainer;
 import org.hibernate.sql.exec.spi.JdbcOperation;
 import org.hibernate.tool.schema.extract.internal.SequenceInformationExtractorH2DatabaseImpl;
 import org.hibernate.tool.schema.extract.internal.SequenceInformationExtractorLegacyImpl;
@@ -426,6 +427,11 @@ public boolean supportsTuplesInSubqueries() {
 		return supportsTuplesInSubqueries;
 	}
 
+	@Override
+	public boolean supportsSelectAliasInGroupByClause() {
+		return true;
+	}
+
 	@Override
 	public IdentityColumnSupport getIdentityColumnSupport() {
 		return new H2IdentityColumnSupport();

hibernate-core/src/main/java/org/hibernate/dialect/MySQLDialect.java

@@ -419,6 +419,11 @@ public boolean supportsUnionAll() {
 		return getMySQLVersion() >= 500;
 	}
 
+	@Override
+	public boolean supportsSelectAliasInGroupByClause() {
+		return true;
+	}
+
 	@Override
 	public boolean supportsColumnCheck() {
 		return false;

hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQLDialect.java

@@ -641,6 +641,11 @@ public boolean supportsRowValueConstructorSyntaxInQuantifiedPredicates() {
 		return true;
 	}
 
+	@Override
+	public boolean supportsSelectAliasInGroupByClause() {
+		return true;
+	}
+
 	@Override
 	public CallableStatementSupport getCallableStatementSupport() {
 		return PostgresCallableStatementSupport.INSTANCE;

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

@@ -341,11 +341,12 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 	private int fetchDepth;
 	private boolean resolvingCircularFetch;
 	private ForeignKeyDescriptor.Nature currentlyResolvingForeignKeySide;
+	private SqmQueryPart<?> currentSqmQueryPart;
 
 	private Map<String, FilterPredicate> collectionFilterPredicates;
 	private OrderByFragmentConsumer orderByFragmentConsumer;
 
-	private Map<String, NavigablePath> joinPathBySqmJoinFullPath = new HashMap<>();
+	private final Map<String, NavigablePath> joinPathBySqmJoinFullPath = new HashMap<>();
 
 	private final SqlAliasBaseManager sqlAliasBaseManager = new SqlAliasBaseManager();
 	private final Stack<SqlAstProcessingState> processingStateStack = new StandardStack<>();
@@ -1315,6 +1316,8 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
 		);
 		final DelegatingSqmAliasedNodeCollector collector = (DelegatingSqmAliasedNodeCollector) processingState
 				.getSqlExpressionResolver();
+		final SqmQueryPart<?> sqmQueryPart = currentSqmQueryPart;
+		currentSqmQueryPart = queryGroup;
 		pushProcessingState( processingState );
 
 		try {
@@ -1335,6 +1338,7 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
 		}
 		finally {
 			popProcessingStateStack();
+			currentSqmQueryPart = sqmQueryPart;
 		}
 	}
 
@@ -1386,6 +1390,8 @@ else if ( sqmQuerySpec.hasPositionalGroupItem() ) {
 			);
 		}
 
+		final SqmQueryPart<?> sqmQueryPart = currentSqmQueryPart;
+		currentSqmQueryPart = sqmQuerySpec;
 		pushProcessingState( processingState );
 
 		try {
@@ -1435,6 +1441,7 @@ else if ( sqmQuerySpec.hasPositionalGroupItem() ) {
 			}
 			additionalRestrictions = originalAdditionalRestrictions;
 			popProcessingStateStack();
+			currentSqmQueryPart = sqmQueryPart;
 		}
 	}
 
@@ -1657,10 +1664,22 @@ protected Expression resolveGroupOrOrderByExpression(SqmExpression<?> groupByCla
 			}
 			else if ( tableGroup.getModelPart() instanceof EntityValuedModelPart ) {
 				final EntityValuedModelPart mapping = (EntityValuedModelPart) tableGroup.getModelPart();
+				final boolean expandToAllColumns;
+				if ( currentClauseStack.getCurrent() == Clause.GROUP ) {
+					// When the table group is known to be fetched i.e. a fetch join
+					// but also when the from clause is part of the select clause
+					// we need to expand to all columns, as we also expand this to all columns in the select clause
+					expandToAllColumns = tableGroup.isFetched()
+							|| groupByClauseExpression instanceof SqmFrom<?, ?> && selectClauseContains( (SqmFrom<?, ?>) groupByClauseExpression );
+				}
+				else {
+					expandToAllColumns = false;
+				}
 				return EntityValuedPathInterpretation.from(
 						tableGroup.getNavigablePath(),
 						tableGroup,
 						mapping,
+						expandToAllColumns,
 						this
 				);
 			}
@@ -1668,6 +1687,20 @@ else if ( tableGroup.getModelPart() instanceof EntityValuedModelPart ) {
 		return expression;
 	}
 
+	private boolean selectClauseContains(SqmFrom<?, ?> from) {
+		final SqmQuerySpec<?> sqmQuerySpec = (SqmQuerySpec<?>) currentSqmQueryPart;
+		final List<SqmSelection> selections = sqmQuerySpec.getSelectClause().getSelections();
+		if ( selections.isEmpty() && from instanceof SqmRoot<?> ) {
+			return true;
+		}
+		for ( SqmSelection selection : selections ) {
+			if ( selection.getSelectableNode() == from ) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	@Override
 	public List<Expression> visitGroupByClause(List<SqmExpression<?>> groupByClauseExpressions) {
 		if ( !groupByClauseExpressions.isEmpty() ) {

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

@@ -14,11 +14,13 @@
 import org.hibernate.metamodel.mapping.BasicValuedModelPart;
 import org.hibernate.metamodel.mapping.CollectionPart;
 import org.hibernate.metamodel.mapping.EntityAssociationMapping;
+import org.hibernate.metamodel.mapping.EntityDiscriminatorMapping;
 import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
 import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.metamodel.mapping.EntityValuedModelPart;
 import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
 import org.hibernate.metamodel.mapping.ModelPart;
+import org.hibernate.metamodel.mapping.SelectableConsumer;
 import org.hibernate.metamodel.mapping.internal.SimpleForeignKeyDescriptor;
 import org.hibernate.query.NavigablePath;
 import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
@@ -65,19 +67,54 @@ public static <T> EntityValuedPathInterpretation<T> from(
 				.findTableGroup( sqmPath.getLhs().getNavigablePath() )
 				.getModelPart()
 				.findSubPart( sqmPath.getReferencedPathSource().getPathName(), null );
-		return from( sqmPath.getNavigablePath(), tableGroup, mapping, sqlAstCreationState );
+		return from( sqmPath.getNavigablePath(), tableGroup, mapping, false, sqlAstCreationState );
 	}
 
 	public static <T> EntityValuedPathInterpretation<T> from(
 			NavigablePath navigablePath,
 			TableGroup tableGroup,
 			EntityValuedModelPart mapping,
+			boolean expandToAllColumns,
 			SqmToSqlAstConverter sqlAstCreationState) {
 		final SqlExpressionResolver sqlExprResolver = sqlAstCreationState.getSqlExpressionResolver();
 		final SessionFactoryImplementor sessionFactory = sqlAstCreationState.getCreationContext().getSessionFactory();
 		final Expression sqlExpression;
 
-		if ( mapping instanceof EntityAssociationMapping ) {
+		if ( expandToAllColumns ) {
+			final EntityMappingType entityMappingType = mapping.getEntityMappingType();
+			final EntityIdentifierMapping identifierMapping = entityMappingType.getIdentifierMapping();
+			final EntityDiscriminatorMapping discriminatorMapping = entityMappingType.getDiscriminatorMapping();
+			final List<Expression> expressions = new ArrayList<>(
+					mapping.getJdbcTypeCount() + identifierMapping.getJdbcTypeCount()
+							+ ( discriminatorMapping == null ? 0 : 1 )
+			);
+			final SelectableConsumer selectableConsumer = (selectionIndex, selectableMapping) -> {
+				final TableReference tableReference = tableGroup.resolveTableReference(
+						navigablePath,
+						selectableMapping.getContainingTableExpression()
+				);
+				expressions.add(
+						sqlExprResolver.resolveSqlExpression(
+								createColumnReferenceKey(
+										tableReference,
+										selectableMapping.getSelectionExpression()
+								),
+								processingState -> new ColumnReference(
+										tableReference,
+										selectableMapping,
+										sessionFactory
+								)
+						)
+				);
+			};
+			identifierMapping.forEachSelectable( selectableConsumer );
+			if ( discriminatorMapping != null ) {
+				discriminatorMapping.forEachSelectable( selectableConsumer );
+			}
+			mapping.forEachSelectable( selectableConsumer );
+			sqlExpression = new SqlTuple( expressions, mapping );
+		}
+		else if ( mapping instanceof EntityAssociationMapping ) {
 			final EntityAssociationMapping associationMapping = (EntityAssociationMapping) mapping;
 			final ForeignKeyDescriptor fkDescriptor = associationMapping.getForeignKeyDescriptor();
 			final String lhsTable;

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

@@ -1518,8 +1518,8 @@ protected final void visitPartitionExpressions(List<Expression> partitionExpress
 		}
 		else {
 			for ( Expression partitionExpression : partitionExpressions ) {
-				if ( partitionExpression instanceof SqlTuple ) {
-					for ( Expression expression : ( (SqlTuple) partitionExpression ).getExpressions() ) {
+				if ( partitionExpression instanceof SqlTupleContainer ) {
+					for ( Expression expression : ( (SqlTupleContainer) partitionExpression ).getSqlTuple().getExpressions() ) {
 						appendSql( separator );
 						renderPartitionItem( resolveAliasedExpression( expression ) );
 						separator = COMA_SEPARATOR;

hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/GroupByTest.java

@@ -0,0 +1,52 @@
+/*
+ * 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.query.hql;
+
+import javax.persistence.Tuple;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.orm.domain.StandardDomainModel;
+import org.hibernate.testing.orm.domain.contacts.Contact;
+import org.hibernate.testing.orm.domain.gambit.EntityOfBasics;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.ServiceRegistry;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+/**
+ * @author Christian Beikov
+ */
+@ServiceRegistry
+@DomainModel(standardModels = StandardDomainModel.CONTACTS)
+@SessionFactory
+public class GroupByTest {
+
+	@BeforeAll
+	public void prepareData(SessionFactoryScope scope) {
+		scope.inTransaction(
+				em -> {
+					Contact entity1 = new Contact();
+					entity1.setId( 123 );
+					em.persist( entity1 );
+				}
+		);
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-1615")
+	public void testGroupByEntity(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					session.createQuery( "select e, count(*) from Contact e group by e", Tuple.class ).list();
+				}
+		);
+	}
+
+}