minor refactorings to AbstractSqlAstTranslator and SelectStatement

Author: gavinking

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

@@ -3657,31 +3657,12 @@ public void visitQuerySpec(QuerySpec querySpec) {
 				this.queryPartForRowNumbering = null;
 				this.queryPartForRowNumberingClauseDepth = -1;
 			}
-			String queryGroupAlias = null;
-			if ( currentQueryPart instanceof QueryGroup ) {
-				// We always need query wrapping if we are in a query group and this query spec has a fetch or order by
-				// clause, because of order by precedence in SQL
-				if ( querySpec.hasOffsetOrFetchClause() || querySpec.hasSortSpecifications() ) {
-					queryGroupAlias = "";
-					// If the parent is a query group with a fetch clause we must use a select wrapper,
-					// or if the database does not support simple query grouping, we must use a select wrapper
-					if ( ( !dialect.supportsSimpleQueryGrouping() || currentQueryPart.hasOffsetOrFetchClause() )
-							// We can skip it though if this query spec is being row numbered,
-							// because then we already have a wrapper
-							&& queryPartForRowNumbering != querySpec ) {
-						queryGroupAlias = " grp_" + queryGroupAliasCounter + '_';
-						queryGroupAliasCounter++;
-						appendSql( "select" );
-						appendSql( queryGroupAlias );
-						appendSql( ".* from " );
-						// We need to assign aliases when we render a query spec as subquery to avoid clashing aliases
-						this.needsSelectAliases = this.needsSelectAliases || hasDuplicateSelectItems( querySpec );
-					}
-					else if ( !dialect.supportsDuplicateSelectItemsInQueryGroup() ) {
-						this.needsSelectAliases = this.needsSelectAliases || hasDuplicateSelectItems( querySpec );
-					}
-				}
-			}
+			final String queryGroupAlias =
+					wrapQueryPartsIfNecessary(
+							querySpec,
+							currentQueryPart,
+							queryPartForRowNumbering
+					);
 			queryPartStack.push( querySpec );
 			if ( queryGroupAlias != null ) {
 				appendSql( OPEN_PARENTHESIS );
@@ -3710,6 +3691,40 @@ else if ( !dialect.supportsDuplicateSelectItemsInQueryGroup() ) {
 		}
 	}
 
+	private String wrapQueryPartsIfNecessary(
+			QuerySpec querySpec, QueryPart currentQueryPart, QueryPart queryPartForRowNumbering) {
+		// We always need query wrapping if we are in a query group and if this query
+		// spec has a fetch or order by clause, because of order by precedence in SQL
+		if ( currentQueryPart instanceof QueryGroup
+				&& ( querySpec.hasOffsetOrFetchClause() || querySpec.hasSortSpecifications() ) ) {
+			// If the parent is a query group with a fetch clause, we must use a select wrapper
+			// Or, if the database does not support simple query grouping, we must use a select wrapper
+			if ( ( !dialect.supportsSimpleQueryGrouping() || currentQueryPart.hasOffsetOrFetchClause() )
+					// We can skip it though if this query spec is being row numbered,
+					// because then we already have a wrapper
+					&& queryPartForRowNumbering != querySpec ) {
+				final String queryGroupAlias = " grp_" + queryGroupAliasCounter + '_';
+				queryGroupAliasCounter++;
+				appendSql( "select" );
+				appendSql( queryGroupAlias );
+				appendSql( ".* from " );
+				// We need to assign aliases when we render a query spec as subquery to avoid clashing aliases
+				this.needsSelectAliases = this.needsSelectAliases || hasDuplicateSelectItems( querySpec );
+				return queryGroupAlias;
+			}
+			else if ( !dialect.supportsDuplicateSelectItemsInQueryGroup() ) {
+				this.needsSelectAliases = this.needsSelectAliases || hasDuplicateSelectItems( querySpec );
+				return "";
+			}
+			else {
+				return "";
+			}
+		}
+		else {
+			return null;
+		}
+	}
+
 	protected void visitQueryClauses(QuerySpec querySpec) {
 		visitSelectClause( querySpec.getSelectClause() );
 		visitFromClause( querySpec.getFromClause() );

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

@@ -64,24 +64,21 @@ public void accept(SqlAstWalker walker) {
 	}
 
 	@Override
-	public DomainResult createDomainResult(String resultVariable, DomainResultCreationState creationState) {
-		final SelectClause selectClause = queryPart.getFirstQuerySpec().getSelectClause();
-		final TypeConfiguration typeConfiguration = creationState.getSqlAstCreationState()
-				.getCreationContext()
-				.getMappingMetamodel()
-				.getTypeConfiguration();
-		final SqlExpressionResolver sqlExpressionResolver = creationState.getSqlAstCreationState().getSqlExpressionResolver();
-		if ( selectClause.getSqlSelections().size() == 1 ) {
-			final SqlSelection first = selectClause.getSqlSelections().get( 0 );
+	public DomainResult<?> createDomainResult(String resultVariable, DomainResultCreationState creationState) {
+		final List<SqlSelection> sqlSelections =
+				queryPart.getFirstQuerySpec().getSelectClause().getSqlSelections();
+		if ( sqlSelections.size() == 1 ) {
+			final SqlSelection first = sqlSelections.get( 0 );
 			final JdbcMapping jdbcMapping = first.getExpressionType().getSingleJdbcMapping();
-
-			final SqlSelection sqlSelection = sqlExpressionResolver.resolveSqlSelection(
-					this,
-					jdbcMapping.getJdbcJavaType(),
-					null,
-					typeConfiguration
-			);
-
+			final SqlSelection sqlSelection =
+					creationState.getSqlAstCreationState().getSqlExpressionResolver()
+							.resolveSqlSelection(
+									this,
+									jdbcMapping.getJdbcJavaType(),
+									null,
+									creationState.getSqlAstCreationState().getCreationContext()
+											.getTypeConfiguration()
+							);
 			return new BasicResult<>(
 					sqlSelection.getValuesArrayPosition(),
 					resultVariable,
@@ -95,20 +92,22 @@ public DomainResult createDomainResult(String resultVariable, DomainResultCreati
 
 	@Override
 	public void applySqlSelections(DomainResultCreationState creationState) {
-		final SelectClause selectClause = queryPart.getFirstQuerySpec().getSelectClause();
-		final TypeConfiguration typeConfiguration = creationState.getSqlAstCreationState()
-				.getCreationContext()
-				.getMappingMetamodel()
-				.getTypeConfiguration();
-		for ( SqlSelection sqlSelection : selectClause.getSqlSelections() ) {
+		final TypeConfiguration typeConfiguration =
+				creationState.getSqlAstCreationState().getCreationContext()
+						.getTypeConfiguration();
+		final SqlExpressionResolver expressionResolver =
+				creationState.getSqlAstCreationState().getSqlExpressionResolver();
+		for ( SqlSelection sqlSelection :
+				queryPart.getFirstQuerySpec().getSelectClause().getSqlSelections() ) {
 			sqlSelection.getExpressionType().forEachJdbcType(
 					(index, jdbcMapping) -> {
-						creationState.getSqlAstCreationState().getSqlExpressionResolver().resolveSqlSelection(
-								this,
-								jdbcMapping.getJdbcJavaType(),
-								null,
-								typeConfiguration
-						);
+						expressionResolver
+								.resolveSqlSelection(
+										this,
+										jdbcMapping.getJdbcJavaType(),
+										null,
+										typeConfiguration
+								);
 					}
 			);
 		}