Various improvements:

* Sprinkle some @nullable annotations to better understand nullness guarantees * Fix some potential NPEs * Improve Oracle array_agg emulation * Prepare for aggregate component array support
Sanne · May 3, 2024 · 940c898 · 940c898
1 parent 75e1f17
commit 940c898
Show file tree

Hide file tree

Showing 41 changed files with 208 additions and 145 deletions.
diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/AvgFunction.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/AvgFunction.java
@@ -24,6 +24,7 @@
 import org.hibernate.query.sqm.produce.function.FunctionArgumentException;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
 import org.hibernate.query.sqm.produce.function.StandardFunctionArgumentTypeResolvers;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlAstNodeRenderingMode;
@@ -44,6 +45,8 @@
 import org.hibernate.type.descriptor.jdbc.ObjectJdbcType;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 import static org.hibernate.query.sqm.produce.function.FunctionParameterType.NUMERIC;
 
 /**
@@ -240,7 +243,7 @@ public BasicValuedMapping resolveFunctionReturnType(
 		@Override
 		public ReturnableType<?> resolveFunctionReturnType(
 				ReturnableType<?> impliedType,
-				Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+				@Nullable SqmToSqlAstConverter converter,
 				List<? extends SqmTypedNode<?>> arguments,
 				TypeConfiguration typeConfiguration) {
 			final SqmExpressible<?> expressible = arguments.get( 0 ).getExpressible();

diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardSQLFunction.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardSQLFunction.java
@@ -10,15 +10,17 @@
 import java.util.function.Supplier;
 
 import org.hibernate.metamodel.mapping.BasicValuedMapping;
-import org.hibernate.metamodel.mapping.MappingModelExpressible;
 import org.hibernate.query.ReturnableType;
 import org.hibernate.query.sqm.function.NamedSqmFunctionDescriptor;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.type.BasicTypeReference;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * Simplified API allowing users to contribute
  * {@link org.hibernate.query.sqm.function.SqmFunctionDescriptor}s
@@ -42,15 +44,7 @@ public StandardSQLFunction(String name, boolean useParentheses, BasicTypeReferen
 			@Override
 			public ReturnableType<?> resolveFunctionReturnType(
 					ReturnableType<?> impliedType,
-					List<? extends SqmTypedNode<?>> arguments,
-					TypeConfiguration typeConfiguration) {
-				return resolveFunctionReturnType( impliedType, null, arguments, typeConfiguration );
-			}
-
-			@Override
-			public ReturnableType<?> resolveFunctionReturnType(
-					ReturnableType<?> impliedType,
-					Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+					@Nullable SqmToSqlAstConverter converter,
 					List<? extends SqmTypedNode<?>> arguments,
 					TypeConfiguration typeConfiguration) {
 				return type == null ? null : typeConfiguration.getBasicTypeRegistry().resolve( type );

diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/SumReturnTypeResolver.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/SumReturnTypeResolver.java
@@ -11,6 +11,7 @@
 import org.hibernate.metamodel.mapping.MappingModelExpressible;
 import org.hibernate.query.ReturnableType;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.type.BasicType;
@@ -21,6 +22,8 @@
 import java.util.List;
 import java.util.function.Supplier;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 import static org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers.extractArgumentType;
 import static org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers.extractArgumentValuedMapping;
 import static org.hibernate.type.SqlTypes.*;
@@ -59,15 +62,7 @@ public SumReturnTypeResolver(TypeConfiguration typeConfiguration) {
 	@Override
 	public ReturnableType<?> resolveFunctionReturnType(
 			ReturnableType<?> impliedType,
-			List<? extends SqmTypedNode<?>> arguments,
-			TypeConfiguration typeConfiguration) {
-		return resolveFunctionReturnType( impliedType, null, arguments, typeConfiguration );
-	}
-
-	@Override
-	public ReturnableType<?> resolveFunctionReturnType(
-			ReturnableType<?> impliedType,
-			Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+			@Nullable SqmToSqlAstConverter converter,
 			List<? extends SqmTypedNode<?>> arguments,
 			TypeConfiguration typeConfiguration) {
 		if ( impliedType != null ) {

diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/TimestampdiffFunction.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/TimestampdiffFunction.java
@@ -23,6 +23,7 @@
 import org.hibernate.query.sqm.produce.function.StandardFunctionArgumentTypeResolvers;
 import org.hibernate.query.sqm.produce.function.StandardFunctionReturnTypeResolvers;
 import org.hibernate.query.sqm.produce.function.internal.PatternRenderer;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.query.sqm.tree.expression.SqmDurationUnit;
 import org.hibernate.sql.ast.SqlAstTranslator;
@@ -35,6 +36,7 @@
 import org.hibernate.type.spi.TypeConfiguration;
 
 import jakarta.persistence.TemporalType;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import static java.util.Arrays.asList;
 import static org.hibernate.query.sqm.produce.function.FunctionParameterType.TEMPORAL;
@@ -129,15 +131,7 @@ public TimestampdiffFunctionReturnTypeResolver(BasicType<Long> longType, BasicTy
 		@Override
 		public ReturnableType<?> resolveFunctionReturnType(
 				ReturnableType<?> impliedType,
-				List<? extends SqmTypedNode<?>> arguments,
-				TypeConfiguration typeConfiguration) {
-			return resolveFunctionReturnType( impliedType, null, arguments, typeConfiguration );
-		}
-
-		@Override
-		public ReturnableType<?> resolveFunctionReturnType(
-				ReturnableType<?> impliedType,
-				Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+				@Nullable SqmToSqlAstConverter converter,
 				List<? extends SqmTypedNode<?>> arguments,
 				TypeConfiguration typeConfiguration) {
 			final BasicType<?> invariantType;

diff --git a/...rc/main/java/org/hibernate/dialect/function/array/ArrayViaArgumentReturnTypeResolver.java b/...rc/main/java/org/hibernate/dialect/function/array/ArrayViaArgumentReturnTypeResolver.java
@@ -15,11 +15,14 @@
 import org.hibernate.query.ReturnableType;
 import org.hibernate.query.sqm.SqmExpressible;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.type.BasicPluralType;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * A {@link FunctionReturnTypeResolver} that resolves the array type based on an argument.
  * The inferred type and implied type have precedence though.
@@ -37,10 +40,12 @@ public ArrayViaArgumentReturnTypeResolver(int arrayIndex) {
 	@Override
 	public ReturnableType<?> resolveFunctionReturnType(
 			ReturnableType<?> impliedType,
-			Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+			@Nullable SqmToSqlAstConverter converter,
 			List<? extends SqmTypedNode<?>> arguments,
 			TypeConfiguration typeConfiguration) {
-		final MappingModelExpressible<?> inferredType = inferredTypeSupplier.get();
+		final MappingModelExpressible<?> inferredType = converter == null
+				? null
+				: converter.resolveFunctionImpliedReturnType();
 		if ( inferredType != null ) {
 			if ( inferredType instanceof ReturnableType<?> ) {
 				return (ReturnableType<?>) inferredType;

diff --git a/.../java/org/hibernate/dialect/function/array/ArrayViaElementArgumentReturnTypeResolver.java b/.../java/org/hibernate/dialect/function/array/ArrayViaElementArgumentReturnTypeResolver.java
@@ -14,10 +14,13 @@
 import org.hibernate.metamodel.model.domain.DomainType;
 import org.hibernate.query.ReturnableType;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * A {@link FunctionReturnTypeResolver} that resolves an array type based on the arguments,
  * which are supposed to be of the element type. The inferred type and implied type have precedence though.
@@ -41,10 +44,12 @@ private ArrayViaElementArgumentReturnTypeResolver(boolean list, int elementIndex
 	@Override
 	public ReturnableType<?> resolveFunctionReturnType(
 			ReturnableType<?> impliedType,
-			Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+			@Nullable SqmToSqlAstConverter converter,
 			List<? extends SqmTypedNode<?>> arguments,
 			TypeConfiguration typeConfiguration) {
-		final MappingModelExpressible<?> inferredType = inferredTypeSupplier.get();
+		final MappingModelExpressible<?> inferredType = converter == null
+				? null
+				: converter.resolveFunctionImpliedReturnType();
 		if ( inferredType != null ) {
 			if ( inferredType instanceof ReturnableType<?> ) {
 				return (ReturnableType<?>) inferredType;

diff --git a/.../java/org/hibernate/dialect/function/array/ElementViaArrayArgumentReturnTypeResolver.java b/.../java/org/hibernate/dialect/function/array/ElementViaArrayArgumentReturnTypeResolver.java
@@ -15,11 +15,14 @@
 import org.hibernate.query.ReturnableType;
 import org.hibernate.query.sqm.SqmExpressible;
 import org.hibernate.query.sqm.produce.function.FunctionReturnTypeResolver;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmTypedNode;
 import org.hibernate.sql.ast.tree.SqlAstNode;
 import org.hibernate.type.BasicPluralType;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * A {@link FunctionReturnTypeResolver} that resolves the array element type based on an argument.
  * The inferred type and implied type have precedence though.
@@ -37,10 +40,12 @@ private ElementViaArrayArgumentReturnTypeResolver(int arrayIndex) {
 	@Override
 	public ReturnableType<?> resolveFunctionReturnType(
 			ReturnableType<?> impliedType,
-			Supplier<MappingModelExpressible<?>> inferredTypeSupplier,
+			@Nullable SqmToSqlAstConverter converter,
 			List<? extends SqmTypedNode<?>> arguments,
 			TypeConfiguration typeConfiguration) {
-		final MappingModelExpressible<?> inferredType = inferredTypeSupplier.get();
+		final MappingModelExpressible<?> inferredType = converter == null
+				? null
+				: converter.resolveFunctionImpliedReturnType();
 		if ( inferredType != null ) {
 			if ( inferredType instanceof ReturnableType<?> ) {
 				return (ReturnableType<?>) inferredType;

diff --git a/...ore/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractEmbeddableMapping.java b/...ore/src/main/java/org/hibernate/metamodel/mapping/internal/AbstractEmbeddableMapping.java
@@ -278,6 +278,7 @@ protected boolean finishInitialization(
 					containingTableExpression = rootTableExpression;
 					columnExpression = rootTableKeyColumnNames[ columnPosition ];
 				}
+				final NavigableRole role = navigableRole.append( bootPropertyDescriptor.getName() );
 				final SelectablePath selectablePath;
 				final String columnDefinition;
 				final Long length;
@@ -310,18 +311,18 @@ protected boolean finishInitialization(
 
 				attributeMapping = MappingModelCreationHelper.buildBasicAttributeMapping(
 						bootPropertyDescriptor.getName(),
-						navigableRole.append( bootPropertyDescriptor.getName() ),
+						role,
 						attributeIndex,
 						attributeIndex,
 						bootPropertyDescriptor,
 						declarer,
-						(BasicType<?>) subtype,
+						basicValue.getResolution().getLegacyResolvedBasicType(),
 						containingTableExpression,
 						columnExpression,
 						selectablePath,
 						selectable.isFormula(),
 						selectable.getCustomReadExpression(),
-						selectable.getWriteExpr( ( (BasicType<?>) subtype ).getJdbcMapping(), dialect ),
+						selectable.getWriteExpr( basicValue.getResolution().getJdbcMapping(), dialect ),
 						columnDefinition,
 						length,
 						precision,

diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java
@@ -5702,6 +5702,7 @@ protected AttributeMapping generateNonIdAttributeMapping(
 		}
 
 		if ( attrType instanceof BasicType ) {
+			final NavigableRole role = getNavigableRole().append( bootProperty.getName() );
 			final String attrColumnExpression;
 			final boolean isAttrColumnExpressionFormula;
 			final String customReadExpr;
@@ -5774,12 +5775,12 @@ protected AttributeMapping generateNonIdAttributeMapping(
 
 			return MappingModelCreationHelper.buildBasicAttributeMapping(
 					attrName,
-					getNavigableRole().append( bootProperty.getName() ),
+					role,
 					stateArrayPosition,
 					fetchableIndex,
 					bootProperty,
 					this,
-					(BasicType<?>) attrType,
+					(BasicType<?>) value.getType(),
 					tableExpression,
 					attrColumnExpression,
 					null,

diff --git a/hibernate-core/src/main/java/org/hibernate/query/criteria/JpaTupleElement.java b/hibernate-core/src/main/java/org/hibernate/query/criteria/JpaTupleElement.java
@@ -7,6 +7,7 @@
 package org.hibernate.query.criteria;
 
 import jakarta.persistence.TupleElement;
+import org.checkerframework.checker.nullness.qual.Nullable;
 
 import org.hibernate.type.descriptor.java.EnumJavaType;
 import org.hibernate.type.descriptor.java.JavaType;
@@ -17,10 +18,10 @@
  * @author Steve Ebersole
  */
 public interface JpaTupleElement<T> extends TupleElement<T>, JpaCriteriaNode {
-	JavaType<T> getJavaTypeDescriptor();
+	@Nullable JavaType<T> getJavaTypeDescriptor();
 
 	@Override
-	default Class<? extends T> getJavaType() {
+	default @Nullable Class<? extends T> getJavaType() {
 		// todo (6.0) : can this signature just return `Class<T>`?
 		return getJavaTypeDescriptor() == null ? null : getJavaTypeDescriptor().getJavaTypeClass();
 	}

diff --git a/...src/main/java/org/hibernate/query/sqm/function/SelfRenderingFunctionSqlAstExpression.java b/...src/main/java/org/hibernate/query/sqm/function/SelfRenderingFunctionSqlAstExpression.java
@@ -33,6 +33,8 @@
 import org.hibernate.type.descriptor.java.JavaType;
 import org.hibernate.type.spi.TypeConfiguration;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * Representation of a function call in the SQL AST for impls that know how to
  * render themselves.
@@ -44,8 +46,8 @@ public class SelfRenderingFunctionSqlAstExpression
 	private final String functionName;
 	private final FunctionRenderer renderer;
 	private final List<? extends SqlAstNode> sqlAstArguments;
-	private final ReturnableType<?> type;
-	private final JdbcMappingContainer expressible;
+	private final @Nullable ReturnableType<?> type;
+	private final @Nullable JdbcMappingContainer expressible;
 
 	/**
 	 * @deprecated Use {@link #SelfRenderingFunctionSqlAstExpression(String, FunctionRenderer, List, ReturnableType, JdbcMappingContainer)} instead
@@ -55,8 +57,8 @@ public SelfRenderingFunctionSqlAstExpression(
 			String functionName,
 			FunctionRenderingSupport renderer,
 			List<? extends SqlAstNode> sqlAstArguments,
-			ReturnableType<?> type,
-			JdbcMappingContainer expressible) {
+			@Nullable ReturnableType<?> type,
+			@Nullable JdbcMappingContainer expressible) {
 		this.functionName = functionName;
 		this.renderer = renderer::render;
 		this.sqlAstArguments = sqlAstArguments;
@@ -69,8 +71,8 @@ public SelfRenderingFunctionSqlAstExpression(
 			String functionName,
 			FunctionRenderer renderer,
 			List<? extends SqlAstNode> sqlAstArguments,
-			ReturnableType<?> type,
-			JdbcMappingContainer expressible) {
+			@Nullable ReturnableType<?> type,
+			@Nullable JdbcMappingContainer expressible) {
 		this.functionName = functionName;
 		this.renderer = renderer;
 		this.sqlAstArguments = sqlAstArguments;
@@ -108,7 +110,7 @@ protected FunctionRenderer getFunctionRenderer() {
 		return renderer;
 	}
 
-	protected ReturnableType<?> getType() {
+	protected @Nullable ReturnableType<?> getType() {
 		return type;
 	}
 

diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/NoParamSqmCopyContext.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/NoParamSqmCopyContext.java
@@ -8,12 +8,14 @@
 
 import org.hibernate.query.sqm.tree.expression.SqmParameter;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * @author Marco Belladelli
  */
 public class NoParamSqmCopyContext extends SimpleSqmCopyContext {
 	@Override
-	public <T> T getCopy(T original) {
+	public <T> @Nullable T getCopy(T original) {
 		if ( original instanceof SqmParameter<?> ) {
 			return original;
 		}

diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleSqmCopyContext.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SimpleSqmCopyContext.java
@@ -10,6 +10,8 @@
 
 import org.hibernate.query.sqm.tree.SqmCopyContext;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
+
 /**
  * @author Marco Belladelli
  */
@@ -18,7 +20,7 @@ public class SimpleSqmCopyContext implements SqmCopyContext {
 
 	@Override
 	@SuppressWarnings( "unchecked" )
-	public <T> T getCopy(T original) {
+	public <T> @Nullable T getCopy(T original) {
 		return (T) map.get( original );
 	}
 

diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/TypecheckUtil.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/TypecheckUtil.java
@@ -336,10 +336,12 @@ private static EntityPersister getEntityDescriptor(SessionFactoryImplementor fac
 	 * @see TypecheckUtil#assertAssignable(String, SqmPath, SqmTypedNode, SessionFactoryImplementor)
 	 */
 	public static void assertComparable(Expression<?> x, Expression<?> y, SessionFactoryImplementor factory) {
-		SqmExpression<?> left = (SqmExpression<?>) x;
-		SqmExpression<?> right = (SqmExpression<?>) y;
-		if (  left.getTupleLength() != null && right.getTupleLength() != null
-				&& left.getTupleLength().intValue() != right.getTupleLength().intValue() ) {
+		final SqmExpression<?> left = (SqmExpression<?>) x;
+		final SqmExpression<?> right = (SqmExpression<?>) y;
+		final Integer leftTupleLength = left.getTupleLength();
+		final Integer rightTupleLength;
+		if (  leftTupleLength != null && ( rightTupleLength = right.getTupleLength() ) != null
+				&& leftTupleLength.intValue() != rightTupleLength.intValue() ) {
 			throw new SemanticException( "Cannot compare tuples of different lengths" );
 		}