HHH-18850 fix count queries with 'distinct' and 'order by'

Author: gavinking

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmOrderByClause.java

@@ -6,13 +6,15 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import org.hibernate.query.sqm.tree.SqmCopyContext;
 import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmExpression;
 
+import static java.util.Collections.emptyList;
+import static java.util.Collections.unmodifiableList;
+
 /**
  * @author Steve Ebersole
  */
@@ -69,12 +71,7 @@ public SqmOrderByClause addSortSpecification(SqmExpression<?> expression) {
 	}
 
 	public List<SqmSortSpecification> getSortSpecifications() {
-		if ( sortSpecifications == null ) {
-			return Collections.emptyList();
-		}
-		else {
-			return Collections.unmodifiableList( sortSpecifications );
-		}
+		return sortSpecifications == null ? emptyList() : unmodifiableList( sortSpecifications );
 	}
 
 	public void setSortSpecifications(List<SqmSortSpecification> sortSpecifications) {

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmQueryPart.java

@@ -4,7 +4,6 @@
  */
 package org.hibernate.query.sqm.tree.select;
 
-import java.util.Collections;
 import java.util.List;
 
 import org.hibernate.query.common.FetchClauseType;
@@ -16,6 +15,8 @@
 import org.hibernate.query.sqm.tree.SqmVisitableNode;
 import org.hibernate.query.sqm.tree.expression.SqmExpression;
 
+import static java.util.Collections.emptyList;
+
 /**
  * Defines the ordering and fetch/offset part of a query which is shared with query groups.
  *
@@ -127,11 +128,7 @@ public FetchClauseType getFetchClauseType() {
 
 	@Override
 	public List<SqmSortSpecification> getSortSpecifications() {
-		if ( getOrderByClause() == null ) {
-			return Collections.emptyList();
-		}
-
-		return getOrderByClause().getSortSpecifications();
+		return getOrderByClause() == null ? emptyList() : getOrderByClause().getSortSpecifications();
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java

@@ -549,6 +549,9 @@ public SqmSelectStatement<Long> createCountQuery() {
 			final SqmSelectStatement<Long> query = nodeBuilder().createQuery( Long.class );
 			query.from( subquery );
 			query.select( nodeBuilder().count() );
+			if ( subquery.getFetch() == null && subquery.getOffset() == null ) {
+				subquery.getQueryPart().setOrderByClause( null );
+			}
 			return query;
 		}
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/id/idClass/IdClassSyntheticAttributesTest.java

@@ -1,8 +1,6 @@
 /*
- * 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>.
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ * Copyright Red Hat Inc. and Hibernate Authors
  */
 package org.hibernate.orm.test.id.idClass;
 
@@ -16,23 +14,23 @@
 import static org.assertj.core.api.Assertions.assertThat;
 
 @DomainModel(
-        annotatedClasses = MyEntity.class
+		annotatedClasses = MyEntity.class
 )
 @SessionFactory
 public class IdClassSyntheticAttributesTest {
 
-    @Jira("https://hibernate.atlassian.net/browse/HHH-18841")
-    @Test
-    public void test(DomainModelScope scope) {
-        final PersistentClass entityBinding = scope.getDomainModel().getEntityBinding(MyEntity.class.getName());
-        assertThat(entityBinding.getProperties()).hasSize(2)
-                .anySatisfy(p -> {
-                    assertThat(p.isSynthetic()).isTrue();
-                    assertThat(p.getName()).isEqualTo("_identifierMapper");
-                })
-                .anySatisfy(p -> {
-                    assertThat(p.isSynthetic()).isFalse();
-                    assertThat(p.getName()).isEqualTo("notes");
-                });
-    }
+	@Jira("https://hibernate.atlassian.net/browse/HHH-18841")
+	@Test
+	public void test(DomainModelScope scope) {
+		final PersistentClass entityBinding = scope.getDomainModel().getEntityBinding(MyEntity.class.getName());
+		assertThat(entityBinding.getProperties()).hasSize(2)
+				.anySatisfy(p -> {
+					assertThat(p.isSynthetic()).isTrue();
+					assertThat(p.getName()).isEqualTo("_identifierMapper");
+				})
+				.anySatisfy(p -> {
+					assertThat(p.isSynthetic()).isFalse();
+					assertThat(p.getName()).isEqualTo("notes");
+				});
+	}
 }

hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CountQueryTests.java

@@ -84,6 +84,48 @@ public void testForHHH17967(SessionFactoryScope scope) {
 		);
 	}
 
+	@Test
+	@JiraKey( "HHH-18850" )
+	public void testForHHH18850(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					HibernateCriteriaBuilder cb = session.getCriteriaBuilder();
+					JpaCriteriaQuery<Contract> cq = cb.createQuery( Contract.class );
+					cq.distinct( true );
+					Root<Contract> root = cq.from( Contract.class );
+					cq.select( root );
+					cq.orderBy( cb.asc( root.get( "customerName" ) ) );
+					TypedQuery<Long> query = session.createQuery( cq.createCountQuery() );
+					try {
+						// Leads to NPE on pre-6.5 versions
+						query.getSingleResult();
+					}
+					catch (Exception e) {
+						fail( e );
+					}
+				}
+		);
+
+		scope.inTransaction(
+				session -> {
+					HibernateCriteriaBuilder cb = session.getCriteriaBuilder();
+					JpaCriteriaQuery<Contract> cq = cb.createQuery( Contract.class );
+					cq.distinct( false );
+					Root<Contract> root = cq.from( Contract.class );
+					cq.select( root );
+					cq.orderBy( cb.desc( root.get( "customerName" ) ) );
+					TypedQuery<Long> query = session.createQuery( cq.createCountQuery() );
+					try {
+						// Leads to NPE on pre-6.5 versions
+						query.getSingleResult();
+					}
+					catch (Exception e) {
+						fail( e );
+					}
+				}
+		);
+	}
+
 	@Test
 	@JiraKey("HHH-17410")
 	public void testBasic(SessionFactoryScope scope) {