HHH-17002, HHH-18820, HHH-19391, HHH-18514 equals() and hashCode() for SQM nodes

- finally enables efficient caching of criteria query plans
- also reconsider how alias generation is done - aliases should only
  be unique to a given query, NOT globally unique, since that results
  in interpretation cache misses
- ran into and fixed several other problems along the way
- note that the previous solution based on translating to HQL was not
  working at all, partly because the translation to HQL is not very
  correct - but anyway this is more efficient, since hashCodes are
  in general more flexible from an efficiency perspective
- there is still a remaining problem where NavigablePaths elements
  are assigned globally unique aliases resulting in cache misses

Author: gavinking

hibernate-core/src/main/java/org/hibernate/dialect/function/array/H2UnnestFunction.java

@@ -63,7 +63,6 @@ public H2UnnestFunction(int maximumArraySize) {
 	protected <T> SelfRenderingSqmSetReturningFunction<T> generateSqmSetReturningFunctionExpression(
 			List<? extends SqmTypedNode<?>> arguments,
 			QueryEngine queryEngine) {
-		//noinspection unchecked
 		return new SelfRenderingSqmSetReturningFunction<>(
 				this,
 				this,

hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/AnyDiscriminatorSqmPath.java

@@ -13,6 +13,8 @@
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.spi.NavigablePath;
 
+import java.util.Objects;
+
 public class AnyDiscriminatorSqmPath<T> extends AbstractSqmPath<T> implements DiscriminatorSqmPath<T> {
 
 	protected AnyDiscriminatorSqmPath(
@@ -45,4 +47,16 @@ public <X> X accept(SemanticQueryWalker<X> walker) {
 	public AnyDiscriminatorSqmPathSource<T> getExpressible() {
 		return (AnyDiscriminatorSqmPathSource<T>) getNodeType();
 	}
+
+
+	@Override
+	public boolean equals(Object object) {
+		return object instanceof AnyDiscriminatorSqmPath<?> that
+			&& Objects.equals( this.getLhs(), that.getLhs() );
+	}
+
+	@Override
+	public int hashCode() {
+		return getLhs().hashCode();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/EmbeddedDiscriminatorSqmPath.java

@@ -14,6 +14,8 @@
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.spi.NavigablePath;
 
+import java.util.Objects;
+
 /**
  * {@link SqmPath} specialization for an embeddable discriminator
  *
@@ -59,4 +61,15 @@ public EmbeddedDiscriminatorSqmPath<T> copy(SqmCopyContext context) {
 	public <X> X accept(SemanticQueryWalker<X> walker) {
 		return walker.visitDiscriminatorPath( this );
 	}
+
+	@Override
+	public boolean equals(Object object) {
+		return object instanceof EmbeddedDiscriminatorSqmPath<?> that
+			&& Objects.equals( this.getLhs(), that.getLhs() );
+	}
+
+	@Override
+	public int hashCode() {
+		return getLhs().hashCode();
+	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/EntityDiscriminatorSqmPath.java

@@ -17,6 +17,8 @@
 import org.hibernate.query.sqm.tree.domain.SqmEntityDomainType;
 import org.hibernate.spi.NavigablePath;
 
+import java.util.Objects;
+
 /**
  * {@link SqmPath} specialization for an entity discriminator
  *
@@ -66,10 +68,19 @@ public EntityDiscriminatorSqmPath copy(SqmCopyContext context) {
 
 	@Override
 	public <X> X accept(SemanticQueryWalker<X> walker) {
-		if ( ! entityDescriptor.hasSubclasses() ) {
-			return walker.visitEntityTypeLiteralExpression( new SqmLiteralEntityType( entityDomainType, nodeBuilder() ) );
-		}
+		return entityDescriptor.hasSubclasses()
+				? walker.visitDiscriminatorPath( this )
+				: walker.visitEntityTypeLiteralExpression( new SqmLiteralEntityType( entityDomainType, nodeBuilder() ) );
+	}
 
-		return walker.visitDiscriminatorPath( this );
+	@Override
+	public boolean equals(Object object) {
+		return object instanceof EntityDiscriminatorSqmPath<?> that
+			&& Objects.equals( this.getLhs(), that.getLhs() );
+	}
+
+	@Override
+	public int hashCode() {
+		return getLhs().hashCode();
 	}
 }

hibernate-core/src/main/java/org/hibernate/query/hql/internal/FullyQualifiedReflectivePathTerminal.java

@@ -8,6 +8,7 @@
 import java.math.BigInteger;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Function;
 
 import org.checkerframework.checker.nullness.qual.Nullable;
@@ -163,6 +164,13 @@ public void appendHqlString(StringBuilder hql, SqmRenderContext context) {
 		hql.append( getLocalName() );
 	}
 
+	@Override
+	public boolean equals(Object node) {
+		return node instanceof FullyQualifiedReflectivePathTerminal<?> that
+			&& Objects.equals( getParent().getFullPath(), that.getParent().getFullPath() )
+			&& Objects.equals( getLocalName(), that.getLocalName() );
+	}
+
 	@Override
 	public SqmExpression<Long> asLong() {
 		return null;

hibernate-core/src/main/java/org/hibernate/query/hql/internal/QuerySplitter.java

@@ -4,8 +4,6 @@
  */
 package org.hibernate.query.hql.internal;
 
-import java.util.Set;
-
 import org.hibernate.metamodel.model.domain.EntityDomainType;
 import org.hibernate.query.sqm.tree.SqmCopyContext;
 import org.hibernate.query.sqm.tree.SqmDeleteOrUpdateStatement;
@@ -30,24 +28,24 @@ public static <R> SqmSelectStatement<R>[] split(SqmSelectStatement<R> statement)
 		// We only allow unmapped polymorphism in a very restricted way.  Specifically,
 		// the unmapped polymorphic reference can only be a root and can be the only
 		// root.  Use that restriction to locate the unmapped polymorphic reference
-		final SqmRoot<?> unmappedPolymorphicReference = findUnmappedPolymorphicReference( statement.getQueryPart() );
-
+		final SqmRoot<?> unmappedPolymorphicReference =
+				findUnmappedPolymorphicReference( statement.getQueryPart() );
 		if ( unmappedPolymorphicReference == null ) {
 			@SuppressWarnings("unchecked")
 			SqmSelectStatement<R>[] sqmSelectStatement = new SqmSelectStatement[] { statement };
 			return sqmSelectStatement;
 		}
 
-		final SqmPolymorphicRootDescriptor<R> unmappedPolymorphicDescriptor = (SqmPolymorphicRootDescriptor<R>) unmappedPolymorphicReference.getReferencedPathSource();
-		final Set<EntityDomainType<? extends R>> implementors = unmappedPolymorphicDescriptor.getImplementors();
+		final var unmappedPolymorphicDescriptor =
+				(SqmPolymorphicRootDescriptor<R>)
+						unmappedPolymorphicReference.getReferencedPathSource();
+		var implementors = unmappedPolymorphicDescriptor.getImplementors();
 		@SuppressWarnings("unchecked")
 		final SqmSelectStatement<R>[] expanded = new SqmSelectStatement[ implementors.size() ];
-
 		int i = 0;
 		for ( EntityDomainType<?> mappedDescriptor : implementors ) {
 			expanded[i++] = copyStatement( statement, unmappedPolymorphicReference, mappedDescriptor );
 		}
-
 		return expanded;
 	}
 
@@ -97,31 +95,29 @@ public static <R> SqmDeleteStatement<R>[] split(SqmDeleteStatement<R> statement)
 		// We only allow unmapped polymorphism in a very restricted way.  Specifically,
 		// the unmapped polymorphic reference can only be a root and can be the only
 		// root.  Use that restriction to locate the unmapped polymorphic reference
-		final SqmRoot<?> unmappedPolymorphicReference = findUnmappedPolymorphicReference( statement );
-
+		final SqmRoot<?> unmappedPolymorphicReference =
+				findUnmappedPolymorphicReference( statement );
 		if ( unmappedPolymorphicReference == null ) {
 			@SuppressWarnings("unchecked")
 			SqmDeleteStatement<R>[] sqmDeleteStatement = new SqmDeleteStatement[] { statement };
 			return sqmDeleteStatement;
 		}
 
-		final SqmPolymorphicRootDescriptor<R> unmappedPolymorphicDescriptor =
-				(SqmPolymorphicRootDescriptor<R>) unmappedPolymorphicReference.getReferencedPathSource();
-		final Set<EntityDomainType<? extends R>> implementors = unmappedPolymorphicDescriptor.getImplementors();
+		final var unmappedPolymorphicDescriptor =
+				(SqmPolymorphicRootDescriptor<R>)
+						unmappedPolymorphicReference.getReferencedPathSource();
+		final var implementors = unmappedPolymorphicDescriptor.getImplementors();
 		@SuppressWarnings("unchecked")
 		final SqmDeleteStatement<R>[] expanded = new SqmDeleteStatement[ implementors.size() ];
-
 		int i = 0;
 		for ( EntityDomainType<?> mappedDescriptor : implementors ) {
 			expanded[i++] = copyStatement( statement, unmappedPolymorphicReference, mappedDescriptor );
 		}
-
 		return expanded;
 	}
 
 	private static SqmRoot<?> findUnmappedPolymorphicReference(SqmDeleteOrUpdateStatement<?> queryPart) {
-		return queryPart.getTarget().getReferencedPathSource() instanceof SqmPolymorphicRootDescriptor<?>
-				? queryPart.getTarget()
-				: null;
+		final SqmRoot<?> target = queryPart.getTarget();
+		return target.getReferencedPathSource() instanceof SqmPolymorphicRootDescriptor<?> ? target : null;
 	}
 }

hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java

@@ -1229,7 +1229,7 @@ private SqmFromClause buildInferredFromClause(HqlParser.SelectClauseContext sele
 			final EntityDomainType<R> entityDescriptor = getResultEntity();
 			if ( entityDescriptor != null ) {
 				final SqmRoot<R> sqmRoot =
-						new SqmRoot<>( entityDescriptor, null, false, creationContext.getNodeBuilder() );
+						new SqmRoot<>( entityDescriptor, "_0", false, creationContext.getNodeBuilder() );
 				processingStateStack.getCurrent().getPathRegistry().register( sqmRoot );
 				fromClause.addRoot( sqmRoot );
 			}

hibernate-core/src/main/java/org/hibernate/query/hql/internal/SqmTreeCreationHelper.java

@@ -134,24 +134,21 @@ public class SqmTreeCreationHelper {
 	 */
 	public static <E> void handleRootAsCrossJoin(
 			HqlParser.EntityWithJoinsContext entityWithJoinsContext,
-			SqmRoot<?> sqmPrimaryRoot,
+			SqmRoot<E> sqmPrimaryRoot,
 			SemanticQueryBuilder<?> sqmBuilder) {
-		final HqlParser.RootEntityContext fromRootContext = (HqlParser.RootEntityContext) entityWithJoinsContext.fromRoot();
+		final HqlParser.RootEntityContext fromRootContext =
+				(HqlParser.RootEntityContext) entityWithJoinsContext.fromRoot();
 
 		//noinspection unchecked
 		final SqmRoot<E> sqmRoot = (SqmRoot<E>) fromRootContext.accept( sqmBuilder );
 		SqmTreeCreationLogger.LOGGER.debugf( "Handling secondary root path as cross-join - %s", sqmRoot.getEntityName() );
-
-		final String alias = extractAlias( fromRootContext.variable(), sqmBuilder );
-		final SqmEntityJoin<?,E> pseudoCrossJoin = new SqmEntityJoin<>(
+		final SqmEntityJoin<E,E> pseudoCrossJoin = new SqmEntityJoin<>(
 				sqmRoot.getManagedType(),
-				alias,
+				extractAlias( fromRootContext.variable(), sqmBuilder ),
 				SqmJoinType.CROSS,
 				sqmPrimaryRoot
 		);
-
-		//noinspection unchecked,rawtypes
-		sqmPrimaryRoot.addSqmJoin( (SqmEntityJoin) pseudoCrossJoin );
+		sqmPrimaryRoot.addSqmJoin( pseudoCrossJoin );
 
 		final SqmCreationProcessingState processingState = sqmBuilder.getProcessingStateStack().getCurrent();
 		final SqmPathRegistry pathRegistry = processingState.getPathRegistry();

hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java

@@ -162,7 +162,7 @@ public <P> QueryParameterBinding<P> getBinding(String name) {
 
 	@Override
 	public void validate() {
-		for ( Map.Entry<QueryParameter<?>, QueryParameterBinding<?>> entry : parameterBindingMap.entrySet() ) {
+		for ( var entry : parameterBindingMap.entrySet() ) {
 			if ( !entry.getValue().isBound() ) {
 				final QueryParameter<?> queryParameter = entry.getKey();
 				if ( queryParameter.isNamed() ) {

hibernate-core/src/main/java/org/hibernate/query/sqm/function/SelfRenderingSqmFunction.java

@@ -6,6 +6,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Supplier;
 
 import org.hibernate.metamodel.mapping.BasicValuedMapping;
@@ -256,4 +257,15 @@ public MappingModelExpressible<?> get() {
 		}
 	}
 
+	@Override
+	// TODO: override on all subtypes
+	public boolean equals(Object other) {
+		return other instanceof SelfRenderingSqmAggregateFunction<?> that
+			&& Objects.equals( this.toHqlString(), that.toHqlString() );
+	}
+
+	@Override
+	public int hashCode() {
+		return toHqlString().hashCode();
+	}
 }

hibernate-core/src/main/java/org/hibernate/query/sqm/function/SelfRenderingSqmOrderedSetAggregateFunction.java

@@ -138,28 +138,31 @@ public void appendHqlString(StringBuilder hql, SqmRenderContext context) {
 		final List<? extends SqmTypedNode<?>> arguments = getArguments();
 		hql.append( getFunctionName() );
 		hql.append( '(' );
-		int i = 1;
-		if ( arguments.get( 0 ) instanceof SqmDistinct<?> ) {
-			arguments.get( 0 ).appendHqlString( hql, context );
-			if ( arguments.size() > 1 ) {
-				hql.append( ' ' );
-				arguments.get( 1 ).appendHqlString( hql, context );
-				i = 2;
+		if ( !arguments.isEmpty() ) {
+			int i = 1;
+			if ( arguments.get( 0 ) instanceof SqmDistinct<?> ) {
+				arguments.get( 0 ).appendHqlString( hql, context );
+				if ( arguments.size() > 1 ) {
+					hql.append( ' ' );
+					arguments.get( 1 ).appendHqlString( hql, context );
+					i = 2;
+				}
+			}
+			for ( ; i < arguments.size(); i++ ) {
+				hql.append( ", " );
+				arguments.get( i ).appendHqlString( hql, context );
 			}
 		}
-		for ( ; i < arguments.size(); i++ ) {
-			hql.append(", ");
-			arguments.get( i ).appendHqlString( hql, context );
-		}
-
 		hql.append( ')' );
 		if ( withinGroup != null ) {
 			hql.append( " within group (order by " );
 			final List<SqmSortSpecification> sortSpecifications = withinGroup.getSortSpecifications();
-			sortSpecifications.get( 0 ).appendHqlString( hql, context );
-			for ( int j = 1; j < sortSpecifications.size(); j++ ) {
-				hql.append( ", " );
-				sortSpecifications.get( j ).appendHqlString( hql, context );
+			if ( !sortSpecifications.isEmpty() ) {
+				sortSpecifications.get( 0 ).appendHqlString( hql, context );
+				for ( int j = 1; j < sortSpecifications.size(); j++ ) {
+					hql.append( ", " );
+					sortSpecifications.get( j ).appendHqlString( hql, context );
+				}
 			}
 			hql.append( ')' );
 		}

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

@@ -205,38 +205,21 @@ public QuerySqmImpl(
 			SqmStatement<R> criteria,
 			boolean copyAst,
 			Class<R> expectedResultType,
-			SharedSessionContractImplementor producer) {
-		super( producer );
+			SharedSessionContractImplementor session) {
+		super( session );
 		hql = CRITERIA_HQL_STRING;
-		if ( copyAst ) {
-			sqm = criteria.copy( SqmCopyContext.simpleContext() );
-			if ( producer.isCriteriaPlanCacheEnabled() ) {
-				queryStringCacheKey = sqm.toHqlString();
-				setQueryPlanCacheable( true );
-			}
-			else {
-				queryStringCacheKey = sqm;
-			}
-		}
-		else {
-			sqm = criteria;
-			if ( producer.isCriteriaPlanCacheEnabled() ) {
-				queryStringCacheKey = sqm.toHqlString();
-			}
-			else {
-				queryStringCacheKey = sqm;
-			}
-			// Cache immutable query plans by default
-			setQueryPlanCacheable( true );
-		}
+		sqm = copyAst ? criteria.copy( SqmCopyContext.simpleContext() ) : criteria;
+		queryStringCacheKey = sqm;
+		// Cache immutable query plans by default
+		setQueryPlanCacheable( !copyAst || session.isCriteriaPlanCacheEnabled() );
 
 		setComment( hql );
 
 		domainParameterXref = DomainParameterXref.from( sqm );
 		parameterMetadata = !domainParameterXref.hasParameters()
 				? ParameterMetadataImpl.EMPTY
 				: new ParameterMetadataImpl( domainParameterXref.getQueryParameters() );
-		parameterBindings = parameterMetadata.createBindings( producer.getFactory() );
+		parameterBindings = parameterMetadata.createBindings( session.getFactory() );
 
 		// Parameters might be created through HibernateCriteriaBuilder.value which we need to bind here
 		for ( SqmParameter<?> sqmParameter : domainParameterXref.getParameterResolutions().getSqmParameters() ) {

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

@@ -23,10 +23,9 @@ public SimpleSqmRenderContext() {
 	@Override
 	public String resolveAlias(SqmFrom<?, ?> from) {
 		final String explicitAlias = from.getExplicitAlias();
-		if ( explicitAlias != null ) {
-			return explicitAlias;
-		}
-		return fromAliases.computeIfAbsent( from, f -> "alias_" + (fromId++) );
+		return explicitAlias == null
+				? fromAliases.computeIfAbsent( from, f -> "alias_" + (fromId++) )
+				: explicitAlias;
 	}
 
 	@Override