HHH-16815 fix generic signature of ascending() / descending()

+ code cleanups in the Query hierarchy

hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureCallImpl.java

@@ -1065,12 +1065,12 @@ public class ProcedureCallImpl<R>
 	}
 
 	@Override
-	public Query<R> ascending(SingularAttribute<R, ?> attribute) {
+	public Query<R> ascending(SingularAttribute<? super R, ?> attribute) {
 		throw new UnsupportedOperationException( "Not supported for procedure calls" );
 	}
 
 	@Override
-	public Query<R> descending(SingularAttribute<R, ?> attribute) {
+	public Query<R> descending(SingularAttribute<? super R, ?> attribute) {
 		throw new UnsupportedOperationException( "Not supported for procedure calls" );
 	}
 

hibernate-core/src/main/java/org/hibernate/query/Query.java

@@ -900,10 +900,10 @@ public interface Query<R> extends SelectionQuery<R>, MutationQuery, TypedQuery<R
 	Query<R> setLockMode(LockModeType lockMode);
 
 	@Override
-	Query<R> ascending(SingularAttribute<R, ?> attribute);
+	Query<R> ascending(SingularAttribute<? super R, ?> attribute);
 
 	@Override
-	Query<R> descending(SingularAttribute<R, ?> attribute);
+	Query<R> descending(SingularAttribute<? super R, ?> attribute);
 
 	@Override
 	Query<R> unordered();

hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java

@@ -472,10 +472,10 @@ public interface SelectionQuery<R> extends CommonQueryContract {
 	SelectionQuery<R> setLockMode(String alias, LockMode lockMode);
 
 	@Incubating
-	SelectionQuery<R> ascending(SingularAttribute<R, ?> attribute);
+	SelectionQuery<R> ascending(SingularAttribute<? super R, ?> attribute);
 
 	@Incubating
-	SelectionQuery<R> descending(SingularAttribute<R, ?> attribute);
+	SelectionQuery<R> descending(SingularAttribute<? super R, ?> attribute);
 
 	@Incubating
 	SelectionQuery<R> unordered();

hibernate-core/src/main/java/org/hibernate/query/hql/spi/SqmQueryImplementor.java

@@ -125,18 +125,18 @@ public interface SqmQueryImplementor<R> extends QueryImplementor<R>, SqmQuery, N
 	SqmQueryImplementor<R> setLockMode(LockModeType lockMode);
 
 	@Override
-	default SqmQueryImplementor<R> ascending(SingularAttribute<R, ?> attribute) {
+	default SqmQueryImplementor<R> ascending(SingularAttribute<? super R, ?> attribute) {
 		addOrdering( attribute, SortOrder.ASCENDING );
 		return this;
 	}
 
 	@Override
-	default SqmQueryImplementor<R> descending(SingularAttribute<R, ?> attribute) {
+	default SqmQueryImplementor<R> descending(SingularAttribute<? super R, ?> attribute) {
 		addOrdering( attribute, SortOrder.DESCENDING );
 		return this;
 	}
 
-	SqmQueryImplementor<R> addOrdering(SingularAttribute<R, ?> attribute, SortOrder order);
+	SqmQueryImplementor<R> addOrdering(SingularAttribute<? super R, ?> attribute, SortOrder order);
 
 	@Override
 	SqmQueryImplementor<R> unordered();

hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java

@@ -71,6 +71,7 @@ import org.hibernate.sql.exec.internal.CallbackImpl;
 import org.hibernate.sql.exec.spi.Callback;
 import org.hibernate.sql.results.internal.TupleMetadata;
 import org.hibernate.type.BasicType;
+import org.hibernate.type.BasicTypeRegistry;
 import org.hibernate.type.descriptor.java.JavaType;
 import org.hibernate.type.descriptor.java.spi.PrimitiveJavaType;
 import org.hibernate.type.descriptor.jdbc.JdbcType;
@@ -138,52 +139,52 @@ public abstract class AbstractSelectionQuery<R>
 	}
 
 	private static TupleElement<?>[] buildTupleElementArray(List<SqmSelection<?>> selections) {
-		final TupleElement<?>[] elements;
 		if ( selections.size() == 1 ) {
 			final SqmSelectableNode<?> selectableNode = selections.get(0).getSelectableNode();
 			if ( selectableNode instanceof CompoundSelection<?> ) {
 				final List<? extends JpaSelection<?>> selectionItems = selectableNode.getSelectionItems();
-				elements  = new TupleElement<?>[ selectionItems.size() ];
+				final TupleElement<?>[] elements = new TupleElement<?>[ selectionItems.size() ];
 				for ( int i = 0; i < selectionItems.size(); i++ ) {
 					elements[i] = selectionItems.get( i );
 				}
+				return elements;
 			}
 			else {
-				elements = new TupleElement<?>[] { selectableNode };
+				return new TupleElement<?>[] { selectableNode };
 			}
 		}
 		else {
-			elements = new TupleElement<?>[ selections.size() ];
+			final TupleElement<?>[] elements = new TupleElement<?>[ selections.size() ];
 			for ( int i = 0; i < selections.size(); i++ ) {
 				elements[i] = selections.get( i ).getSelectableNode();
 			}
-		}
 			return elements;
 		}
+	}
 
 	private static String[] buildTupleAliasArray(List<SqmSelection<?>> selections) {
-		final String[] elements;
 		if ( selections.size() == 1 ) {
 			final SqmSelectableNode<?> selectableNode = selections.get(0).getSelectableNode();
 			if ( selectableNode instanceof CompoundSelection<?> ) {
 				final List<? extends JpaSelection<?>> selectionItems = selectableNode.getSelectionItems();
-				elements  = new String[ selectionItems.size() ];
+				final String[] elements  = new String[ selectionItems.size() ];
 				for ( int i = 0; i < selectionItems.size(); i++ ) {
 					elements[i] = selectionItems.get( i ).getAlias();
 				}
+				return elements;
 			}
 			else {
-				elements = new String[] { selectableNode.getAlias() };
+				return new String[] { selectableNode.getAlias() };
 			}
 		}
 		else {
-			elements = new String[ selections.size() ];
+			final String[] elements = new String[ selections.size() ];
 			for ( int i = 0; i < selections.size(); i++ ) {
 				elements[i] = selections.get( i ).getAlias();
 			}
-		}
 			return elements;
 		}
+	}
 
 	protected void applyOptions(NamedSqmQueryMemento memento) {
 		applyOptions( (NamedQueryMemento) memento );
@@ -197,14 +198,13 @@ public abstract class AbstractSelectionQuery<R>
 		}
 
 		if ( memento.getParameterTypes() != null ) {
+			final BasicTypeRegistry basicTypeRegistry =
+					getSessionFactory().getTypeConfiguration().getBasicTypeRegistry();
 			for ( Map.Entry<String, String> entry : memento.getParameterTypes().entrySet() ) {
-				final QueryParameterImplementor<?> parameter =
-						getParameterMetadata().getQueryParameter( entry.getKey() );
 				final BasicType<?> type =
-						getSessionFactory().getTypeConfiguration()
+						basicTypeRegistry.getRegisteredType( entry.getValue() );
-								.getBasicTypeRegistry()
+				getParameterMetadata()
-								.getRegisteredType( entry.getValue() );
+						.getQueryParameter( entry.getKey() ).applyAnticipatedType( type );
-				parameter.applyAnticipatedType( type );
 			}
 		}
 	}
@@ -270,11 +270,10 @@ public abstract class AbstractSelectionQuery<R>
 				}
 				// if there is a single root, use that as the selection
 				if ( sqmRoots.size() == 1 ) {
-					final SqmRoot<?> sqmRoot = sqmRoots.get( 0 );
+					sqmQuerySpec.getSelectClause().add( sqmRoots.get( 0 ), null );
-					sqmQuerySpec.getSelectClause().add( sqmRoot, null );
 				}
 				else {
-					throw new IllegalArgumentException(  );
+					throw new IllegalArgumentException( "Criteria has multiple query roots" );
 				}
 			}
 
@@ -313,11 +312,7 @@ public abstract class AbstractSelectionQuery<R>
 					}
 				}
 
-				final boolean jpaQueryComplianceEnabled =
+				if ( !sessionFactory.getSessionFactoryOptions().getJpaCompliance().isJpaQueryComplianceEnabled() ) {
-						sessionFactory.getSessionFactoryOptions()
-								.getJpaCompliance()
-								.isJpaQueryComplianceEnabled();
-				if ( !jpaQueryComplianceEnabled ) {
 					verifyResultType( expectedResultClass, sqmSelection.getNodeType() );
 				}
 			}
@@ -347,16 +342,15 @@ public abstract class AbstractSelectionQuery<R>
 		final Class<?> javaTypeClass = expressibleJavaType.getJavaTypeClass();
 		if ( !resultClass.isAssignableFrom( javaTypeClass ) ) {
 			if ( expressibleJavaType instanceof PrimitiveJavaType ) {
-				if ( ( (PrimitiveJavaType<?>) expressibleJavaType ).getPrimitiveClass() != resultClass ) {
+				final PrimitiveJavaType<?> javaType = (PrimitiveJavaType<?>) expressibleJavaType;
+				if ( javaType.getPrimitiveClass() != resultClass ) {
 					throwQueryTypeMismatchException( resultClass, sqmExpressible );
 				}
 			}
-			else if ( isMatchingDateType( javaTypeClass, resultClass, sqmExpressible ) ) {
+			else if ( !isMatchingDateType( javaTypeClass, resultClass, sqmExpressible ) ) {
-				// special case, we are good
-			}
-			else {
 				throwQueryTypeMismatchException( resultClass, sqmExpressible );
 			}
+			// else special case, we are good
 		}
 	}
 
@@ -375,7 +369,8 @@ public abstract class AbstractSelectionQuery<R>
 			return ( (BasicDomainType<?>) sqmExpressible).getJdbcType();
 		}
 		else if ( sqmExpressible instanceof SqmPathSource<?> ) {
-			final DomainType<?> domainType = ( (SqmPathSource<?>) sqmExpressible).getSqmPathType();
+			final SqmPathSource<?> pathSource = (SqmPathSource<?>) sqmExpressible;
+			final DomainType<?> domainType = pathSource.getSqmPathType();
 			if ( domainType instanceof BasicDomainType<?> ) {
 				return ( (BasicDomainType<?>) domainType ).getJdbcType();
 			}

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

@@ -1501,12 +1501,12 @@ public class NativeQueryImpl<R>
 	}
 
 	@Override
-	public Query<R> ascending(SingularAttribute<R, ?> attribute) {
+	public Query<R> ascending(SingularAttribute<? super R, ?> attribute) {
 		throw new UnsupportedOperationException("Not yet supported for native queries");
 	}
 
 	@Override
-	public Query<R> descending(SingularAttribute<R, ?> attribute) {
+	public Query<R> descending(SingularAttribute<? super R, ?> attribute) {
 		throw new UnsupportedOperationException("Not yet supported for native queries");
 	}
 

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

@@ -27,6 +27,7 @@ import org.hibernate.LockMode;
 import org.hibernate.LockOptions;
 import org.hibernate.ScrollMode;
 import org.hibernate.TypeMismatchException;
+import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.engine.query.spi.EntityGraphQueryHint;
 import org.hibernate.engine.spi.LoadQueryInfluencers;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
@@ -123,7 +124,10 @@ import static org.hibernate.jpa.SpecHints.HINT_SPEC_CACHE_RETRIEVE_MODE;
 import static org.hibernate.jpa.SpecHints.HINT_SPEC_CACHE_STORE_MODE;
 import static org.hibernate.query.spi.SqlOmittingQueryOptions.omitSqlQueryOptions;
 import static org.hibernate.query.spi.SqlOmittingQueryOptions.omitSqlQueryOptionsWithUniqueSemanticFilter;
+import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.createInterpretationsKey;
+import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.generateNonSelectKey;
 import static org.hibernate.query.sqm.internal.SqmUtil.isSelect;
+import static org.hibernate.query.sqm.internal.SqmUtil.verifyIsNonSelectStatement;
 
 /**
  * {@link Query} implementation based on an SQM
@@ -158,8 +162,7 @@ public class QuerySqmImpl<R>
 		this.hql = memento.getHqlString();
 		this.resultType = expectedResultType;
 
-		final SessionFactoryImplementor factory = session.getFactory();
+		final QueryEngine queryEngine = session.getFactory().getQueryEngine();
-		final QueryEngine queryEngine = factory.getQueryEngine();
 		final QueryInterpretationCache interpretationCache = queryEngine.getInterpretationCache();
 		final HqlInterpretation hqlInterpretation = interpretationCache.resolveHqlInterpretation(
 				hql,
@@ -225,39 +228,32 @@ public class QuerySqmImpl<R>
 			Class<R> expectedResultType,
 			SharedSessionContractImplementor producer) {
 		super( producer );
-		this.hql = CRITERIA_HQL_STRING;
+		hql = CRITERIA_HQL_STRING;
 		if ( producer.isCriteriaCopyTreeEnabled() ) {
-			this.sqm = criteria.copy( SqmCopyContext.simpleContext() );
+			sqm = criteria.copy( SqmCopyContext.simpleContext() );
 		}
 		else {
-			this.sqm = criteria;
+			sqm = criteria;
 			// Cache immutable query plans by default
 			setQueryPlanCacheable( true );
 		}
 
 		setComment( hql );
 
-		this.domainParameterXref = DomainParameterXref.from( this.sqm );
+		domainParameterXref = DomainParameterXref.from( sqm );
 		if ( ! domainParameterXref.hasParameters() ) {
-			this.parameterMetadata = ParameterMetadataImpl.EMPTY;
+			parameterMetadata = ParameterMetadataImpl.EMPTY;
 		}
 		else {
-			this.parameterMetadata = new ParameterMetadataImpl( domainParameterXref.getQueryParameters() );
+			parameterMetadata = new ParameterMetadataImpl( domainParameterXref.getQueryParameters() );
 		}
 
-		this.parameterBindings = QueryParameterBindingsImpl.from( parameterMetadata, producer.getFactory() );
+		parameterBindings = QueryParameterBindingsImpl.from( parameterMetadata, producer.getFactory() );
 
 		// Parameters might be created through HibernateCriteriaBuilder.value which we need to bind here
-		for ( SqmParameter<?> sqmParameter : this.domainParameterXref.getParameterResolutions().getSqmParameters() ) {
+		for ( SqmParameter<?> sqmParameter : domainParameterXref.getParameterResolutions().getSqmParameters() ) {
 			if ( sqmParameter instanceof SqmJpaCriteriaParameterWrapper<?> ) {
-				final JpaCriteriaParameter<Object> jpaCriteriaParameter = ( (SqmJpaCriteriaParameterWrapper<Object>) sqmParameter ).getJpaCriteriaParameter();
+				bindCriteriaParameter((SqmJpaCriteriaParameterWrapper<?>) sqmParameter);
-				final Object value = jpaCriteriaParameter.getValue();
-				// We don't set a null value, unless the type is also null which is the case when using HibernateCriteriaBuilder.value
-				if ( value != null || jpaCriteriaParameter.getNodeType() == null ) {
-					// Use the anticipated type for binding the value if possible
-					getQueryParameterBindings().getBinding( jpaCriteriaParameter )
-							.setBindValue( value, jpaCriteriaParameter.getAnticipatedType() );
-				}
 			}
 		}
 
@@ -274,18 +270,13 @@ public class QuerySqmImpl<R>
 		}
 		else {
 			if ( expectedResultType != null ) {
-				throw new IllegalQueryOperationException(
+				throw new IllegalQueryOperationException( "Result type given for a non-SELECT Query", hql, null );
-						"Result type given for a non-SELECT Query",
-						hql,
-						null
-				);
 			}
 			if ( sqm instanceof SqmUpdateStatement<?> ) {
 				final SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqm;
 				verifyImmutableEntityUpdate( CRITERIA_HQL_STRING, updateStatement, producer.getFactory() );
-				if ( updateStatement.getSetClause() == null || updateStatement.getSetClause()
+				if ( updateStatement.getSetClause() == null
-						.getAssignments()
+						|| updateStatement.getSetClause().getAssignments().isEmpty() ) {
-						.isEmpty() ) {
 					throw new IllegalArgumentException( "No assignments specified as part of UPDATE criteria" );
 				}
 			}
@@ -294,8 +285,20 @@ public class QuerySqmImpl<R>
 			}
 		}
 
-		this.resultType = expectedResultType;
+		resultType = expectedResultType;
-		this.tupleMetadata = buildTupleMetadata( criteria, expectedResultType );
+		tupleMetadata = buildTupleMetadata( criteria, expectedResultType );
+	}
+
+	private <T> void bindCriteriaParameter(SqmJpaCriteriaParameterWrapper<T> sqmParameter) {
+		final JpaCriteriaParameter<T> jpaCriteriaParameter = sqmParameter.getJpaCriteriaParameter();
+		final T value = jpaCriteriaParameter.getValue();
+		// We don't set a null value, unless the type is also null which
+		// is the case when using HibernateCriteriaBuilder.value
+		if ( value != null || jpaCriteriaParameter.getNodeType() == null ) {
+			// Use the anticipated type for binding the value if possible
+			getQueryParameterBindings().getBinding( jpaCriteriaParameter )
+					.setBindValue( value, jpaCriteriaParameter.getAnticipatedType() );
+		}
 	}
 
 	private void validateStatement(SqmStatement<R> sqmStatement, Class<R> resultType) {
@@ -304,11 +307,7 @@ public class QuerySqmImpl<R>
 		}
 		else {
 			if ( resultType != null ) {
-				throw new IllegalQueryOperationException(
+				throw new IllegalQueryOperationException( "Result type given for a non-SELECT Query", hql, null );
-						"Result type given for a non-SELECT Query",
-						hql,
-						null
-				);
 			}
 			if ( sqmStatement instanceof SqmUpdateStatement<?> ) {
 				final SqmUpdateStatement<R> updateStatement = (SqmUpdateStatement<R>) sqmStatement;
@@ -325,30 +324,22 @@ public class QuerySqmImpl<R>
 			String hqlString,
 			SqmUpdateStatement<R> sqmStatement,
 			SessionFactoryImplementor factory) {
-		final EntityPersister entityDescriptor = factory.getRuntimeMetamodels()
+		final EntityPersister persister =
-				.getMappingMetamodel()
+				factory.getMappingMetamodel().getEntityDescriptor( sqmStatement.getTarget().getEntityName() );
-				.getEntityDescriptor( sqmStatement.getTarget().getEntityName() );
+		if ( !persister.isMutable() ) {
-		if ( entityDescriptor.isMutable() ) {
+			final ImmutableEntityUpdateQueryHandlingMode mode =
-			return;
+					factory.getSessionFactoryOptions().getImmutableEntityUpdateQueryHandlingMode();
-		}
+			final String querySpaces = Arrays.toString( persister.getQuerySpaces() );
-		final ImmutableEntityUpdateQueryHandlingMode immutableEntityUpdateQueryHandlingMode = factory
+			switch ( mode ) {
-				.getSessionFactoryOptions()
-				.getImmutableEntityUpdateQueryHandlingMode();
-
-		final String querySpaces = Arrays.toString( entityDescriptor.getQuerySpaces() );
-
-		switch ( immutableEntityUpdateQueryHandlingMode ) {
 				case WARNING:
 					LOG.immutableEntityUpdateQuery( hqlString, querySpaces );
 					break;
 				case EXCEPTION:
-				throw new HibernateException(
+					throw new HibernateException( "The query [" + hqlString + "] attempts to update an immutable entity: "
-						"The query: [" + hqlString + "] attempts to update an immutable entity: " + querySpaces
+							+ querySpaces );
-				);
 				default:
-				throw new UnsupportedOperationException(
+					throw new UnsupportedOperationException( "The " + mode + " is not supported" );
-						"The " + immutableEntityUpdateQueryHandlingMode + " is not supported"
+			}
-				);
 		}
 	}
 
@@ -387,7 +378,8 @@ public class QuerySqmImpl<R>
 		}
 		else {
 			final SqmInsertSelectStatement<R> statement = (SqmInsertSelectStatement<R>) sqmStatement;
-			final List<SqmSelectableNode<?>> selections = statement.getSelectQueryPart()
+			final List<SqmSelectableNode<?>> selections =
+					statement.getSelectQueryPart()
 							.getFirstQuerySpec()
 							.getSelectClause()
 							.getSelectionItems();
@@ -515,14 +507,15 @@ public class QuerySqmImpl<R>
 		verifySelect();
 
 		final SqmSelectStatement<?> sqmStatement = (SqmSelectStatement<?>) getSqmStatement();
-		final boolean containsCollectionFetches = sqmStatement.containsCollectionFetches() || AppliedGraphs.containsCollectionFetches(
+		final boolean containsCollectionFetches =
-				getQueryOptions() );
+				sqmStatement.containsCollectionFetches()
+						|| AppliedGraphs.containsCollectionFetches( getQueryOptions() );
 		final boolean hasLimit = hasLimit( sqmStatement, getQueryOptions() );
 		final boolean needsDistinct = containsCollectionFetches
-				&& ( sqmStatement.usesDistinct() || hasAppliedGraph( getQueryOptions() ) || hasLimit );
+				&& ( hasLimit || sqmStatement.usesDistinct() || hasAppliedGraph( getQueryOptions() ) );
 
 		final List<R> list = resolveSelectQueryPlan()
-				.performList( executionContextFordoList( containsCollectionFetches, hasLimit, needsDistinct ) );
+				.performList( executionContextForDoList( containsCollectionFetches, hasLimit, needsDistinct ) );
 
 		if ( needsDistinct ) {
 			final int first = !hasLimit || getQueryOptions().getLimit().getFirstRow() == null
@@ -532,52 +525,54 @@ public class QuerySqmImpl<R>
 					? getMaxRows( sqmStatement, list.size() )
 					: getQueryOptions().getLimit().getMaxRows();
 			if ( first > 0 || max != -1 ) {
-				final int toIndex;
 				final int resultSize = list.size();
-				if ( max != -1 ) {
+				final int toIndex = max != -1 ? first + max : resultSize;
-					toIndex = first + max;
-				}
-				else {
-					toIndex = resultSize;
-				}
 				if ( first > resultSize ) {
 					return new ArrayList<>(0);
 				}
-				return list.subList( first, toIndex > resultSize ? resultSize : toIndex );
+				return list.subList( first, Math.min( toIndex, resultSize ) );
 			}
 		}
 		return list;
 
 	}
 
-	protected DomainQueryExecutionContext executionContextFordoList(boolean containsCollectionFetches, boolean hasLimit, boolean needsDistinct) {
+	protected DomainQueryExecutionContext executionContextForDoList(boolean containsCollectionFetches, boolean hasLimit, boolean needsDistinct) {
-		final DomainQueryExecutionContext executionContextToUse;
+		final MutableQueryOptions originalQueryOptions;
+		final QueryOptions normalizedQueryOptions;
 		if ( hasLimit && containsCollectionFetches ) {
-			boolean fail = getSessionFactory().getSessionFactoryOptions().isFailOnPaginationOverCollectionFetchEnabled();
+			if ( getSessionFactory().getSessionFactoryOptions().isFailOnPaginationOverCollectionFetchEnabled() ) {
-			if (fail) {
 				throw new HibernateException(
-						"firstResult/maxResults specified with collection fetch. " +
+						"setFirstResult() or setMaxResults() specified with collection fetch join "
-								"In memory pagination was about to be applied. " +
+								+ "(in-memory pagination was about to be applied, but '"
-								"Failing because 'Fail on pagination over collection fetch' is enabled."
+								+ AvailableSettings.FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH
+								+ "' is enabled)"
 				);
 			}
 			else {
 				QueryLogging.QUERY_MESSAGE_LOGGER.firstOrMaxResultsSpecifiedWithCollectionFetch();
 			}
 
-			final MutableQueryOptions originalQueryOptions = getQueryOptions();
+			originalQueryOptions = getQueryOptions();
-			final QueryOptions normalizedQueryOptions;
+			normalizedQueryOptions = needsDistinct
+					? omitSqlQueryOptionsWithUniqueSemanticFilter( originalQueryOptions, true, false )
+					: omitSqlQueryOptions( originalQueryOptions, true, false );
+		}
+		else {
 			if ( needsDistinct ) {
-				normalizedQueryOptions = omitSqlQueryOptionsWithUniqueSemanticFilter( originalQueryOptions, true, false );
+				originalQueryOptions = getQueryOptions();
+				normalizedQueryOptions = uniqueSemanticQueryOptions( originalQueryOptions );
 			}
 			else {
-				normalizedQueryOptions = omitSqlQueryOptions( originalQueryOptions, true, false );
+				return this;
 			}
+		}
+
 		if ( originalQueryOptions == normalizedQueryOptions ) {
-				executionContextToUse = this;
+			return this;
 		}
 		else {
-				executionContextToUse = new DelegatingDomainQueryExecutionContext( this ) {
+			return new DelegatingDomainQueryExecutionContext( this ) {
 				@Override
 				public QueryOptions getQueryOptions() {
 					return normalizedQueryOptions;
@@ -585,46 +580,17 @@ public class QuerySqmImpl<R>
 			};
 		}
 	}
-		else {
-			if ( needsDistinct ) {
-				final MutableQueryOptions originalQueryOptions = getQueryOptions();
-				final QueryOptions normalizedQueryOptions = uniqueSemanticQueryOptions( originalQueryOptions );
-				if ( originalQueryOptions == normalizedQueryOptions ) {
-					executionContextToUse = this;
-				}
-				else {
-					executionContextToUse = new DelegatingDomainQueryExecutionContext( this ) {
-						@Override
-						public QueryOptions getQueryOptions() {
-							return normalizedQueryOptions;
-						}
-					};
-				}
-			}
-			else {
-				executionContextToUse = this;
-			}
-		}
-		return executionContextToUse;
-	}
 
 	public static QueryOptions uniqueSemanticQueryOptions(QueryOptions originalOptions) {
-		final ListResultsConsumer.UniqueSemantic semantic = originalOptions.getUniqueSemantic();
+		return originalOptions.getUniqueSemantic() == ListResultsConsumer.UniqueSemantic.FILTER
-
+				? originalOptions
-
+				: new UniqueSemanticFilterQueryOption( originalOptions );
-		if ( semantic == ListResultsConsumer.UniqueSemantic.FILTER ) {
-			return originalOptions;
 	}
 
-		return new UniqueSemanticFilterQueryOption( originalOptions );
+	private static class UniqueSemanticFilterQueryOption extends DelegatingQueryOptions{
-	}
+		private UniqueSemanticFilterQueryOption(QueryOptions queryOptions) {
-
-	public static class UniqueSemanticFilterQueryOption extends DelegatingQueryOptions{
-
-		public UniqueSemanticFilterQueryOption(QueryOptions queryOptions) {
 			super( queryOptions );
 		}
-
 		@Override
 		public ListResultsConsumer.UniqueSemantic getUniqueSemantic() {
 			return ListResultsConsumer.UniqueSemantic.FILTER;
@@ -632,7 +598,7 @@ public class QuerySqmImpl<R>
 	}
 
 	@Override
-	protected ScrollableResultsImplementor doScroll(ScrollMode scrollMode) {
+	protected ScrollableResultsImplementor<R> doScroll(ScrollMode scrollMode) {
 		return resolveSelectQueryPlan().performScroll( scrollMode, this );
 	}
 
@@ -649,12 +615,10 @@ public class QuerySqmImpl<R>
 	}
 
 	private SelectQueryPlan<R> resolveSelectQueryPlan() {
-		final QueryInterpretationCache.Key cacheKey = SqmInterpretationsKey.createInterpretationsKey( this );
+		final QueryInterpretationCache.Key cacheKey = createInterpretationsKey( this );
 		if ( cacheKey != null ) {
-			return getSession().getFactory().getQueryEngine().getInterpretationCache().resolveSelectQueryPlan(
+			return getSession().getFactory().getQueryEngine().getInterpretationCache()
-					cacheKey,
+					.resolveSelectQueryPlan( cacheKey, this::buildSelectQueryPlan );
-					this::buildSelectQueryPlan
-			);
 		}
 		else {
 			return buildSelectQueryPlan();
@@ -737,7 +701,7 @@ public class QuerySqmImpl<R>
 
 	protected void verifyUpdate() {
 		try {
-			SqmUtil.verifyIsNonSelectStatement( getSqmStatement(), hql );
+			verifyIsNonSelectStatement( getSqmStatement(), hql );
 		}
 		catch (IllegalQueryOperationException e) {
 			// per JPA
@@ -755,15 +719,17 @@ public class QuerySqmImpl<R>
 
 		NonSelectQueryPlan queryPlan = null;
 
-		final QueryInterpretationCache.Key cacheKey = SqmInterpretationsKey.generateNonSelectKey( this );
+		final QueryInterpretationCache.Key cacheKey = generateNonSelectKey( this );
+		final QueryInterpretationCache interpretationCache =
+				getSession().getFactory().getQueryEngine().getInterpretationCache();
 		if ( cacheKey != null ) {
-			queryPlan = getSession().getFactory().getQueryEngine().getInterpretationCache().getNonSelectQueryPlan( cacheKey );
+			queryPlan = interpretationCache.getNonSelectQueryPlan( cacheKey );
 		}
 
 		if ( queryPlan == null ) {
 			queryPlan = buildNonSelectQueryPlan();
 			if ( cacheKey != null ) {
-				getSession().getFactory().getQueryEngine().getInterpretationCache().cacheNonSelectQueryPlan( cacheKey, queryPlan );
+				interpretationCache.cacheNonSelectQueryPlan( cacheKey, queryPlan );
 			}
 		}
 
@@ -788,34 +754,26 @@ public class QuerySqmImpl<R>
 		throw new UnsupportedOperationException( "Query#executeUpdate for Statements of type [" + getSqmStatement() + "] not supported" );
 	}
 
-	@SuppressWarnings({ "unchecked", "rawtypes" })
 	private NonSelectQueryPlan buildDeleteQueryPlan() {
-		final SqmDeleteStatement[] concreteSqmStatements = QuerySplitter.split(
+		final SqmDeleteStatement<R>[] concreteSqmStatements = QuerySplitter.split(
-				(SqmDeleteStatement) getSqmStatement(),
+				(SqmDeleteStatement<R>) getSqmStatement(),
 				getSessionFactory()
 		);
 
-		if ( concreteSqmStatements.length > 1 ) {
+		return concreteSqmStatements.length > 1
-			return buildAggregatedDeleteQueryPlan( concreteSqmStatements );
+				? buildAggregatedDeleteQueryPlan( concreteSqmStatements )
-		}
+				: buildConcreteDeleteQueryPlan( concreteSqmStatements[0] );
-		else {
-			return buildConcreteDeleteQueryPlan( concreteSqmStatements[0] );
-		}
 	}
 
 	private NonSelectQueryPlan buildConcreteDeleteQueryPlan(@SuppressWarnings("rawtypes") SqmDeleteStatement sqmDelete) {
 		final EntityDomainType<?> entityDomainType = sqmDelete.getTarget().getModel();
 		final String entityNameToDelete = entityDomainType.getHibernateEntityName();
-		final EntityPersister entityDescriptor = getSessionFactory().getRuntimeMetamodels()
+		final EntityPersister persister =
-				.getMappingMetamodel()
+				getSessionFactory().getMappingMetamodel().getEntityDescriptor( entityNameToDelete );
-				.getEntityDescriptor( entityNameToDelete );
+		final SqmMultiTableMutationStrategy multiTableStrategy = persister.getSqmMultiTableMutationStrategy();
-		final SqmMultiTableMutationStrategy multiTableStrategy = entityDescriptor.getSqmMultiTableMutationStrategy();
+		return multiTableStrategy == null
-		if ( multiTableStrategy == null ) {
+				? new SimpleDeleteQueryPlan( persister, sqmDelete, domainParameterXref )
-			return new SimpleDeleteQueryPlan( entityDescriptor, sqmDelete, domainParameterXref );
+				: new MultiTableDeleteQueryPlan( sqmDelete, domainParameterXref, multiTableStrategy );
-		}
-		else {
-			return new MultiTableDeleteQueryPlan( sqmDelete, domainParameterXref, multiTableStrategy );
-		}
 	}
 
 	private NonSelectQueryPlan buildAggregatedDeleteQueryPlan(@SuppressWarnings("rawtypes") SqmDeleteStatement[] concreteSqmStatements) {
@@ -829,64 +787,46 @@ public class QuerySqmImpl<R>
 	}
 
 	private NonSelectQueryPlan buildUpdateQueryPlan() {
-		//noinspection rawtypes
+		final SqmUpdateStatement<R> sqmUpdate = (SqmUpdateStatement<R>) getSqmStatement();
-		final SqmUpdateStatement sqmUpdate = (SqmUpdateStatement) getSqmStatement();
 
 		final String entityNameToUpdate = sqmUpdate.getTarget().getModel().getHibernateEntityName();
-		final EntityPersister entityDescriptor = getSessionFactory().getRuntimeMetamodels()
+		final EntityPersister persister =
-				.getMappingMetamodel()
+				getSessionFactory().getMappingMetamodel().getEntityDescriptor( entityNameToUpdate );
-				.getEntityDescriptor( entityNameToUpdate );
 
-		final SqmMultiTableMutationStrategy multiTableStrategy = entityDescriptor.getSqmMultiTableMutationStrategy();
+		final SqmMultiTableMutationStrategy multiTableStrategy = persister.getSqmMultiTableMutationStrategy();
-		if ( multiTableStrategy == null ) {
+		return multiTableStrategy == null
-			return new SimpleUpdateQueryPlan( sqmUpdate, domainParameterXref );
+				? new SimpleUpdateQueryPlan( sqmUpdate, domainParameterXref )
-		}
+				: new MultiTableUpdateQueryPlan( sqmUpdate, domainParameterXref, multiTableStrategy );
-		else {
-			return new MultiTableUpdateQueryPlan( sqmUpdate, domainParameterXref, multiTableStrategy );
-		}
 	}
 
 	private NonSelectQueryPlan buildInsertQueryPlan() {
-		//noinspection rawtypes
+		final SqmInsertStatement<R> sqmInsert = (SqmInsertStatement<R>) getSqmStatement();
-		final SqmInsertStatement sqmInsert = (SqmInsertStatement) getSqmStatement();
 
 		final String entityNameToInsert = sqmInsert.getTarget().getModel().getHibernateEntityName();
-		final AbstractEntityPersister entityDescriptor = (AbstractEntityPersister) getSessionFactory().getRuntimeMetamodels()
+		final AbstractEntityPersister persister = (AbstractEntityPersister)
-				.getMappingMetamodel()
+				getSessionFactory().getMappingMetamodel().getEntityDescriptor( entityNameToInsert );
-				.getEntityDescriptor( entityNameToInsert );
 
-		boolean useMultiTableInsert = entityDescriptor.isMultiTable();
+		boolean useMultiTableInsert = persister.isMultiTable();
-		if ( !useMultiTableInsert && !isSimpleValuesInsert( sqmInsert, entityDescriptor ) ) {
+		if ( !useMultiTableInsert && !isSimpleValuesInsert( sqmInsert, persister ) ) {
-			final Generator identifierGenerator = entityDescriptor.getGenerator();
+			final Generator identifierGenerator = persister.getGenerator();
 			if ( identifierGenerator instanceof BulkInsertionCapableIdentifierGenerator
 					&& identifierGenerator instanceof OptimizableGenerator ) {
 				final Optimizer optimizer = ( (OptimizableGenerator) identifierGenerator ).getOptimizer();
 				if ( optimizer != null && optimizer.getIncrementSize() > 1 ) {
-					useMultiTableInsert = !hasIdentifierAssigned( sqmInsert, entityDescriptor );
+					useMultiTableInsert = !hasIdentifierAssigned( sqmInsert, persister );
 				}
 			}
 		}
-		if ( !useMultiTableInsert ) {
+		return !useMultiTableInsert
-			return new SimpleInsertQueryPlan( sqmInsert, domainParameterXref );
+				? new SimpleInsertQueryPlan( sqmInsert, domainParameterXref )
-		}
+				: new MultiTableInsertQueryPlan( sqmInsert, domainParameterXref, persister.getSqmMultiTableInsertStrategy() );
-		else {
-			return new MultiTableInsertQueryPlan(
-					sqmInsert,
-					domainParameterXref,
-					entityDescriptor.getSqmMultiTableInsertStrategy()
-			);
-		}
 	}
 
 	protected boolean hasIdentifierAssigned(SqmInsertStatement<?> sqmInsert, EntityPersister entityDescriptor) {
 		final EntityIdentifierMapping identifierMapping = entityDescriptor.getIdentifierMapping();
-		final String partName;
+		final String partName = identifierMapping instanceof SingleAttributeIdentifierMapping
-		if ( identifierMapping instanceof SingleAttributeIdentifierMapping ) {
+				? identifierMapping.getAttributeName()
-			partName = ( (SingleAttributeIdentifierMapping) identifierMapping ).getAttributeName();
+				: EntityIdentifierMapping.ROLE_LOCAL_NAME;
-		}
-		else {
-			partName = EntityIdentifierMapping.ROLE_LOCAL_NAME;
-		}
 		for ( SqmPath<?> insertionTargetPath : sqmInsert.getInsertionTargetPaths() ) {
 			final SqmPath<?> lhs = insertionTargetPath.getLhs();
 			if ( !( lhs instanceof SqmRoot<?> ) ) {
@@ -1000,7 +940,7 @@ public class QuerySqmImpl<R>
 	}
 
 	@Override
-	public SqmQueryImplementor<R> addOrdering(SingularAttribute<R, ?> attribute, SortOrder order) {
+	public SqmQueryImplementor<R> addOrdering(SingularAttribute<? super R, ?> attribute, SortOrder order) {
 		if ( sqm instanceof SqmSelectStatement ) {
 			SqmSelectStatement<R> select = (SqmSelectStatement<R>) sqm;
 			List<Order> orders = new ArrayList<>( select.getOrderList() );
@@ -1123,7 +1063,7 @@ public class QuerySqmImpl<R>
 	@Override
 	public NamedQueryMemento toMemento(String name) {
 		if ( CRITERIA_HQL_STRING.equals( getQueryString() ) ) {
-			final SqmStatement sqmStatement ;
+			final SqmStatement<R> sqmStatement;
 			if ( !getSession().isCriteriaCopyTreeEnabled() ) {
 				sqmStatement = getSqmStatement().copy( SqmCopyContext.simpleContext() );
 			}

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

@@ -29,8 +29,8 @@ import org.hibernate.FlushMode;
 import org.hibernate.HibernateException;
 import org.hibernate.LockMode;
 import org.hibernate.ScrollMode;
+import org.hibernate.cfg.AvailableSettings;
 import org.hibernate.engine.spi.LoadQueryInfluencers;
-import org.hibernate.engine.spi.SessionFactoryImplementor;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.graph.spi.AppliedGraph;
 import org.hibernate.internal.util.collections.IdentitySet;
@@ -85,7 +85,8 @@ import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.createInter
 /**
  * @author Steve Ebersole
  */
-public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implements SqmSelectionQuery<R>, InterpretationsKeySource {
+public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R>
+		implements SqmSelectionQuery<R>, InterpretationsKeySource {
 	private final String hql;
 	private final SqmSelectStatement<R> sqm;
 
@@ -162,8 +163,7 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 		this.expectedResultType = resultType;
 		this.resultType = resultType;
 
-		final SessionFactoryImplementor factory = session.getFactory();
+		final QueryEngine queryEngine = session.getFactory().getQueryEngine();
-		final QueryEngine queryEngine = factory.getQueryEngine();
 		final QueryInterpretationCache interpretationCache = queryEngine.getInterpretationCache();
 		final HqlInterpretation hqlInterpretation = interpretationCache.resolveHqlInterpretation(
 				hql,
@@ -220,16 +220,7 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 		// Parameters might be created through HibernateCriteriaBuilder.value which we need to bind here
 		for ( SqmParameter<?> sqmParameter : domainParameterXref.getParameterResolutions().getSqmParameters() ) {
 			if ( sqmParameter instanceof SqmJpaCriteriaParameterWrapper<?> ) {
-				final JpaCriteriaParameter<Object> jpaCriteriaParameter =
+				bindCriteriaParameter( (SqmJpaCriteriaParameterWrapper<?>) sqmParameter );
-						( (SqmJpaCriteriaParameterWrapper<Object>) sqmParameter ).getJpaCriteriaParameter();
-				final Object value = jpaCriteriaParameter.getValue();
-				// We don't set a null value, unless the type is also null which is the case when using HibernateCriteriaBuilder.value
-				if ( value != null || jpaCriteriaParameter.getNodeType() == null ) {
-					// Use the anticipated type for binding the value if possible
-					getQueryParameterBindings()
-							.getBinding( jpaCriteriaParameter )
-							.setBindValue( value, jpaCriteriaParameter.getAnticipatedType() );
-				}
 			}
 		}
 
@@ -242,6 +233,19 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 		this.tupleMetadata = buildTupleMetadata( sqm, expectedResultType );
 	}
 
+	private <T> void bindCriteriaParameter(SqmJpaCriteriaParameterWrapper<T> sqmParameter) {
+		final JpaCriteriaParameter<T> jpaCriteriaParameter = sqmParameter.getJpaCriteriaParameter();
+		final T value = jpaCriteriaParameter.getValue();
+		// We don't set a null value, unless the type is also null which
+		// is the case when using HibernateCriteriaBuilder.value
+		if ( value != null || jpaCriteriaParameter.getNodeType() == null ) {
+			// Use the anticipated type for binding the value if possible
+			getQueryParameterBindings()
+					.getBinding( jpaCriteriaParameter )
+					.setBindValue( value, jpaCriteriaParameter.getAnticipatedType() );
+		}
+	}
+
 	public TupleMetadata getTupleMetadata() {
 		return tupleMetadata;
 	}
@@ -281,7 +285,7 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 	}
 
 	protected List<R> doList() {
-		final SqmSelectStatement<?> sqmStatement = (SqmSelectStatement<?>) getSqmStatement();
+		final SqmSelectStatement<?> sqmStatement = getSqmStatement();
 		final boolean containsCollectionFetches = sqmStatement.containsCollectionFetches();
 		final boolean hasLimit = hasLimit( sqmStatement, getQueryOptions() );
 		final boolean needsDistinct = containsCollectionFetches
@@ -289,12 +293,12 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 
 		final DomainQueryExecutionContext executionContextToUse;
 		if ( hasLimit && containsCollectionFetches ) {
-			boolean fail = getSessionFactory().getSessionFactoryOptions().isFailOnPaginationOverCollectionFetchEnabled();
+			if ( getSessionFactory().getSessionFactoryOptions().isFailOnPaginationOverCollectionFetchEnabled() ) {
-			if ( fail ) {
 				throw new HibernateException(
-						"firstResult/maxResults specified with collection fetch. " +
+						"setFirstResult() or setMaxResults() specified with collection fetch join "
-								"In memory pagination was about to be applied. " +
+								+ "(in-memory pagination was about to be applied, but '"
-								"Failing because 'Fail on pagination over collection fetch' is enabled."
+								+ AvailableSettings.FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH
+								+ "' is enabled)"
 				);
 			}
 			else {
@@ -342,7 +346,7 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 				}
 				tmp.add( result );
 				// NOTE : ( max - 1 ) because first is zero-based while max is not...
-				if ( max >= 0 && ( includedCount - first ) >= ( max - 1 ) ) {
+				if ( max >= 0 && includedCount - first >= max - 1 ) {
 					break;
 				}
 			}
@@ -552,18 +556,18 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 	}
 
 	@Override
-	public SqmSelectionQuery<R> ascending(SingularAttribute<R, ?> attribute) {
+	public SqmSelectionQuery<R> ascending(SingularAttribute<? super R, ?> attribute) {
 		addOrdering( attribute, SortOrder.ASCENDING );
 		return this;
 	}
 
 	@Override
-	public SqmSelectionQuery<R> descending(SingularAttribute<R, ?> attribute) {
+	public SqmSelectionQuery<R> descending(SingularAttribute<? super R, ?> attribute) {
 		addOrdering( attribute, SortOrder.DESCENDING );
 		return this;
 	}
 
-	public SqmSelectionQuery<R> addOrdering(SingularAttribute<R, ?> attribute, SortOrder order) {
+	private void addOrdering(SingularAttribute<? super R, ?> attribute, SortOrder order) {
 		List<Order> orders = new ArrayList<>( sqm.getOrderList() );
 		sqm.getQuerySpec().getRoots().forEach( root -> {
 			@SuppressWarnings("unchecked")
@@ -574,7 +578,6 @@ public class SqmSelectionQueryImpl<R> extends AbstractSelectionQuery<R> implemen
 
 		} );
 		sqm.orderBy( orders );
-		return this;
 	}
 
 	@Override

hibernate-core/src/test/java/org/hibernate/orm/test/annotations/immutable/ImmutableEntityUpdateQueryHandlingModeExceptionTest.java

@@ -68,7 +68,7 @@ public class ImmutableEntityUpdateQueryHandlingModeExceptionTest extends BaseNon
 		catch (PersistenceException e) {
 			assertTrue( e instanceof HibernateException );
 			assertEquals(
-					"The query: [update Country set name = :name] attempts to update an immutable entity: [Country]",
+					"The query [update Country set name = :name] attempts to update an immutable entity: [Country]",
 					e.getMessage()
 			);
 		}