Introduce FunctionExpression in SQL AST and remove shallowness handling in sqm to sql converter

hibernate-core/src/main/java/org/hibernate/dialect/function/TimestampaddFunction.java

@@ -90,6 +90,7 @@ public class TimestampaddFunction
 			SqlAstNode... sqlAstArguments) {
 		Expression to = (Expression) sqlAstArguments[2];
 		return new SelfRenderingFunctionSqlAstExpression(
+				getName(),
 				this::render,
 				asList( sqlAstArguments ),
 				impliedResultType,

hibernate-core/src/main/java/org/hibernate/dialect/function/TimestampdiffFunction.java

@@ -95,6 +95,7 @@ public class TimestampdiffFunction
 			SqlAstNode... sqlAstArguments) {
 		DurationUnit field = (DurationUnit) sqlAstArguments[0];
 		return new SelfRenderingFunctionSqlAstExpression(
+				getName(),
 				this::render,
 				asList( sqlAstArguments ),
 				impliedResultType,

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

@@ -22,6 +22,7 @@ import org.hibernate.sql.ast.spi.SqlAstCreationState;
 import org.hibernate.sql.ast.spi.SqlExpressionResolver;
 import org.hibernate.sql.ast.spi.SqlSelection;
 import org.hibernate.sql.ast.tree.SqlAstNode;
+import org.hibernate.sql.ast.tree.expression.FunctionExpression;
 import org.hibernate.sql.ast.tree.expression.SelfRenderingExpression;
 import org.hibernate.sql.results.graph.DomainResult;
 import org.hibernate.sql.results.graph.DomainResultCreationState;
@@ -40,17 +41,20 @@ import java.util.List;
  * @author Steve Ebersole
  */
 public class SelfRenderingFunctionSqlAstExpression
-		implements SelfRenderingExpression, Selectable, SqlExpressable, DomainResultProducer {
+		implements SelfRenderingExpression, Selectable, SqlExpressable, DomainResultProducer, FunctionExpression {
+	private final String functionName;
 	private final FunctionRenderingSupport renderer;
 	private final List<SqlAstNode> sqlAstArguments;
 	private final AllowableFunctionReturnType<?> type;
 	private final MappingModelExpressable<?> expressable;
 
 	public SelfRenderingFunctionSqlAstExpression(
+			String functionName,
 			FunctionRenderingSupport renderer,
 			List<SqlAstNode> sqlAstArguments,
 			AllowableFunctionReturnType<?> type,
 			MappingModelExpressable<?> expressable) {
+		this.functionName = functionName;
 		this.renderer = renderer;
 		this.sqlAstArguments = sqlAstArguments;
 		this.type = type;
@@ -58,6 +62,16 @@ public class SelfRenderingFunctionSqlAstExpression
 		this.expressable = expressable;
 	}
 
+	@Override
+	public String getFunctionName() {
+		return functionName;
+	}
+
+	@Override
+	public List<SqlAstNode> getArguments() {
+		return sqlAstArguments;
+	}
+
 	@Override
 	public MappingModelExpressable getExpressionType() {
 		return expressable;

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

@@ -31,9 +31,7 @@ import static java.util.Collections.emptyList;
 public class SelfRenderingSqmFunction<T> extends SqmFunction<T> {
 	private final AllowableFunctionReturnType<T> impliedResultType;
 	private final FunctionReturnTypeResolver returnTypeResolver;
-	private final String name;
 	private final FunctionRenderingSupport renderingSupport;
-	private final List<SqmTypedNode<?>> arguments;
 	private AllowableFunctionReturnType<?> resultType;
 
 	public SelfRenderingSqmFunction(
@@ -44,41 +42,38 @@ public class SelfRenderingSqmFunction<T> extends SqmFunction<T> {
 			FunctionReturnTypeResolver returnTypeResolver,
 			NodeBuilder nodeBuilder,
 			String name) {
-		super( name, descriptor, impliedResultType, nodeBuilder );
+		super( name, descriptor, impliedResultType, arguments, nodeBuilder );
 		this.renderingSupport = renderingSupport;
-		this.arguments = arguments;
 		this.impliedResultType = impliedResultType;
 		this.returnTypeResolver = returnTypeResolver;
-		this.name = name;
-	}
-
-	public List<SqmTypedNode<?>> getArguments() {
-		return arguments;
 	}
 
 	public FunctionRenderingSupport getRenderingSupport() {
 		return renderingSupport;
 	}
 
-	private static List<SqlAstNode> resolveSqlAstArguments(List<SqmTypedNode<?>> sqmArguments, SqmToSqlAstConverter walker) {
+	protected static List<SqlAstNode> resolveSqlAstArguments(List<SqmTypedNode<?>> sqmArguments, SqmToSqlAstConverter walker) {
 		if ( sqmArguments == null || sqmArguments.isEmpty() ) {
 			return emptyList();
 		}
 
-		final ArrayList<SqlAstNode> sqlAstArguments = new ArrayList<>();
+		final ArrayList<SqlAstNode> sqlAstArguments = new ArrayList<>( sqmArguments.size() );
 		for ( SqmTypedNode<?> sqmArgument : sqmArguments ) {
-			sqlAstArguments.add((SqlAstNode) ((SqmVisitableNode) sqmArgument).accept(walker));
+			sqlAstArguments.add( (SqlAstNode) ( (SqmVisitableNode) sqmArgument ).accept( walker ) );
 		}
 		return sqlAstArguments;
 	}
 
 	@Override
 	public SelfRenderingFunctionSqlAstExpression convertToSqlAst(SqmToSqlAstConverter walker) {
-		resolveResultType( walker.getCreationContext().getDomainModel().getTypeConfiguration() );
+		final AllowableFunctionReturnType<?> resultType = resolveResultType(
+				walker.getCreationContext().getDomainModel().getTypeConfiguration()
+		);
 
 		return new SelfRenderingFunctionSqlAstExpression(
+				getFunctionName(),
 				getRenderingSupport(),
-				resolveSqlAstArguments( getArguments(), walker),
+				resolveSqlAstArguments( getArguments(), walker ),
 				resultType,
 				getMappingModelExpressable( walker, resultType )
 		);
@@ -93,7 +88,7 @@ public class SelfRenderingSqmFunction<T> extends SqmFunction<T> {
 		return nodeType;
 	}
 
-	private void resolveResultType(TypeConfiguration typeConfiguration) {
+	protected AllowableFunctionReturnType<?> resolveResultType(TypeConfiguration typeConfiguration) {
 		if ( resultType == null ) {
 			resultType = returnTypeResolver.resolveFunctionReturnType(
 				impliedResultType,
@@ -102,9 +97,10 @@ public class SelfRenderingSqmFunction<T> extends SqmFunction<T> {
 			);
 			setExpressableType( resultType );
 		}
+		return resultType;
 	}
 
-	private MappingModelExpressable<?> getMappingModelExpressable(
+	protected MappingModelExpressable<?> getMappingModelExpressable(
 			SqmToSqlAstConverter walker,
 			AllowableFunctionReturnType<?> resultType) {
 		MappingModelExpressable<?> mapping;
@@ -131,17 +127,12 @@ public class SelfRenderingSqmFunction<T> extends SqmFunction<T> {
 							return null; // this works at least approximately
 						}
 					},
-					resolveSqlAstArguments( arguments, walker )
+					resolveSqlAstArguments( getArguments(), walker )
 			);
 		}
 		return mapping;
 	}
 
-	@Override
-	public String getFunctionName() {
-		return name;
-	}
-
 	@Override
 	public void applySqlSelections(DomainResultCreationState creationState) {
 		//implemented on SelfRenderingFunctionSqlAstExpression

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

@@ -293,13 +293,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 
 	private static final Logger log = Logger.getLogger( BaseSqmToSqlAstConverter.class );
 
-	protected enum Shallowness {
-		NONE,
-		CTOR,
-		FUNCTION,
-		SUBQUERY
-	}
-
 	private final SqlAstCreationContext creationContext;
 	private final SqmStatement<?> statement;
 
@@ -326,7 +319,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 	private FromClauseIndex lastPoppedFromClauseIndex;
 
 	private final Stack<Clause> currentClauseStack = new StandardStack<>();
-	private final Stack<Shallowness> shallownessStack = new StandardStack<>( Shallowness.NONE );
 
 	private SqmByUnit appliedByUnit;
 	private Expression adjustedTimestamp;
@@ -1331,7 +1323,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 	@Override
 	public SelectClause visitSelectClause(SqmSelectClause selectClause) {
 		currentClauseStack.push( Clause.SELECT );
-		shallownessStack.push( Shallowness.SUBQUERY );
 		try {
 			super.visitSelectClause( selectClause );
 
@@ -1340,7 +1331,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 			return sqlSelectClause;
 		}
 		finally {
-			shallownessStack.pop();
 			currentClauseStack.pop();
 		}
 	}
@@ -2297,12 +2287,10 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 	@Override
 	public Expression visitFunction(SqmFunction sqmFunction) {
 		inferableTypeAccessStack.push( () -> null );
-		shallownessStack.push( Shallowness.FUNCTION );
 		try {
 			return sqmFunction.convertToSqlAst( this );
 		}
 		finally {
-			shallownessStack.pop();
 			inferableTypeAccessStack.pop();
 		}
 	}
@@ -2319,61 +2307,37 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 
 	@Override
 	public Object visitTrimSpecification(SqmTrimSpecification specification) {
-		shallownessStack.push( Shallowness.FUNCTION );
-		try {
-			return new TrimSpecification( specification.getSpecification() );
-		}
-		finally {
-			shallownessStack.pop();
-		}
+		return new TrimSpecification( specification.getSpecification() );
 	}
 
 	@Override
 	public Object visitCastTarget(SqmCastTarget target) {
-		shallownessStack.push( Shallowness.FUNCTION );
-		try {
-			BasicValuedMapping targetType = (BasicValuedMapping) target.getType();
-			if ( targetType instanceof BasicType<?> ) {
-				targetType = InferredBasicValueResolver.resolveSqlTypeIndicators( this, (BasicType<?>) targetType );
-			}
-			return new CastTarget(
-					targetType,
-					target.getLength(),
-					target.getPrecision(),
-					target.getScale()
-			);
-		}
-		finally {
-			shallownessStack.pop();
+		BasicValuedMapping targetType = (BasicValuedMapping) target.getType();
+		if ( targetType instanceof BasicType<?> ) {
+			targetType = InferredBasicValueResolver.resolveSqlTypeIndicators( this, (BasicType<?>) targetType );
 		}
+		return new CastTarget(
+				targetType,
+				target.getLength(),
+				target.getPrecision(),
+				target.getScale()
+		);
 	}
 
 	@Override
 	public Object visitExtractUnit(SqmExtractUnit unit) {
-		shallownessStack.push( Shallowness.FUNCTION );
-		try {
-			return new ExtractUnit(
-					unit.getUnit(),
-					(BasicValuedMapping) unit.getType()
-			);
-		}
-		finally {
-			shallownessStack.pop();
-		}
+		return new ExtractUnit(
+				unit.getUnit(),
+				(BasicValuedMapping) unit.getType()
+		);
 	}
 
 	@Override
 	public Object visitDurationUnit(SqmDurationUnit unit) {
-		shallownessStack.push( Shallowness.FUNCTION );
-		try {
-			return new DurationUnit(
-					unit.getUnit(),
-					(BasicValuedMapping) unit.getType()
-			);
-		}
-		finally {
-			shallownessStack.pop();
-		}
+		return new DurationUnit(
+				unit.getUnit(),
+				(BasicValuedMapping) unit.getType()
+		);
 	}
 
 	@Override
@@ -2764,18 +2728,11 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 
 	@Override
 	public Object visitUnaryOperationExpression(SqmUnaryOperation expression) {
-		shallownessStack.push( Shallowness.NONE );
-
-		try {
-			return new UnaryOperation(
-					interpret( expression.getOperation() ),
-					toSqlExpression( expression.getOperand().accept( this ) ),
-					(BasicValuedMapping) determineValueMapping( expression.getOperand() )
-			);
-		}
-		finally {
-			shallownessStack.pop();
-		}
+		return new UnaryOperation(
+				interpret( expression.getOperation() ),
+				toSqlExpression( expression.getOperand().accept( this ) ),
+				(BasicValuedMapping) determineValueMapping( expression.getOperand() )
+		);
 	}
 
 	private UnaryArithmeticOperator interpret(UnaryArithmeticOperator operator) {
@@ -2784,52 +2741,45 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 
 	@Override
 	public Object visitBinaryArithmeticExpression(SqmBinaryArithmetic expression) {
-		shallownessStack.push( Shallowness.NONE );
+		SqmExpression leftOperand = expression.getLeftHandOperand();
+		SqmExpression rightOperand = expression.getRightHandOperand();
 
-		try {
-			SqmExpression leftOperand = expression.getLeftHandOperand();
-			SqmExpression rightOperand = expression.getRightHandOperand();
+		boolean durationToRight = TypeConfiguration.isDuration( rightOperand.getNodeType() );
+		TypeConfiguration typeConfiguration = getCreationContext().getDomainModel().getTypeConfiguration();
+		TemporalType temporalTypeToLeft = typeConfiguration.getSqlTemporalType( leftOperand.getNodeType() );
+		TemporalType temporalTypeToRight = typeConfiguration.getSqlTemporalType( rightOperand.getNodeType() );
+		boolean temporalTypeSomewhereToLeft = adjustedTimestamp != null || temporalTypeToLeft != null;
 
-			boolean durationToRight = TypeConfiguration.isDuration( rightOperand.getNodeType() );
-			TypeConfiguration typeConfiguration = getCreationContext().getDomainModel().getTypeConfiguration();
-			TemporalType temporalTypeToLeft = typeConfiguration.getSqlTemporalType( leftOperand.getNodeType() );
-			TemporalType temporalTypeToRight = typeConfiguration.getSqlTemporalType( rightOperand.getNodeType() );
-			boolean temporalTypeSomewhereToLeft = adjustedTimestamp != null || temporalTypeToLeft != null;
-
-			if ( temporalTypeToLeft != null && durationToRight ) {
-				if ( adjustmentScale != null || negativeAdjustment ) {
-					//we can't distribute a scale over a date/timestamp
-					throw new SemanticException( "scalar multiplication of temporal value" );
-				}
-			}
-
-			if ( durationToRight && temporalTypeSomewhereToLeft ) {
-				return transformDurationArithmetic( expression );
-			}
-			else if ( temporalTypeToLeft != null && temporalTypeToRight != null ) {
-				return transformDatetimeArithmetic( expression );
-			}
-			else if ( durationToRight && appliedByUnit != null ) {
-				return new BinaryArithmeticExpression(
-						toSqlExpression( leftOperand.accept( this ) ),
-						expression.getOperator(),
-						toSqlExpression( rightOperand.accept( this ) ),
-						//after distributing the 'by unit' operator
-						//we always get a Long value back
-						(BasicValuedMapping) appliedByUnit.getNodeType()
-				);
-			}
-			else {
-				return new BinaryArithmeticExpression(
-						toSqlExpression( leftOperand.accept( this ) ),
-						expression.getOperator(),
-						toSqlExpression( rightOperand.accept( this ) ),
-						getExpressionType( expression )
-				);
+		if ( temporalTypeToLeft != null && durationToRight ) {
+			if ( adjustmentScale != null || negativeAdjustment ) {
+				//we can't distribute a scale over a date/timestamp
+				throw new SemanticException( "scalar multiplication of temporal value" );
 			}
 		}
-		finally {
-			shallownessStack.pop();
+
+		if ( durationToRight && temporalTypeSomewhereToLeft ) {
+			return transformDurationArithmetic( expression );
+		}
+		else if ( temporalTypeToLeft != null && temporalTypeToRight != null ) {
+			return transformDatetimeArithmetic( expression );
+		}
+		else if ( durationToRight && appliedByUnit != null ) {
+			return new BinaryArithmeticExpression(
+					toSqlExpression( leftOperand.accept( this ) ),
+					expression.getOperator(),
+					toSqlExpression( rightOperand.accept( this ) ),
+					//after distributing the 'by unit' operator
+					//we always get a Long value back
+					(BasicValuedMapping) appliedByUnit.getNodeType()
+			);
+		}
+		else {
+			return new BinaryArithmeticExpression(
+					toSqlExpression( leftOperand.accept( this ) ),
+					expression.getOperator(),
+					toSqlExpression( rightOperand.accept( this ) ),
+					getExpressionType( expression )
+			);
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/expression/FunctionExpression.java

@@ -0,0 +1,23 @@
+/*
+ * 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.sql.ast.tree.expression;
+
+import java.util.List;
+
+import org.hibernate.sql.ast.tree.SqlAstNode;
+
+/**
+ * Models a function expression at the SQL AST level.
+ *
+ * @author Christian Beikov
+ */
+public interface FunctionExpression extends Expression {
+
+	String getFunctionName();
+
+	List<SqlAstNode> getArguments();
+}