re-enable tests

re-organize some tests
continuing with o.h.test.hql - order-by

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

@@ -28,14 +28,11 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.Supplier;
 import java.util.regex.Pattern;
 
-import org.hibernate.query.FetchClauseType;
 import org.hibernate.NotYetImplementedFor6Exception;
-import org.hibernate.query.NullPrecedence;
 import org.hibernate.QueryException;
-import org.hibernate.query.SetOperator;
-import org.hibernate.query.SortOrder;
 import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
 import org.hibernate.boot.registry.classloading.spi.ClassLoadingException;
 import org.hibernate.grammars.hql.HqlLexer;
@@ -50,13 +47,16 @@
 import org.hibernate.metamodel.model.domain.EntityDomainType;
 import org.hibernate.metamodel.model.domain.IdentifiableDomainType;
 import org.hibernate.metamodel.model.domain.ManagedDomainType;
-import org.hibernate.metamodel.model.domain.MapPersistentAttribute;
 import org.hibernate.metamodel.model.domain.PluralPersistentAttribute;
 import org.hibernate.metamodel.model.domain.SingularPersistentAttribute;
 import org.hibernate.query.BinaryArithmeticOperator;
 import org.hibernate.query.ComparisonOperator;
+import org.hibernate.query.FetchClauseType;
+import org.hibernate.query.NullPrecedence;
 import org.hibernate.query.PathException;
 import org.hibernate.query.SemanticException;
+import org.hibernate.query.SetOperator;
+import org.hibernate.query.SortOrder;
 import org.hibernate.query.TemporalUnit;
 import org.hibernate.query.TrimSpec;
 import org.hibernate.query.UnaryArithmeticOperator;
@@ -104,6 +104,7 @@
 import org.hibernate.query.sqm.tree.domain.SqmPluralValuedSimplePath;
 import org.hibernate.query.sqm.tree.domain.SqmPolymorphicRootDescriptor;
 import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmAny;
 import org.hibernate.query.sqm.tree.expression.SqmBinaryArithmetic;
 import org.hibernate.query.sqm.tree.expression.SqmByUnit;
@@ -161,6 +162,7 @@
 import org.hibernate.query.sqm.tree.predicate.SqmPredicate;
 import org.hibernate.query.sqm.tree.predicate.SqmWhereClause;
 import org.hibernate.query.sqm.tree.select.AbstractSqmSelectQuery;
+import org.hibernate.query.sqm.tree.select.SqmAliasedNode;
 import org.hibernate.query.sqm.tree.select.SqmDynamicInstantiation;
 import org.hibernate.query.sqm.tree.select.SqmDynamicInstantiationArgument;
 import org.hibernate.query.sqm.tree.select.SqmOrderByClause;
@@ -241,17 +243,19 @@ public static <R> SqmStatement<R> buildSemanticModel(
 	private final Stack<SqmCreationProcessingState> processingStateStack = new StandardStack<>();
 
 	private ParameterCollector parameterCollector;
-
+	private BasicDomainType<Integer> integerDomainType;
-
+	private JavaTypeDescriptor<List<?>> listJavaTypeDescriptor;
-	public Stack<SqmCreationProcessingState> getProcessingStateStack() {
+	private JavaTypeDescriptor<Map<?,?>> mapJavaTypeDescriptor;
-		return processingStateStack;
-	}
 
 	@SuppressWarnings("WeakerAccess")
 	public SemanticQueryBuilder(SqmCreationOptions creationOptions, SqmCreationContext creationContext) {
 		this.creationOptions = creationOptions;
 		this.creationContext = creationContext;
 		this.dotIdentifierConsumerStack = new StandardStack<>( new BasicDotIdentifierConsumer( this ) );
+		this.integerDomainType = creationContext
+				.getNodeBuilder()
+				.getTypeConfiguration()
+				.standardBasicTypeForJavaType( Integer.class );
 	}
 
 	@Override
@@ -264,6 +268,10 @@ public SqmCreationOptions getCreationOptions() {
 		return creationOptions;
 	}
 
+	public Stack<SqmCreationProcessingState> getProcessingStateStack() {
+		return processingStateStack;
+	}
+
 	protected Stack<ParameterDeclarationContext> getParameterDeclarationContextStack() {
 		return parameterDeclarationContextStack;
 	}
@@ -770,28 +778,41 @@ public SqmSelection visitSelection(HqlParser.SelectionContext ctx) {
 		final SqmSelectableNode selectableNode = visitSelectableNode( ctx );
 
 		//noinspection unchecked
-		final SqmSelection selection = new SqmSelection(
+		final SqmSelection<?> selection = new SqmSelection<>(
 				selectableNode,
+				// NOTE : SqmSelection forces the alias down to its selectableNode.
+				//		- no need to do that here
 				resultIdentifier,
 				creationContext.getNodeBuilder()
 		);
 
-		getProcessingStateStack().getCurrent().getPathRegistry().register( selection );
+		// if the node is not a dynamic-instantiation, register it with
+		// the path-registry
+		//noinspection StatementWithEmptyBody
+		if ( selectableNode instanceof SqmDynamicInstantiation ) {
+			// nothing else to do (avoid kludgy `! ( instanceof )` syntax
+		}
+		else {
+			getCurrentProcessingState().getPathRegistry().register( selection );
+		}
 
 		return selection;
 	}
 
-	private SqmSelectableNode visitSelectableNode(HqlParser.SelectionContext ctx) {
+	private SqmSelectableNode<?> visitSelectableNode(HqlParser.SelectionContext ctx) {
 		if ( ctx.selectExpression().dynamicInstantiation() != null ) {
 			return visitDynamicInstantiation( ctx.selectExpression().dynamicInstantiation() );
 		}
-		else if ( ctx.selectExpression().jpaSelectObjectSyntax() != null ) {
+
+		if ( ctx.selectExpression().jpaSelectObjectSyntax() != null ) {
 			return visitJpaSelectObjectSyntax( ctx.selectExpression().jpaSelectObjectSyntax() );
 		}
-		else if ( ctx.selectExpression().mapEntrySelection() != null ) {
+
+		if ( ctx.selectExpression().mapEntrySelection() != null ) {
 			return visitMapEntrySelection( ctx.selectExpression().mapEntrySelection() );
 		}
-		else if ( ctx.selectExpression().expression() != null ) {
+
+		if ( ctx.selectExpression().expression() != null ) {
 			return (SqmExpression) ctx.selectExpression().expression().accept( this );
 		}
 
@@ -829,12 +850,9 @@ public String visitResultIdentifier(HqlParser.ResultIdentifierContext resultIden
 		return null;
 	}
 
-	private JavaTypeDescriptor<List> listJavaTypeDescriptor;
-	private JavaTypeDescriptor<Map> mapJavaTypeDescriptor;
-
 	@Override
-	public SqmDynamicInstantiation visitDynamicInstantiation(HqlParser.DynamicInstantiationContext ctx) {
+	public SqmDynamicInstantiation<?> visitDynamicInstantiation(HqlParser.DynamicInstantiationContext ctx) {
-		final SqmDynamicInstantiation dynamicInstantiation;
+		final SqmDynamicInstantiation<?> dynamicInstantiation;
 
 		if ( ctx.dynamicInstantiationTarget().MAP() != null ) {
 			if ( mapJavaTypeDescriptor == null ) {
@@ -863,7 +881,7 @@ else if ( ctx.dynamicInstantiationTarget().LIST() != null ) {
 		else {
 			final String className = ctx.dynamicInstantiationTarget().dotIdentifierSequence().getText();
 			try {
-				final JavaTypeDescriptor jtd = resolveInstantiationTargetJtd( className );
+				final JavaTypeDescriptor<?> jtd = resolveInstantiationTargetJtd( className );
 				dynamicInstantiation = SqmDynamicInstantiation.forClassInstantiation(
 						jtd,
 						creationContext.getNodeBuilder()
@@ -881,7 +899,7 @@ else if ( ctx.dynamicInstantiationTarget().LIST() != null ) {
 		return dynamicInstantiation;
 	}
 
-	private JavaTypeDescriptor resolveInstantiationTargetJtd(String className) {
+	private JavaTypeDescriptor<?> resolveInstantiationTargetJtd(String className) {
 		final Class<?> targetJavaType = classForName( creationContext.getJpaMetamodel().qualifyImportableName( className ) );
 		return creationContext.getJpaMetamodel()
 				.getTypeConfiguration()
@@ -889,18 +907,32 @@ private JavaTypeDescriptor resolveInstantiationTargetJtd(String className) {
 				.resolveDescriptor( targetJavaType );
 	}
 
-	private Class classForName(String className) {
+	private Class<?> classForName(String className) {
 		return creationContext.getServiceRegistry().getService( ClassLoaderService.class ).classForName( className );
 	}
 
 	@Override
-	public SqmDynamicInstantiationArgument visitDynamicInstantiationArg(HqlParser.DynamicInstantiationArgContext ctx) {
+	public SqmDynamicInstantiationArgument<?> visitDynamicInstantiationArg(HqlParser.DynamicInstantiationArgContext ctx) {
+		final String alias = ctx.identifier() == null ? null : ctx.identifier().getText();
+
+		final SqmSelectableNode<?> argExpression = visitDynamicInstantiationArgExpression( ctx.dynamicInstantiationArgExpression() );
+
 		//noinspection unchecked
-		return new SqmDynamicInstantiationArgument(
+		final SqmDynamicInstantiationArgument<?> argument = new SqmDynamicInstantiationArgument<>(
-				visitDynamicInstantiationArgExpression( ctx.dynamicInstantiationArgExpression() ),
+				argExpression,
-				ctx.identifier() == null ? null : ctx.identifier().getText(),
+				alias,
 				creationContext.getNodeBuilder()
 		);
+
+		//noinspection StatementWithEmptyBody
+		if ( argExpression instanceof SqmDynamicInstantiation ) {
+			// nothing else to do (avoid kludgy `! ( instanceof )` syntax
+		}
+		else {
+			getCurrentProcessingState().getPathRegistry().register( argument );
+		}
+
+		return argument;
 	}
 
 	@Override
@@ -930,56 +962,71 @@ public List<SqmExpression<?>> visitGroupByClause(HqlParser.GroupByClauseContext
 		return visitExpressions( ctx.groupByExpression() );
 	}
 
-	@Override
+	private SqmExpression<?> resolveOrderByOrGroupByExpression(
-	public SqmExpression<?> visitGroupByExpression(HqlParser.GroupByExpressionContext ctx) {
+			Supplier<TerminalNode> positionLiteralAccess,
-		if ( ctx.INTEGER_LITERAL() != null ) {
+			Supplier<HqlParser.IdentifierContext> identifierAccess,
-			// This is syntactically disallowed
+			Supplier<HqlParser.ExpressionContext> expressionContextAccess,
-			if ( ctx.collationSpecification() != null ) {
+			boolean definedCollate) {
-				throw new ParsingException( "COLLATE is not allowed for position based group by items!" );
+		if ( positionLiteralAccess.get() != null ) {
-			}
+			if ( definedCollate ) {
-			final int position = Integer.parseInt( ctx.INTEGER_LITERAL().getText() );
+				// This is syntactically disallowed
-			final SqmSelection<?> selection = getCurrentProcessingState().getPathRegistry().findSelectionByPosition( position );
+				throw new ParsingException( "COLLATE is not allowed for position based order-by or group-by items" );
-			if ( selection == null ) {
-				throw new ParsingException( "Invalid select item position " + position + " used for order by item!" );
 			}
 
-			return new SqmLiteral<>(
+			final int position = Integer.parseInt( positionLiteralAccess.get().getText() );
-					position,
+
-					resolveExpressableTypeBasic( Integer.class ),
+			// make sure this selection exists
-					creationContext.getNodeBuilder()
+			final SqmAliasedNode<?> nodeByPosition = getCurrentProcessingState()
-			);
+					.getPathRegistry()
+					.findAliasedNodeByPosition( position );
+			if ( nodeByPosition == null ) {
+				throw new ParsingException( "Numeric literal `" + position + "` used in group-by does not match a registered select-item" );
+			}
+
+			return new SqmAliasedNodeRef( position, integerDomainType, creationContext.getNodeBuilder() );
 		}
 
-		if ( ctx.identifier() != null ) {
+		final HqlParser.IdentifierContext identifier = identifierAccess.get();
-			final HqlParser.CollationSpecificationContext collationSpecificationContext = ctx.collationSpecification();
+		if ( identifier != null ) {
-			final SqmSelection<?> selection = getCurrentProcessingState().getPathRegistry().findSelectionByAlias( ctx.identifier().getText() );
+			final String identifierText = identifier.getText();
-			if ( selection != null ) {
+
-				// This is syntactically disallowed
+			final Integer correspondingPosition = getCurrentProcessingState()
-				if ( collationSpecificationContext != null ) {
+					.getPathRegistry()
-					throw new ParsingException( "COLLATE is not allowed for alias based group by items!" );
+					.findAliasedNodePosition( identifierText );
+			if ( correspondingPosition != null ) {
+				if ( definedCollate ) {
+					// This is syntactically disallowed
+					throw new ParsingException( "COLLATE is not allowed for alias based order-by or group-by items" );
 				}
-				return new SqmLiteral<>(
+				return new SqmAliasedNodeRef( correspondingPosition, integerDomainType, creationContext.getNodeBuilder() );
-						getSelectionPosition( selection ),
-						resolveExpressableTypeBasic( Integer.class ),
-						creationContext.getNodeBuilder()
-				);
 			}
 
-			final SqmFrom<?, ?> sqmFrom = getCurrentProcessingState().getPathRegistry().findFromByAlias( ctx.identifier().getText() );
+			final SqmFrom<?, ?> sqmFrom = getCurrentProcessingState().getPathRegistry().findFromByAlias( identifierText );
 			if ( sqmFrom != null ) {
-				// This is syntactically disallowed
+				if ( definedCollate ) {
-				if ( collationSpecificationContext != null ) {
+					// This is syntactically disallowed
-					throw new ParsingException( "COLLATE is not allowed for alias based group by items!" );
+					throw new ParsingException( "COLLATE is not allowed for alias based order-by or group-by items" );
 				}
+				// this will group-by all of the sub-parts in the from-element's model part
 				return sqmFrom;
 			}
 
 			final DotIdentifierConsumer dotIdentifierConsumer = dotIdentifierConsumerStack.getCurrent();
-			dotIdentifierConsumer.consumeIdentifier( ctx.getText(), true, true );
+			dotIdentifierConsumer.consumeIdentifier( identifierText, true, true );
 			return (SqmExpression<?>) dotIdentifierConsumer.getConsumedPart();
 		}
 
-		return (SqmExpression<?>) ctx.expression().accept( this );
+		return (SqmExpression<?>) expressionContextAccess.get().accept( this );
+	}
+
+	@Override
+	public SqmExpression<?> visitGroupByExpression(HqlParser.GroupByExpressionContext ctx) {
+		return resolveOrderByOrGroupByExpression(
+				ctx::INTEGER_LITERAL,
+				ctx::identifier,
+				ctx::expression,
+				ctx.collationSpecification() != null
+		);
 	}
 
 	@Override
@@ -1034,66 +1081,12 @@ public SqmSortSpecification visitSortSpecification(HqlParser.SortSpecificationCo
 
 	@Override
 	public SqmExpression<?> visitSortExpression(HqlParser.SortExpressionContext ctx) {
-		if ( ctx.INTEGER_LITERAL() != null ) {
+		return resolveOrderByOrGroupByExpression(
-			// This is syntactically disallowed
+				ctx::INTEGER_LITERAL,
-			if ( ctx.collationSpecification() != null ) {
+				ctx::identifier,
-				throw new ParsingException( "COLLATE is not allowed for position based order by items!" );
+				ctx::expression,
-			}
+				ctx.collationSpecification() != null
-			final int position = Integer.parseInt( ctx.INTEGER_LITERAL().getText() );
+		);
-			SqmSelection<?> selection = getCurrentProcessingState().getPathRegistry().findSelectionByPosition( position );
-			if ( selection == null ) {
-				selection = currentQuerySpec().getSelectClause().getSelections().get( position - 1 );
-			}
-			if ( selection == null ) {
-				throw new ParsingException( "Invalid select item position " + position + " used for order by item!" );
-			}
-
-			return new SqmLiteral<>(
-					position,
-					resolveExpressableTypeBasic( Integer.class ),
-					creationContext.getNodeBuilder()
-			);
-		}
-
-		if ( ctx.identifier() != null ) {
-			final HqlParser.CollationSpecificationContext collationSpecificationContext = ctx.collationSpecification();
-			final String alias = ctx.identifier().getText();
-			SqmSelection<?> selection = getCurrentProcessingState().getPathRegistry().findSelectionByAlias( alias );
-			if ( selection == null ) {
-				for ( SqmSelection sqmSelection : currentQuerySpec().getSelectClause().getSelections() ) {
-					if ( alias.equals( sqmSelection.getAlias() ) ) {
-						selection = sqmSelection;
-						break;
-					}
-				}
-			}
-			if ( selection != null ) {
-				// This is syntactically disallowed
-				if ( collationSpecificationContext != null ) {
-					throw new ParsingException( "COLLATE is not allowed for alias based order by items!" );
-				}
-				return new SqmLiteral<>(
-						getSelectionPosition( selection ),
-						resolveExpressableTypeBasic( Integer.class ),
-						creationContext.getNodeBuilder()
-				);
-			}
-
-			final SqmFrom<?, ?> sqmFrom = getCurrentProcessingState().getPathRegistry().findFromByAlias( ctx.identifier().getText() );
-			if ( sqmFrom != null ) {
-				// This is syntactically disallowed
-				if ( collationSpecificationContext != null ) {
-					throw new ParsingException( "COLLATE is not allowed for alias based order by items!" );
-				}
-				return sqmFrom;
-			}
-
-			final DotIdentifierConsumer dotIdentifierConsumer = dotIdentifierConsumerStack.getCurrent();
-			dotIdentifierConsumer.consumeIdentifier( ctx.getText(), true, true );
-			return (SqmExpression<?>) dotIdentifierConsumer.getConsumedPart();
-		}
-
-		return (SqmExpression<?>) ctx.expression().accept( this );
 	}
 
 	private SqmQuerySpec<?> currentQuerySpec() {

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

@@ -6,12 +6,15 @@
  */
 package org.hibernate.query.hql.internal;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.function.Function;
 
+import org.hibernate.internal.util.MutableInteger;
 import org.hibernate.jpa.spi.JpaCompliance;
 import org.hibernate.query.NavigablePath;
 import org.hibernate.query.hql.HqlLogging;
@@ -23,7 +26,7 @@
 import org.hibernate.query.sqm.SqmTreeCreationLogger;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
-import org.hibernate.query.sqm.tree.select.SqmSelection;
+import org.hibernate.query.sqm.tree.select.SqmAliasedNode;
 
 /**
  * Container for indexing needed while building an SQM tree.
@@ -39,7 +42,7 @@ public class SqmPathRegistryImpl implements SqmPathRegistry {
 
 	private final Map<String, SqmFrom> sqmFromByAlias = new HashMap<>();
 
-	private final LinkedHashMap<String, SqmSelection> sqmSelectionsByAlias = new LinkedHashMap<>();
+	private final List<SqmAliasedNode> simpleSelectionNodes = new ArrayList<>();
 
 	public SqmPathRegistryImpl(SqmCreationProcessingState associatedProcessingState) {
 		this.associatedProcessingState = associatedProcessingState;
@@ -227,21 +230,13 @@ private boolean definesAttribute(SqmPathSource containerType, String name) {
 	}
 
 	@Override
-	public SqmSelection findSelectionByAlias(String alias) {
+	public SqmAliasedNode<?> findAliasedNodeByAlias(String alias) {
-		return sqmSelectionsByAlias.get( alias );
+		assert alias != null;
-	}
 
-	@Override
+		for ( int i = 0; i < simpleSelectionNodes.size(); i++ ) {
-	public SqmSelection findSelectionByPosition(int position) {
+			final SqmAliasedNode<?> node = simpleSelectionNodes.get( i );
-		// NOTE : 1-based
+			if ( alias.equals( node.getAlias() ) ) {
-		//		so incoming position must be between >= 1 and <= map.size
+				return node;
-
-		if ( position >= 1 && position <= sqmSelectionsByAlias.size() ) {
-			int i = 1;
-			for ( Map.Entry<String, SqmSelection> entry : sqmSelectionsByAlias.entrySet() ) {
-				if ( position == i++ ) {
-					return entry.getValue();
-				}
 			}
 		}
 
@@ -249,22 +244,50 @@ public SqmSelection findSelectionByPosition(int position) {
 	}
 
 	@Override
-	public void register(SqmSelection selection) {
+	public Integer findAliasedNodePosition(String alias) {
-		if ( selection.getAlias() != null ) {
+		if ( alias == null ) {
-			checkResultVariable( selection );
+			return null;
-			sqmSelectionsByAlias.put( selection.getAlias(), selection );
 		}
+
+		// NOTE : 1-based
+
+		for ( int i = 0; i < simpleSelectionNodes.size(); i++ ) {
+			final SqmAliasedNode<?> node = simpleSelectionNodes.get( i );
+			if ( alias.equals( node.getAlias() ) ) {
+				return i + 1;
+			}
+		}
+
+		return null;
 	}
 
-	private void checkResultVariable(SqmSelection selection) {
+	@Override
-		final String alias = selection.getAlias();
+	public SqmAliasedNode<?> findAliasedNodeByPosition(int position) {
+		// NOTE : 1-based
 
-		if ( sqmSelectionsByAlias.containsKey( alias ) ) {
+		return simpleSelectionNodes.get( position - 1 );
+	}
+
+	@Override
+	public void register(SqmAliasedNode<?> node) {
+		checkResultVariable( node );
+		simpleSelectionNodes.add( node );
+	}
+
+	private void checkResultVariable(SqmAliasedNode selection) {
+		final String alias = selection.getAlias();
+		if ( alias == null ) {
+			return;
+		}
+
+		final Integer position = findAliasedNodePosition( alias );
+		if ( position != null ) {
 			throw new AliasCollisionException(
 					String.format(
 							Locale.ENGLISH,
-							"Alias [%s] is already used in same select clause",
+							"Alias [%s] is already used in same select clause [position=%s]",
-							alias
+							alias,
+							position
 					)
 			);
 		}

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

@@ -12,7 +12,7 @@
 import org.hibernate.query.NavigablePath;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
-import org.hibernate.query.sqm.tree.select.SqmSelection;
+import org.hibernate.query.sqm.tree.select.SqmAliasedNode;
 
 /**
  * Registry for SqmPath references providing the ability to access them
@@ -76,17 +76,30 @@ public interface SqmPathRegistry {
 	// SqmSelection
 
 	/**
-	 * Register an SqmSelection
+	 * Register a node aliased within the select-clause
 	 */
-	void register(SqmSelection selection);
+	void register(SqmAliasedNode<?> aliasedNode);
 
 	/**
-	 * Find an SqmSelection by the explicit alias assigned to it
+	 * Find a node (if one) by the explicit alias assigned to it
+	 * within the select-clause
+	 *
+	 * @return The matching node, or null
 	 */
-	SqmSelection findSelectionByAlias(String alias);
+	SqmAliasedNode<?> findAliasedNodeByAlias(String alias);
+
+	/**
+	 * Find the position of a node with the given alias, relative to the
+	 * underlying SQL select-list.
+	 *
+	 * @return The position, or null
+	 */
+	Integer findAliasedNodePosition(String alias);
 
 	/**
 	 * Find an SqmSelection by its position in the SqmSelectClause
+	 *
+	 * @return The matching node, or null
 	 */
-	SqmSelection findSelectionByPosition(int position);
+	SqmAliasedNode<?> findAliasedNodeByPosition(int position);
 }

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

@@ -8,7 +8,6 @@
 
 import org.hibernate.Incubating;
 import org.hibernate.query.sqm.tree.select.SqmSelectQuery;
-import org.hibernate.query.sqm.tree.select.SqmSelection;
 
 /**
  * SqmCreationProcessingState specialization for processing a SQM query-spec
@@ -17,10 +16,6 @@
  */
 @Incubating
 public interface SqmQuerySpecCreationProcessingState extends SqmCreationProcessingState {
-	void registerSelection(SqmSelection selection);
-	SqmSelection findSelectionByAlias(String alias);
-	SqmSelection findSelectionByPosition(int position);
-
 	@Override
 	SqmSelectQuery<?> getProcessingQuery();
 }

hibernate-core/src/main/java/org/hibernate/query/results/SqlAstCreationStateImpl.java

@@ -1,122 +0,0 @@
-/*
- * 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.query.results;
-
-import java.util.HashMap;
-import java.util.Map;
-import java.util.function.Consumer;
-import java.util.function.Function;
-
-import org.hibernate.Internal;
-import org.hibernate.LockMode;
-import org.hibernate.engine.spi.SessionFactoryImplementor;
-import org.hibernate.sql.ast.spi.SqlAliasBaseGenerator;
-import org.hibernate.sql.ast.spi.SqlAliasBaseManager;
-import org.hibernate.sql.ast.spi.SqlAstCreationContext;
-import org.hibernate.sql.ast.spi.SqlAstCreationState;
-import org.hibernate.sql.ast.spi.SqlAstProcessingState;
-import org.hibernate.sql.ast.spi.SqlExpressionResolver;
-import org.hibernate.sql.ast.spi.SqlSelection;
-import org.hibernate.sql.ast.tree.expression.Expression;
-import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
-import org.hibernate.type.spi.TypeConfiguration;
-
-/**
- * SqlAstCreationState implementation for result-set mapping handling
- *
- * @author Steve Ebersole
- */
-@Internal
-public class SqlAstCreationStateImpl implements SqlAstCreationState, SqlAstProcessingState, SqlExpressionResolver {
-
-	private final FromClauseAccessImpl fromClauseAccess;
-	private final SqlAliasBaseManager sqlAliasBaseManager;
-
-	private final Consumer<SqlSelection> sqlSelectionConsumer;
-	private final Map<String, SqlSelectionImpl> sqlSelectionMap = new HashMap<>();
-
-	private final SessionFactoryImplementor sessionFactory;
-
-	public SqlAstCreationStateImpl(
-			FromClauseAccessImpl fromClauseAccess,
-			SqlAliasBaseManager sqlAliasBaseManager,
-			Consumer<SqlSelection> sqlSelectionConsumer,
-			SessionFactoryImplementor sessionFactory) {
-		this.fromClauseAccess = fromClauseAccess;
-		this.sqlAliasBaseManager = sqlAliasBaseManager;
-		this.sqlSelectionConsumer = sqlSelectionConsumer;
-		this.sessionFactory = sessionFactory;
-	}
-
-
-	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-	// SqlAstProcessingState
-
-	@Override
-	public SqlAstProcessingState getParentState() {
-		return null;
-	}
-
-	@Override
-	public SqlAstCreationState getSqlAstCreationState() {
-		return this;
-	}
-
-
-	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-	// SqlAstCreationState
-
-	@Override
-	public SqlAstCreationContext getCreationContext() {
-		return sessionFactory;
-	}
-
-	@Override
-	public SqlAstCreationStateImpl getCurrentProcessingState() {
-		return this;
-	}
-
-	@Override
-	public SqlAstCreationStateImpl getSqlExpressionResolver() {
-		return this;
-	}
-
-	@Override
-	public FromClauseAccessImpl getFromClauseAccess() {
-		return fromClauseAccess;
-	}
-
-	@Override
-	public SqlAliasBaseGenerator getSqlAliasBaseGenerator() {
-		return sqlAliasBaseManager;
-	}
-
-	@Override
-	public LockMode determineLockMode(String identificationVariable) {
-		return LockMode.READ;
-	}
-
-
-	// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-	// SqlExpressionResolver
-
-
-	@Override
-	public Expression resolveSqlExpression(
-			String key,
-			Function<SqlAstProcessingState, Expression> creator) {
-		return null;
-	}
-
-	@Override
-	public SqlSelection resolveSqlSelection(
-			Expression expression,
-			JavaTypeDescriptor javaTypeDescriptor,
-			TypeConfiguration typeConfiguration) {
-		return null;
-	}
-}

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

@@ -39,10 +39,6 @@ public SqmQuery<?> getProcessingQuery() {
 		return processingQuery;
 	}
 
-	protected SqmPathRegistryImpl getProcessingIndex() {
-		return processingIndex;
-	}
-
 	@Override
 	public SqmCreationState getCreationState() {
 		return creationState;

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

@@ -7,10 +7,9 @@
 package org.hibernate.query.sqm.internal;
 
 import org.hibernate.query.hql.spi.SqmCreationProcessingState;
-import org.hibernate.query.hql.spi.SqmQuerySpecCreationProcessingState;
 import org.hibernate.query.hql.spi.SqmCreationState;
+import org.hibernate.query.hql.spi.SqmQuerySpecCreationProcessingState;
 import org.hibernate.query.sqm.tree.select.SqmSelectQuery;
-import org.hibernate.query.sqm.tree.select.SqmSelection;
 
 /**
  * Models the state related to parsing a sqm spec.  As a "linked list" to account for
@@ -42,19 +41,4 @@ public SqmCreationProcessingState getParentProcessingState() {
 	public SqmSelectQuery<?> getProcessingQuery() {
 		return (SqmSelectQuery<?>) super.getProcessingQuery();
 	}
-
-	@Override
-	public void registerSelection(SqmSelection selection) {
-		getProcessingIndex().register( selection );
-	}
-
-	@Override
-	public SqmSelection findSelectionByAlias(String alias) {
-		return getProcessingIndex().findSelectionByAlias( alias );
-	}
-
-	@Override
-	public SqmSelection findSelectionByPosition(int position) {
-		return getProcessingIndex().findSelectionByPosition( position );
-	}
 }

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

@@ -219,10 +219,9 @@ public void visitSelectClause(
 				sqlQuerySpec,
 				rootProcessingState,
 				this,
-				r -> new SqlSelectionForSqmSelectionResolver(
+				r -> new SqmAliasedNodePositionTracker(
 						r,
-						sqmSelectClause.getSelectionItems()
+						sqmSelectClause.getSelectionItems().size()
-								.size()
 				),
 				getCurrentClauseStack()::getCurrent
 		) {

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

@@ -23,6 +23,7 @@
 import javax.persistence.TemporalType;
 
 import org.hibernate.HibernateException;
+import org.hibernate.Internal;
 import org.hibernate.LockMode;
 import org.hibernate.LockOptions;
 import org.hibernate.NotYetImplementedFor6Exception;
@@ -124,6 +125,7 @@
 import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
 import org.hibernate.query.sqm.tree.expression.Conversion;
 import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter;
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmAny;
 import org.hibernate.query.sqm.tree.expression.SqmBinaryArithmetic;
 import org.hibernate.query.sqm.tree.expression.SqmByUnit;
@@ -361,11 +363,20 @@ public BaseSqmToSqlAstConverter(
 		this.statement = statement;
 
 		if ( statement instanceof SqmSelectStatement<?> ) {
+			// NOTE: note the difference here between `JpaSelection#getSelectionItems`
+			//		and `SqmSelectClause#getSelections`.
+			//
+			//		- `#getSelectionItems` returns individual select-items.  "grouped" selections,
+			//			such as dynamic-instantiation, unwrap themselves (possibly recursively).
+			//			It is a JPA-defined method
+			//
+			//		- `#getSelections` returns top-level selections.  These are ultimately the
+			//			domain-results of the query
 			this.domainResults = new ArrayList<>(
 					( (SqmSelectStatement<?>) statement ).getQueryPart()
 							.getFirstQuerySpec()
 							.getSelectClause()
-							.getSelectionItems()
+							.getSelections()
 							.size()
 			);
 
@@ -885,7 +896,7 @@ public InsertStatement visitInsertSelectStatement(SqmInsertSelectStatement<?> sq
 				new SqlAstProcessingStateImpl(
 						null,
 						this,
-						r -> new SqlSelectionForSqmSelectionResolver(
+						r -> new SqmAliasedNodePositionTracker(
 								r,
 								selectQueryPart.getFirstQuerySpec()
 										.getSelectClause()
@@ -1072,12 +1083,18 @@ public DynamicInstantiation<?> visitDynamicInstantiation(SqmDynamicInstantiation
 		);
 
 		for ( SqmDynamicInstantiationArgument<?> sqmArgument : sqmDynamicInstantiation.getArguments() ) {
-			final DomainResultProducer<?> argumentResultProducer = (DomainResultProducer<?>) sqmArgument.getSelectableNode()
+			//noinspection StatementWithEmptyBody
+			if ( sqmArgument.getSelectableNode() instanceof SqmDynamicInstantiation ) {
+				// see discussion on `#visitSelection` wrt dynamic-instantiation
+			}
+			else {
+				currentSqlSelectionCollector().next();
+			}
+
+			final DomainResultProducer<?> argumentResultProducer = (DomainResultProducer<?>) sqmArgument
+					.getSelectableNode()
 					.accept( this );
-			dynamicInstantiation.addArgument(
+			dynamicInstantiation.addArgument( sqmArgument.getAlias(), argumentResultProducer, this );
-					sqmArgument.getAlias(),
-					argumentResultProducer
-			);
 		}
 
 		dynamicInstantiation.complete();
@@ -1086,14 +1103,14 @@ public DynamicInstantiation<?> visitDynamicInstantiation(SqmDynamicInstantiation
 	}
 
 	@SuppressWarnings("unchecked")
-	private <T> JavaTypeDescriptor<T> interpretInstantiationTarget(SqmDynamicInstantiationTarget<?> instantiationTarget) {
+	private <X> JavaTypeDescriptor<X> interpretInstantiationTarget(SqmDynamicInstantiationTarget<?> instantiationTarget) {
-		final Class<T> targetJavaType;
+		final Class<X> targetJavaType;
 
 		if ( instantiationTarget.getNature() == DynamicInstantiationNature.LIST ) {
-			targetJavaType = (Class<T>) List.class;
+			targetJavaType = (Class<X>) List.class;
 		}
 		else if ( instantiationTarget.getNature() == DynamicInstantiationNature.MAP ) {
-			targetJavaType = (Class<T>) Map.class;
+			targetJavaType = (Class<X>) Map.class;
 		}
 		else {
 			targetJavaType = instantiationTarget.getJavaType();
@@ -1250,6 +1267,8 @@ public Map<String, CteStatement> visitCteContainer(SqmCteContainer consumer) {
 		return cteStatements;
 	}
 
+	private boolean trackSelectionsForGroup;
+
 	@Override
 	public QueryPart visitQueryPart(SqmQueryPart<?> queryPart) {
 		return (QueryPart) super.visitQueryPart( queryPart );
@@ -1265,26 +1284,36 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
 				queryGroup.getSetOperator(),
 				newQueryParts
 		);
+
+		if ( queryGroup.getOrderByClause() != null && queryGroup.getOrderByClause().hasPositionalSortItem() ) {
+			trackSelectionsForGroup = true;
+		}
+
 		final SqlAstQueryPartProcessingStateImpl processingState = new SqlAstQueryPartProcessingStateImpl(
 				group,
 				getCurrentProcessingState(),
 				this,
-				DelegatingSqlSelectionForSqmSelectionCollector::new,
+				DelegatingSqmAliasedNodeCollector::new,
 				currentClauseStack::getCurrent
 		);
-		final DelegatingSqlSelectionForSqmSelectionCollector collector = (DelegatingSqlSelectionForSqmSelectionCollector) processingState
+		final DelegatingSqmAliasedNodeCollector collector = (DelegatingSqmAliasedNodeCollector) processingState
 				.getSqlExpressionResolver();
 		pushProcessingState( processingState );
 
 		try {
 			newQueryParts.add( visitQueryPart( queryParts.get( 0 ) ) );
-			collector.setSqlSelectionForSqmSelectionCollector(
+
-					(SqlSelectionForSqmSelectionCollector) lastPoppedProcessingState.getSqlExpressionResolver()
+			collector.setSqmAliasedNodeCollector(
+					(SqmAliasedNodeCollector) lastPoppedProcessingState.getSqlExpressionResolver()
 			);
+
+			visitOrderByOffsetAndFetch( queryGroup, group );
+
+			trackSelectionsForGroup = false;
 			for ( int i = 1; i < size; i++ ) {
 				newQueryParts.add( visitQueryPart( queryParts.get( i ) ) );
 			}
-			visitOrderByOffsetAndFetch( queryGroup, group );
+
 			return group;
 		}
 		finally {
@@ -1294,8 +1323,9 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
 
 	@Override
 	public QuerySpec visitQuerySpec(SqmQuerySpec<?> sqmQuerySpec) {
+		final boolean topLevel = getProcessingStateStack().isEmpty();
 		final QuerySpec sqlQuerySpec = new QuerySpec(
-				getProcessingStateStack().isEmpty(),
+				topLevel,
 				sqmQuerySpec.getFromClause().getNumberOfRoots()
 		);
 		final SqmSelectClause selectClause = sqmQuerySpec.getSelectClause();
@@ -1303,22 +1333,45 @@ public QuerySpec visitQuerySpec(SqmQuerySpec<?> sqmQuerySpec) {
 		Predicate originalAdditionalRestrictions = additionalRestrictions;
 		additionalRestrictions = null;
 
-		pushProcessingState(
+		final boolean trackAliasedNodePositions;
-				new SqlAstQueryPartProcessingStateImpl(
+		if ( trackSelectionsForGroup ) {
-						sqlQuerySpec,
+			trackAliasedNodePositions = true;
-						getCurrentProcessingState(),
+		}
-						this,
+		else if ( sqmQuerySpec.getOrderByClause() != null && sqmQuerySpec.getOrderByClause().hasPositionalSortItem() ) {
-						r -> new SqlSelectionForSqmSelectionResolver(
+			trackAliasedNodePositions = true;
-								r,
+		}
-								selectClause.getSelectionItems()
+		else if ( sqmQuerySpec.hasPositionalGroupItem() ) {
-										.size()
+			trackAliasedNodePositions = true;
-						),
+		}
-						currentClauseStack::getCurrent
+		else {
-				)
+			trackAliasedNodePositions = false;
-		);
+		}
+
+		final SqlAstProcessingState processingState;
+		if ( trackAliasedNodePositions ) {
+			processingState = new SqlAstQueryPartProcessingStateImpl(
+					sqlQuerySpec,
+					getCurrentProcessingState(),
+					this,
+					r -> new SqmAliasedNodePositionTracker(
+							r,
+							selectClause.getSelectionItems().size()
+					),
+					currentClauseStack::getCurrent
+			);
+		}
+		else {
+			processingState = new SqlAstQueryPartProcessingStateImpl(
+					sqlQuerySpec,
+					getCurrentProcessingState(),
+					this,
+					currentClauseStack::getCurrent
+			);
+		}
+
+		pushProcessingState( processingState );
 
 		try {
-			final boolean topLevel = sqlQuerySpec.isRoot();
 			if ( topLevel ) {
 				orderByFragmentConsumer = new StandardOrderByFragmentConsumer();
 			}
@@ -1464,12 +1517,13 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) {
 
 	@Override
 	public Void visitSelection(SqmSelection<?> sqmSelection) {
-		currentSqlSelectionCollector().next();
 		final Map<String, DomainResultProducer<?>> resultProducers;
+
 		if ( sqmSelection.getSelectableNode() instanceof SqmJpaCompoundSelection<?> ) {
 			SqmJpaCompoundSelection<?> selectableNode = (SqmJpaCompoundSelection<?>) sqmSelection.getSelectableNode();
 			resultProducers = new HashMap<>( selectableNode.getSelectionItems().size() );
 			for ( SqmSelectableNode<?> selectionItem : selectableNode.getSelectionItems() ) {
+				currentSqlSelectionCollector().next();
 				resultProducers.put(
 						selectionItem.getAlias(),
 						(DomainResultProducer<?>) selectionItem.accept( this )
@@ -1478,6 +1532,18 @@ public Void visitSelection(SqmSelection<?> sqmSelection) {
 
 		}
 		else {
+			//noinspection StatementWithEmptyBody
+			if ( sqmSelection.getSelectableNode() instanceof SqmDynamicInstantiation ) {
+				// this `currentSqlSelectionCollector().next()` is meant solely for resolving
+				// literal reference to a selection-item in the order-by or group-by clause.
+				// in the case of `SqmDynamicInstantiation`, that ordering should ignore that
+				// level here.  visiting the dynamic-instantiation will manage this for its
+				// arguments
+			}
+			else {
+				// otherwise, position the collector at the next index in prep for visitation
+				currentSqlSelectionCollector().next();
+			}
 			resultProducers = Collections.singletonMap(
 					sqmSelection.getAlias(),
 					(DomainResultProducer<?>) sqmSelection.getSelectableNode().accept( this )
@@ -1524,31 +1590,32 @@ public Void visitSelection(SqmSelection<?> sqmSelection) {
 	}
 
 	protected Expression resolveGroupOrOrderByExpression(SqmExpression<?> groupByClauseExpression) {
-		if ( groupByClauseExpression instanceof SqmLiteral<?> ) {
+		if ( groupByClauseExpression instanceof SqmAliasedNodeRef ) {
-			Object literal = ( (SqmLiteral<?>) groupByClauseExpression ).getLiteralValue();
+			final int aliasedNodeOrdinal = ( (SqmAliasedNodeRef) groupByClauseExpression ).getPosition();
-			if ( literal instanceof Integer ) {
+			final int sqmPosition = aliasedNodeOrdinal - 1;
-				// Integer literals have a special meaning in the GROUP BY and ORDER BY clause i.e. they refer to a select item
+			final List<SqlSelection> selections = currentSqlSelectionCollector().getSelections( sqmPosition );
-				final int sqmPosition = (Integer) literal;
+			assert selections != null : String.format( Locale.ROOT, "No SqlSelections for SQM position `%s`", sqmPosition );
-				final List<SqlSelection> selections = currentSqlSelectionCollector().getSelections( sqmPosition );
+			final List<Expression> expressions = new ArrayList<>( selections.size() );
-				final List<Expression> expressions = new ArrayList<>( selections.size() );
+			OUTER: for ( int i = 0; i < selections.size(); i++ ) {
-				OUTER: for ( int i = 0; i < selections.size(); i++ ) {
+				final SqlSelection selection = selections.get( i );
-					final SqlSelection selection = selections.get( i );
+				// We skip duplicate selections which can occur when grouping/ordering by an entity alias.
-					// We skip duplicate selections which can occur when grouping/ordering by an entity alias.
+				// Duplication happens because the primary key of an entity usually acts as FK target of collections
-					// Duplication happens because the primary key of an entity usually acts as FK target of collections
+				// which is, just like the identifier itself, also registered as selection
-					// which is, just like the identifier itself, also registered as selection
+				for ( int j = 0; j < i; j++ ) {
-					for ( int j = 0; j < i; j++ ) {
+					if ( selections.get( j ) == selection ) {
-						if ( selections.get( j ) == selection ) {
+						continue OUTER;
-							continue OUTER;
-						}
 					}
-					expressions.add( new SqlSelectionExpression( selection ) );
 				}
-				if ( expressions.size() == 1 ) {
+				expressions.add( new SqlSelectionExpression( selection ) );
-					return expressions.get( 0 );
-				}
-				return new SqlTuple( expressions, null );
 			}
+
+			if ( expressions.size() == 1 ) {
+				return expressions.get( 0 );
+			}
+
+			return new SqlTuple( expressions, null );
 		}
+
 		return (Expression) groupByClauseExpression.accept( this );
 	}
 
@@ -2064,8 +2131,8 @@ private QueryPart currentQueryPart() {
 		return processingState.getInflightQueryPart();
 	}
 
-	protected SqlSelectionForSqmSelectionCollector currentSqlSelectionCollector() {
+	protected SqmAliasedNodeCollector currentSqlSelectionCollector() {
-		return (SqlSelectionForSqmSelectionCollector) getCurrentProcessingState().getSqlExpressionResolver();
+		return (SqmAliasedNodeCollector) getCurrentProcessingState().getSqlExpressionResolver();
 	}
 
 	@Override
@@ -4502,17 +4569,18 @@ else if ( getLoadQueryInfluencers().hasEnabledFetchProfiles() ) {
 		}
 	}
 
-	protected interface SqlSelectionForSqmSelectionCollector {
+	@Internal
+	public interface SqmAliasedNodeCollector {
 		void next();
 		List<SqlSelection> getSelections(int position);
 	}
 
-	protected static class DelegatingSqlSelectionForSqmSelectionCollector implements SqlExpressionResolver, SqlSelectionForSqmSelectionCollector {
+	protected static class DelegatingSqmAliasedNodeCollector implements SqlExpressionResolver, SqmAliasedNodeCollector {
 
 		private final SqlExpressionResolver delegate;
-		private SqlSelectionForSqmSelectionCollector sqlSelectionForSqmSelectionCollector;
+		private SqmAliasedNodeCollector sqmAliasedNodeCollector;
 
-		public DelegatingSqlSelectionForSqmSelectionCollector(SqlExpressionResolver delegate) {
+		public DelegatingSqmAliasedNodeCollector(SqlExpressionResolver delegate) {
 			this.delegate = delegate;
 		}
 
@@ -4523,7 +4591,7 @@ public void next() {
 
 		@Override
 		public List<SqlSelection> getSelections(int position) {
-			return sqlSelectionForSqmSelectionCollector.getSelections( position );
+			return sqmAliasedNodeCollector.getSelections( position );
 		}
 
 		@Override
@@ -4539,22 +4607,21 @@ public SqlSelection resolveSqlSelection(
 			return delegate.resolveSqlSelection( expression, javaTypeDescriptor, typeConfiguration );
 		}
 
-		public SqlSelectionForSqmSelectionCollector getSqlSelectionForSqmSelectionCollector() {
+		public SqmAliasedNodeCollector getSqmAliasedNodeCollector() {
-			return sqlSelectionForSqmSelectionCollector;
+			return sqmAliasedNodeCollector;
 		}
 
-		public void setSqlSelectionForSqmSelectionCollector(SqlSelectionForSqmSelectionCollector sqlSelectionForSqmSelectionCollector) {
+		public void setSqmAliasedNodeCollector(SqmAliasedNodeCollector sqmAliasedNodeCollector) {
-			this.sqlSelectionForSqmSelectionCollector = sqlSelectionForSqmSelectionCollector;
+			this.sqmAliasedNodeCollector = sqmAliasedNodeCollector;
 		}
 	}
 
-	protected static class SqlSelectionForSqmSelectionResolver implements SqlExpressionResolver, SqlSelectionForSqmSelectionCollector {
+	protected static class SqmAliasedNodePositionTracker implements SqlExpressionResolver, SqmAliasedNodeCollector {
-
 		private final SqlExpressionResolver delegate;
 		private final List<SqlSelection>[] sqlSelectionsForSqmSelection;
 		private int index = -1;
 
-		public SqlSelectionForSqmSelectionResolver(SqlExpressionResolver delegate, int sqmSelectionCount) {
+		public SqmAliasedNodePositionTracker(SqlExpressionResolver delegate, int sqmSelectionCount) {
 			this.delegate = delegate;
 			sqlSelectionsForSqmSelection = new List[sqmSelectionCount];
 		}
@@ -4566,7 +4633,7 @@ public void next() {
 
 		@Override
 		public List<SqlSelection> getSelections(int position) {
-			return sqlSelectionsForSqmSelection[position - 1];
+			return sqlSelectionsForSqmSelection[ position ];
 		}
 
 		@Override

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

@@ -8,10 +8,13 @@
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.function.Function;
 import java.util.function.Supplier;
 
+import org.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter;
+import org.hibernate.query.sqm.sql.BaseSqmToSqlAstConverter.SqmAliasedNodeCollector;
 import org.hibernate.query.sqm.sql.ConversionException;
 import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.spi.SqlAstCreationState;
@@ -29,7 +32,8 @@
  *
  * @author Steve Ebersole
  */
-public class SqlAstProcessingStateImpl implements SqlAstProcessingState, SqlExpressionResolver {
+public class SqlAstProcessingStateImpl
+		implements SqlAstProcessingState, SqlExpressionResolver, SqmAliasedNodeCollector {
 	private final SqlAstProcessingState parentState;
 	private final SqlAstCreationState creationState;
 	private final SqlExpressionResolver expressionResolver;
@@ -141,4 +145,15 @@ public SqlSelection resolveSqlSelection(
 			TypeConfiguration typeConfiguration) {
 		throw new ConversionException( "Unexpected call to resolve SqlSelection outside of QuerySpec processing" );
 	}
+
+	@Override
+	public void next() {
+		// nothing to do
+		int i = 1;
+	}
+
+	@Override
+	public List<SqlSelection> getSelections(int position) {
+		throw new UnsupportedOperationException();
+	}
 }

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmAliasedNodeRef.java

@@ -0,0 +1,36 @@
+/*
+ * 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.query.sqm.tree.expression;
+
+import org.hibernate.query.sqm.NodeBuilder;
+import org.hibernate.query.sqm.SemanticQueryWalker;
+import org.hibernate.query.sqm.SqmExpressable;
+
+/**
+ * Models a reference to a {@link org.hibernate.query.sqm.tree.select.SqmAliasedNode}
+ * used in the order-by or group-by clause by either position or alias,
+ * though the reference is normalized here to a positional ref
+ */
+public class SqmAliasedNodeRef extends AbstractSqmExpression<Integer> {
+	private final int position;
+
+	public SqmAliasedNodeRef(int position, SqmExpressable<Integer> intType, NodeBuilder criteriaBuilder) {
+		super( intType, criteriaBuilder );
+		this.position = position;
+	}
+
+	public int getPosition() {
+		return position;
+	}
+
+	@Override
+	public <X> X accept(SemanticQueryWalker<X> walker) {
+		// we expect this to be handled specially in
+		// `BaseSqmToSqlAstConverter#resolveGroupOrOrderByExpression`
+		throw new UnsupportedOperationException();
+	}
+}

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

@@ -10,23 +10,32 @@
 import java.util.Collections;
 import java.util.List;
 
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmExpression;
 
 /**
  * @author Steve Ebersole
  */
 public class SqmOrderByClause {
+	private boolean hasPositionalSortItem;
 	private List<SqmSortSpecification> sortSpecifications;
 
 	public SqmOrderByClause() {
 	}
 
+	public boolean hasPositionalSortItem() {
+		return hasPositionalSortItem;
+	}
+
 	@SuppressWarnings("UnusedReturnValue")
 	public SqmOrderByClause addSortSpecification(SqmSortSpecification sortSpecification) {
 		if ( sortSpecifications == null ) {
 			sortSpecifications = new ArrayList<>();
 		}
 		sortSpecifications.add( sortSpecification );
+		if ( sortSpecification.getExpression() instanceof SqmAliasedNodeRef ) {
+			hasPositionalSortItem = true;
+		}
 		return this;
 	}
 
@@ -49,5 +58,11 @@ public List<SqmSortSpecification> getSortSpecifications() {
 	public void setSortSpecifications(List<SqmSortSpecification> sortSpecifications) {
 		this.sortSpecifications = new ArrayList<>();
 		this.sortSpecifications.addAll( sortSpecifications );
+		for ( int i = 0; i < sortSpecifications.size(); i++ ) {
+			final SqmSortSpecification sortSpecification = sortSpecifications.get( i );
+			if ( sortSpecification.getExpression() instanceof SqmAliasedNodeRef ) {
+				hasPositionalSortItem = true;
+			}
+		}
 	}
 }

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

@@ -83,6 +83,7 @@ public void setSetOperator(SetOperator setOperator) {
 		this.setOperator = setOperator;
 	}
 
+
 	@Override
 	public SqmQueryGroup<T> setSortSpecifications(List<? extends JpaOrder> sortSpecifications) {
 		return (SqmQueryGroup<T>) super.setSortSpecifications( sortSpecifications );

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

@@ -24,6 +24,7 @@
 import org.hibernate.query.sqm.NodeBuilder;
 import org.hibernate.query.sqm.SemanticQueryWalker;
 import org.hibernate.query.sqm.tree.SqmNode;
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmExpression;
 import org.hibernate.query.sqm.tree.from.SqmAttributeJoin;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
@@ -46,6 +47,7 @@ public class SqmQuerySpec<T> extends SqmQueryPart<T>
 	private SqmSelectClause selectClause;
 	private SqmWhereClause whereClause;
 
+	private boolean hasPositionalGroupItem;
 	private List<SqmExpression<?>> groupByClauseExpressions = Collections.emptyList();
 	private SqmPredicate havingClausePredicate;
 
@@ -129,6 +131,10 @@ public void applyPredicate(SqmPredicate predicate) {
 		whereClause.applyPredicate( predicate );
 	}
 
+	public boolean hasPositionalGroupItem() {
+		return hasPositionalGroupItem;
+	}
+
 	public List<SqmExpression<?>> getGroupByClauseExpressions() {
 		return groupByClauseExpressions;
 	}
@@ -137,6 +143,13 @@ public void setGroupByClauseExpressions(List<SqmExpression<?>> groupByClauseExpr
 		this.groupByClauseExpressions = groupByClauseExpressions == null
 				? Collections.emptyList()
 				: groupByClauseExpressions;
+
+		for ( int i = 0; i < groupByClauseExpressions.size(); i++ ) {
+			final SqmExpression<?> groupItem = groupByClauseExpressions.get( i );
+			if ( groupItem instanceof SqmAliasedNodeRef ) {
+				hasPositionalGroupItem = true;
+			}
+		}
 	}
 
 	public SqmPredicate getHavingClausePredicate() {

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QueryPart.java

@@ -12,6 +12,7 @@
 
 import org.hibernate.query.FetchClauseType;
 import org.hibernate.query.sqm.sql.internal.DomainResultProducer;
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.sql.ast.tree.expression.Expression;
 
@@ -19,9 +20,11 @@
  * @author Christian Beikov
  */
 public abstract class QueryPart implements SqlAstNode, Expression, DomainResultProducer {
-
 	private final boolean isRoot;
+
+	private boolean hasPositionalSortItem;
 	private List<SortSpecification> sortSpecifications;
+
 	private Expression offsetClauseExpression;
 	private Expression fetchClauseExpression;
 	private FetchClauseType fetchClauseType = FetchClauseType.ROWS_ONLY;
@@ -48,6 +51,10 @@ public boolean hasSortSpecifications() {
 		return sortSpecifications != null && !sortSpecifications.isEmpty();
 	}
 
+	public boolean hasPositionalSortItem() {
+		return hasPositionalSortItem;
+	}
+
 	public List<SortSpecification> getSortSpecifications() {
 		return sortSpecifications;
 	}
@@ -63,6 +70,12 @@ public void addSortSpecification(SortSpecification specification) {
 			sortSpecifications = new ArrayList<>();
 		}
 		sortSpecifications.add( specification );
+
+		if ( isRoot ) {
+			if ( specification.getSortExpression() instanceof SqmAliasedNodeRef ) {
+				hasPositionalSortItem = true;
+			}
+		}
 	}
 
 	public boolean hasOffsetOrFetchClause() {

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/select/QuerySpec.java

@@ -12,6 +12,7 @@
 
 import org.hibernate.metamodel.mapping.MappingModelExpressable;
 import org.hibernate.query.sqm.sql.internal.DomainResultProducer;
+import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.sql.ast.SqlAstWalker;
 import org.hibernate.sql.ast.spi.SqlAstTreeHelper;
 import org.hibernate.sql.ast.spi.SqlExpressionResolver;
@@ -37,6 +38,7 @@ public class QuerySpec extends QueryPart implements SqlAstNode, PredicateContain
 
 	private Predicate whereClauseRestrictions;
 
+	private boolean hasPositionalGroupItem;
 	private List<Expression> groupByClauseExpressions = Collections.emptyList();
 	private Predicate havingClauseRestrictions;
 
@@ -86,8 +88,20 @@ public List<Expression> getGroupByClauseExpressions() {
 		return groupByClauseExpressions;
 	}
 
+	public boolean hasPositionalGroupItem() {
+		return hasPositionalGroupItem;
+	}
+
 	public void setGroupByClauseExpressions(List<Expression> groupByClauseExpressions) {
 		this.groupByClauseExpressions = groupByClauseExpressions == null ? Collections.emptyList() : groupByClauseExpressions;
+		if ( isRoot() ) {
+			for ( int i = 0; i < groupByClauseExpressions.size(); i++ ) {
+				final Expression groupItem = groupByClauseExpressions.get( i );
+				if ( groupItem instanceof SqmAliasedNodeRef ) {
+					hasPositionalGroupItem = true;
+				}
+			}
+		}
 	}
 
 	public Predicate getHavingClauseRestrictions() {

hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiation.java

@@ -58,7 +58,7 @@ public Class<T> getTargetJavaType() {
 		return getTargetJavaTypeDescriptor().getJavaTypeClass();
 	}
 
-	public void addArgument(String alias, DomainResultProducer argumentResultProducer) {
+	public void addArgument(String alias, DomainResultProducer<?> argumentResultProducer, DomainResultCreationState creationState) {
 		if ( argumentAdditionsComplete ) {
 			throw new ConversionException( "Unexpected call to DynamicInstantiation#addAgument after previously complete" );
 		}
@@ -90,7 +90,7 @@ else if ( Map.class.equals( getTargetJavaTypeDescriptor().getJavaTypeClass() ) )
 			arguments = new ArrayList<>();
 		}
 
-		arguments.add( new DynamicInstantiationArgument( argumentResultProducer, alias ) );
+		arguments.add( new DynamicInstantiationArgument<>( alias, argumentResultProducer, creationState ) );
 	}
 
 	public void complete() {

hibernate-core/src/main/java/org/hibernate/sql/results/graph/instantiation/internal/DynamicInstantiationArgument.java

@@ -7,18 +7,22 @@
 package org.hibernate.sql.results.graph.instantiation.internal;
 
 import org.hibernate.query.sqm.sql.internal.DomainResultProducer;
+import org.hibernate.sql.results.graph.DomainResult;
 import org.hibernate.sql.results.graph.DomainResultCreationState;
 
 /**
  * @author Steve Ebersole
  */
-public class DynamicInstantiationArgument {
+public class DynamicInstantiationArgument<T> {
-	private final DomainResultProducer argumentResultProducer;
+	private final ArgumentDomainResult<T> argumentResult;
 	private final String alias;
 
 	@SuppressWarnings("WeakerAccess")
-	public DynamicInstantiationArgument(DomainResultProducer argumentResultProducer, String alias) {
+	public DynamicInstantiationArgument(
-		this.argumentResultProducer = argumentResultProducer;
+			String alias,
+			DomainResultProducer<T> argumentResultProducer,
+			DomainResultCreationState creationState) {
+		this.argumentResult = new ArgumentDomainResult<>( argumentResultProducer.createDomainResult( alias, creationState ) );
 		this.alias = alias;
 	}
 
@@ -26,9 +30,7 @@ public String getAlias() {
 		return alias;
 	}
 
-	public ArgumentDomainResult buildArgumentDomainResult(DomainResultCreationState creationState) {
+	public ArgumentDomainResult<T> buildArgumentDomainResult(DomainResultCreationState creationState) {
-		return new ArgumentDomainResult(
+		return argumentResult;
-				argumentResultProducer.createDomainResult( alias, creationState )
-		);
 	}
 }

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

@@ -1,10 +1,10 @@
 /*
  * Hibernate, Relational Persistence for Idiomatic Java
  *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
+ * 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>.
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
  */
-package org.hibernate.orm.test.set;
+package org.hibernate.orm.test.query.hql.org.hibernate.orm.test.query.hql.set;
 
 import java.util.List;
 

hibernate-core/src/test/java/org/hibernate/test/hql/ASTParserLoadingOrderByTest.java

@@ -612,18 +612,18 @@ public void testOrderBySelectNewArgAliasRef() {
 
 		// ordered by address, name:
 		//   zoo3  Zoo         1312 Mockingbird Lane, Anywhere, IL USA
-		//   zoo4  Duh Zoo     1312 Mockingbird Lane, Nowhere, IL USA
 		//   zoo2  A Zoo       1313 Mockingbird Lane, Anywhere, IL USA
 		//   zoo1  Zoo         1313 Mockingbird Lane, Anywhere, IL USA
+		//   zoo4  Duh Zoo     1312 Mockingbird Lane, Nowhere, IL USA
 		list =
 				s.createQuery(
 						"select new Zoo( z.name as zname, z.address as zaddress) from Zoo z order by zaddress, zname"
 				).list();
 		assertEquals( 4, list.size() );
 		assertEquals( zoo3, list.get( 0 ) );
-		assertEquals( zoo4, list.get( 1 ) );
+		assertEquals( zoo2, list.get( 1 ) );
-		assertEquals( zoo2, list.get( 2 ) );
+		assertEquals( zoo1, list.get( 2 ) );
-		assertEquals( zoo1, list.get( 3 ) );
+		assertEquals( zoo4, list.get( 3 ) );
 
 
 		t.commit();
@@ -660,22 +660,26 @@ public void testOrderBySelectNewMapArgAliasRef() {
 
 		// ordered by address, name:
 		//   zoo3  Zoo         1312 Mockingbird Lane, Anywhere, IL USA
-		//   zoo4  Duh Zoo     1312 Mockingbird Lane, Nowhere, IL USA
 		//   zoo2  A Zoo       1313 Mockingbird Lane, Anywhere, IL USA
 		//   zoo1  Zoo         1313 Mockingbird Lane, Anywhere, IL USA
+		//   zoo4  Duh Zoo     1312 Mockingbird Lane, Nowhere, IL USA
 		list =
 				s.createQuery(
 						"select new map( z.name as zname, z.address as zaddress ) from Zoo z left join z.mammals m order by zaddress, zname"
 				).list();
 		assertEquals( 4, list.size() );
+
 		assertEquals( zoo3.getName(), ( ( Map ) list.get( 0 ) ).get( "zname" ) );
 		assertEquals( zoo3.getAddress(), ( ( Map ) list.get( 0 ) ).get( "zaddress" ) );
-		assertEquals( zoo4.getName(), ( ( Map ) list.get( 1 ) ).get( "zname" ) );
+
-		assertEquals( zoo4.getAddress(), ( ( Map ) list.get( 1 ) ).get( "zaddress" ) );
+		assertEquals( zoo2.getName(), ( ( Map ) list.get( 1 ) ).get( "zname" ) );
-		assertEquals( zoo2.getName(), ( ( Map ) list.get( 2 ) ).get( "zname" ) );
+		assertEquals( zoo2.getAddress(), ( ( Map ) list.get( 1 ) ).get( "zaddress" ) );
-		assertEquals( zoo2.getAddress(), ( ( Map ) list.get( 2 ) ).get( "zaddress" ) );
+
-		assertEquals( zoo1.getName(), ( ( Map ) list.get( 3 ) ).get( "zname" ) );
+		assertEquals( zoo1.getName(), ( ( Map ) list.get( 2 ) ).get( "zname" ) );
-		assertEquals( zoo1.getAddress(), ( ( Map ) list.get( 3 ) ).get( "zaddress" ) );
+		assertEquals( zoo1.getAddress(), ( ( Map ) list.get( 2 ) ).get( "zaddress" ) );
+
+		assertEquals( zoo4.getName(), ( ( Map ) list.get( 3 ) ).get( "zname" ) );
+		assertEquals( zoo4.getAddress(), ( ( Map ) list.get( 3 ) ).get( "zaddress" ) );
 		t.commit();
 		s.close();