HHH-13619 - Support for JPA's `size` function as a select expression

- code cleanup

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/HqlSqlWalker.java

@@ -84,7 +84,6 @@ import org.hibernate.param.VersionTypeSeedParameterSpecification;
 import org.hibernate.persister.collection.CollectionPropertyNames;
 import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.persister.entity.EntityPersister;
-import org.hibernate.persister.entity.PropertyMapping;
 import org.hibernate.persister.entity.Queryable;
 import org.hibernate.sql.JoinType;
 import org.hibernate.type.AssociationType;
@@ -645,8 +644,7 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
 	@Override
 	protected AST createCollectionSizeFunction(AST collectionPath, boolean inSelect) throws SemanticException {
 		assert collectionPath instanceof CollectionPathNode;
-
+		return new CollectionSizeNode( (CollectionPathNode) collectionPath );
-		return new CollectionSizeNode( (CollectionPathNode) collectionPath, this );
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/SqlGenerator.java

@@ -423,9 +423,9 @@ public class SqlGenerator extends SqlGeneratorBase implements ErrorReporter {
 
 	@Override
 	protected String renderOrderByElement(String expression, String order, String nulls) {
-		final NullPrecedence nullPrecedence = NullPrecedence.parse( nulls,
+		final NullPrecedence nullPrecedence = NullPrecedence.parse(
-																	sessionFactory.getSettings()
+				nulls,
-																			.getDefaultNullPrecedence()
+				sessionFactory.getSettings().getDefaultNullPrecedence()
 		);
 		return sessionFactory.getDialect().renderOrderByElement( expression, null, order, nullPrecedence );
 	}
@@ -436,11 +436,13 @@ public class SqlGenerator extends SqlGeneratorBase implements ErrorReporter {
 
 		final CollectionSizeNode collectionSizeNode = (CollectionSizeNode) ast;
 
-		// todo : or `#getStringBuilder()` directly?
 		try {
 			writer.clause( collectionSizeNode.toSqlExpression() );
 		}
-		catch (SemanticException e) {
+		catch (QueryException qe) {
+			throw qe;
+		}
+		catch (Exception e) {
 			throw new QueryException( "Unable to render collection-size node" );
 		}
 	}

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/CollectionPathNode.java

@@ -11,9 +11,11 @@ import java.util.List;
 import org.hibernate.QueryException;
 import org.hibernate.hql.internal.ast.HqlSqlWalker;
 import org.hibernate.hql.internal.ast.SqlASTFactory;
+import org.hibernate.internal.util.StringHelper;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.persister.collection.QueryableCollection;
 import org.hibernate.persister.entity.EntityPersister;
+import org.hibernate.persister.entity.Joinable;
 import org.hibernate.persister.entity.PropertyMapping;
 import org.hibernate.type.CollectionType;
 import org.hibernate.type.CompositeType;
@@ -38,6 +40,7 @@ public class CollectionPathNode extends SqlNode {
 	private final String collectionPropertyPath;
 	private final String collectionQueryPath;
 
+	private final HqlSqlWalker walker;
 
 
 	/**
@@ -50,12 +53,16 @@ public class CollectionPathNode extends SqlNode {
 			CollectionPersister collectionDescriptor,
 			String collectionPropertyName,
 			String collectionQueryPath,
-			String collectionPropertyPath) {
+			String collectionPropertyPath,
+			HqlSqlWalker walker) {
 		this.ownerFromElement = ownerFromElement;
 		this.collectionDescriptor = collectionDescriptor;
 		this.collectionPropertyName = collectionPropertyName;
 		this.collectionQueryPath = collectionQueryPath;
 		this.collectionPropertyPath = collectionPropertyPath;
+		this.walker = walker;
+
+		walker.addQuerySpaces( collectionDescriptor.getCollectionSpaces() );
 
 		super.setType( SqlASTFactory.COLL_PATH );
 		super.setDataType( collectionDescriptor.getCollectionType() );
@@ -99,7 +106,8 @@ public class CollectionPathNode extends SqlNode {
 						collectionDescriptor,
 						referenceName,
 						referencePath,
-						referenceName
+						referenceName,
+						walker
 				);
 			}
 			else {
@@ -126,7 +134,8 @@ public class CollectionPathNode extends SqlNode {
 							walker.getSessionFactoryHelper().requireQueryableCollection( collectionType.getRole() ),
 							referenceName,
 							referencePath,
-							referenceName
+							referenceName,
+							walker
 					);
 				}
 				else {
@@ -171,7 +180,8 @@ public class CollectionPathNode extends SqlNode {
 							walker.getSessionFactoryHelper().requireQueryableCollection( collectionType.getRole() ),
 							referenceName,
 							referencePath,
-							referenceName
+							referenceName,
+							walker
 					);
 				}
 			}
@@ -221,12 +231,13 @@ public class CollectionPathNode extends SqlNode {
 					walker.getSessionFactoryHelper().requireQueryableCollection( collectionType.getRole() ),
 					referenceName,
 					referencePath,
-					mappedPath
+					mappedPath,
+					walker
 			);
 		}
 	}
 
-	public FromElement getCollectionOwnerRef() {
+	public FromElement getCollectionOwnerFromElement() {
 		return ownerFromElement;
 	}
 
@@ -245,4 +256,26 @@ public class CollectionPathNode extends SqlNode {
 	public String getCollectionQueryPath() {
 		return collectionQueryPath;
 	}
+
+	public String[] resolveOwnerKeyColumnExpressions() {
+		final AST ast = walker.getAST();
+		final String ownerTableAlias;
+		if ( ast instanceof DeleteStatement || ast instanceof UpdateStatement ) {
+			ownerTableAlias = ownerFromElement.getTableName();
+		}
+		else {
+			ownerTableAlias = ownerFromElement.getTableAlias();
+		}
+
+		final String lhsPropertyName = collectionDescriptor.getCollectionType().getLHSPropertyName();
+		if ( lhsPropertyName == null ) {
+			return StringHelper.qualify(
+					ownerTableAlias,
+					( (Joinable) collectionDescriptor.getOwnerEntityPersister() ).getKeyColumnNames()
+			);
+		}
+		else {
+			return ownerFromElement.toColumns( ownerTableAlias, lhsPropertyName, true );
+		}
+	}
 }

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/CollectionSizeNode.java

@@ -9,19 +9,14 @@ package org.hibernate.hql.internal.ast.tree;
 import org.hibernate.AssertionFailure;
 import org.hibernate.hql.internal.NameGenerator;
 import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
-import org.hibernate.hql.internal.ast.HqlSqlWalker;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.persister.collection.CollectionPropertyMapping;
 import org.hibernate.persister.collection.CollectionPropertyNames;
 import org.hibernate.persister.collection.QueryableCollection;
-import org.hibernate.persister.entity.Joinable;
 import org.hibernate.type.StandardBasicTypes;
 
 import org.jboss.logging.Logger;
 
-import antlr.SemanticException;
-import antlr.collections.AST;
-
 /**
  * @author Steve Ebersole
  */
@@ -31,13 +26,10 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 	private final CollectionPathNode collectionPathNode;
 	private final CollectionPropertyMapping collectionPropertyMapping;
 
-	private final HqlSqlWalker walker;
 	private String alias;
 
-	public CollectionSizeNode(CollectionPathNode collectionPathNode, HqlSqlWalker walker) {
+	public CollectionSizeNode(CollectionPathNode collectionPathNode) {
 		this.collectionPathNode = collectionPathNode;
-		this.walker = walker;
-
 		this.collectionPropertyMapping = new CollectionPropertyMapping( (QueryableCollection) collectionPathNode.getCollectionDescriptor() );
 
 		setType( HqlSqlTokenTypes.COLL_SIZE );
@@ -45,15 +37,12 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 		setText( "collection-size" );
 	}
 
+	@SuppressWarnings("unused")
 	public CollectionPathNode getCollectionPathNode() {
 		return collectionPathNode;
 	}
 
-	public HqlSqlWalker getWalker() {
+	public String toSqlExpression() {
-		return walker;
-	}
-
-	public String toSqlExpression() throws SemanticException {
 		// generate subquery in the form:
 		//
 		// select count( alias_.<collection-size-columns> )
@@ -67,36 +56,10 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 		//		<owner-key-column>			=> ???
 
 
-		final FromElement collectionOwnerFromElement = collectionPathNode.getCollectionOwnerRef();
+		final String[] ownerKeyColumns = collectionPathNode.resolveOwnerKeyColumnExpressions();
+
+		final FromElement collectionOwnerFromElement = collectionPathNode.getCollectionOwnerFromElement();
 		final QueryableCollection collectionDescriptor = (QueryableCollection) collectionPathNode.getCollectionDescriptor();
-		final String collectionPropertyName = collectionPathNode.getCollectionPropertyName();
-
-		getWalker().addQuerySpaces( collectionDescriptor.getCollectionSpaces() );
-
-		// silly : need to prime `SessionFactoryHelper#collectionPropertyMappingByRole`
-		walker.getSessionFactoryHelper().requireQueryableCollection( collectionDescriptor.getRole() );
-
-		// owner-key
-		final String[] ownerKeyColumns;
-		final AST ast = walker.getAST();
-		final String ownerTableAlias;
-		if ( ast instanceof DeleteStatement || ast instanceof UpdateStatement ) {
-			ownerTableAlias = collectionOwnerFromElement.getTableName();
-		}
-		else {
-			ownerTableAlias = collectionOwnerFromElement.getTableAlias();
-		}
-
-		final String lhsPropertyName = collectionDescriptor.getCollectionType().getLHSPropertyName();
-		if ( lhsPropertyName == null ) {
-			ownerKeyColumns = StringHelper.qualify(
-					ownerTableAlias,
-					( (Joinable) collectionDescriptor.getOwnerEntityPersister() ).getKeyColumnNames()
-			);
-		}
-		else {
-			ownerKeyColumns = collectionOwnerFromElement.toColumns( ownerTableAlias, lhsPropertyName, true );
-		}
 
 		// collection-key
 		final String collectionTableAlias = collectionOwnerFromElement.getFromClause()
@@ -104,15 +67,10 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 				.createName( collectionPathNode.getCollectionPropertyName() );
 		final String[] collectionKeyColumns = StringHelper.qualify( collectionTableAlias, collectionDescriptor.getKeyColumnNames() );
 
-
 		if ( collectionKeyColumns.length != ownerKeyColumns.length ) {
 			throw new AssertionFailure( "Mismatch between collection key columns" );
 		}
 
-		// PropertyMapping(c).toColumns(customers)
-		// PropertyMapping(c.customers).toColumns(SIZE)
-
-		// size expression (the count function)
 		final String[] sizeColumns = this.collectionPropertyMapping.toColumns(
 				collectionTableAlias,
 				CollectionPropertyNames.COLLECTION_SIZE
@@ -159,13 +117,13 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 	// SelectExpression
 
 	@Override
-	public void setScalarColumnText(int i) throws SemanticException {
+	public void setScalarColumnText(int i) {
 		log.debugf( "setScalarColumnText(%s)", i );
 		scalarName = NameGenerator.scalarName( i, 0 );
 	}
 
 	@Override
-	public void setScalarColumn(int i) throws SemanticException {
+	public void setScalarColumn(int i) {
 		log.debugf( "setScalarColumn(%s)", i );
 		setScalarColumnText( i );
 	}
@@ -186,12 +144,12 @@ public class CollectionSizeNode extends SqlNode implements SelectExpression {
 	}
 
 	@Override
-	public boolean isReturnableEntity() throws SemanticException {
+	public boolean isReturnableEntity() {
 		return false;
 	}
 
 	@Override
-	public boolean isScalar() throws SemanticException {
+	public boolean isScalar() {
 		return false;
 	}