From 1e4b9e8ffb8aeee024faa2160f3e79019564e424 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 29 Jun 2023 00:36:29 +0200 Subject: [PATCH] HHH-16858 improve typechecking for comparisons/assignments (#6910) * HHH-16858 improve typechecking for comparisons/assignments In particular, correctly typecheck comparisons between enums and other enums, and literal integers / strings. Actually I'm not a great fan of comparing enums with int/string literals but since we used to support it in 5, and kinda mostly support it in earlier releases of 6, on balance we might as well continue to allow it. * improve typechecking for arguments to min() & max() - use the known JdbcType which previously we didn't have proper access to - and accidentally fix HHH-16859 by side-effect (I didn't really want to fix that one, but it was easier to fix it than to unfix it.) * HHH-16858 handle MySQL enum types correctly in comparison typecheck --- .../org/hibernate/query/OutputableType.java | 4 +- .../hql/internal/SemanticQueryBuilder.java | 140 +++++++----------- .../query/sqm/internal/QuerySqmImpl.java | 46 +++++- .../sqm/internal/SqmCriteriaNodeBuilder.java | 47 ++++-- .../function/ArgumentTypesValidator.java | 65 +++++--- .../descriptor/java/CurrencyJavaType.java | 11 -- .../type/descriptor/java/EnumJavaType.java | 22 +-- .../java/JdbcTimestampJavaType.java | 1 - .../type/descriptor/jdbc/JdbcType.java | 6 + .../orm/test/jpa/CompositeIdFkUpdateTest.java | 2 +- .../test/query/hql/EnumComparisonTest.java | 63 ++++++++ 11 files changed, 238 insertions(+), 169 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/EnumComparisonTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/OutputableType.java b/hibernate-core/src/main/java/org/hibernate/query/OutputableType.java index 26b56b7b90..8871528f16 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/OutputableType.java +++ b/hibernate-core/src/main/java/org/hibernate/query/OutputableType.java @@ -14,8 +14,8 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.type.descriptor.jdbc.JdbcType; /** - * Specialization of DomainType for types that can be used as a - * parameter output for a {@link org.hibernate.procedure.ProcedureCall} + * Specialization of {@link org.hibernate.metamodel.model.domain.DomainType} for types that + * can be used as a parameter output for a {@link org.hibernate.procedure.ProcedureCall}. * * @apiNote We assume a type that maps to exactly one SQL value, hence {@link #getJdbcType()} * diff --git a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java index 04390d1c7b..4c52d6a4fd 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java @@ -434,22 +434,12 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public SqmRoot visitTargetEntity(HqlParser.TargetEntityContext dmlTargetContext) { - final HqlParser.EntityNameContext entityNameContext = (HqlParser.EntityNameContext) dmlTargetContext.getChild( 0 ); - final String identificationVariable; - if ( dmlTargetContext.getChildCount() == 1 ) { - identificationVariable = null; - } - else { - identificationVariable = applyJpaCompliance( - visitVariable( - (HqlParser.VariableContext) dmlTargetContext.getChild( 1 ) - ) - ); - } + final HqlParser.EntityNameContext entityNameContext = dmlTargetContext.entityName(); + final HqlParser.VariableContext variable = dmlTargetContext.variable(); //noinspection unchecked return new SqmRoot<>( (EntityDomainType) visitEntityName( entityNameContext ), - identificationVariable, + variable == null ? null : applyJpaCompliance( visitVariable( variable ) ), false, creationContext.getNodeBuilder() ); @@ -457,17 +447,8 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public SqmInsertStatement visitInsertStatement(HqlParser.InsertStatementContext ctx) { - final int dmlTargetIndex; - if ( ctx.getChild( 1 ) instanceof HqlParser.TargetEntityContext ) { - dmlTargetIndex = 1; - } - else { - dmlTargetIndex = 2; - } - final HqlParser.TargetEntityContext dmlTargetContext = (HqlParser.TargetEntityContext) ctx.getChild( dmlTargetIndex ); - final HqlParser.TargetFieldsContext targetFieldsSpecContext = (HqlParser.TargetFieldsContext) ctx.getChild( - dmlTargetIndex + 1 - ); + final HqlParser.TargetEntityContext dmlTargetContext = ctx.targetEntity(); + final HqlParser.TargetFieldsContext targetFieldsSpecContext = ctx.targetFields(); final SqmRoot root = visitTargetEntity( dmlTargetContext ); if ( root.getModel() instanceof SqmPolymorphicRootDescriptor ) { throw new SemanticException( @@ -480,7 +461,8 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem final HqlParser.QueryExpressionContext queryExpressionContext = ctx.queryExpression(); if ( queryExpressionContext != null ) { - final SqmInsertSelectStatement insertStatement = new SqmInsertSelectStatement<>( root, creationContext.getNodeBuilder() ); + final SqmInsertSelectStatement insertStatement = + new SqmInsertSelectStatement<>( root, creationContext.getNodeBuilder() ); parameterCollector = insertStatement; final SqmDmlCreationProcessingState processingState = new SqmDmlCreationProcessingState( insertStatement, @@ -517,7 +499,8 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem } else { - final SqmInsertValuesStatement insertStatement = new SqmInsertValuesStatement<>( root, creationContext.getNodeBuilder() ); + final SqmInsertValuesStatement insertStatement = + new SqmInsertValuesStatement<>( root, creationContext.getNodeBuilder() ); parameterCollector = insertStatement; final SqmDmlCreationProcessingState processingState = new SqmDmlCreationProcessingState( insertStatement, @@ -553,9 +536,8 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public SqmUpdateStatement visitUpdateStatement(HqlParser.UpdateStatementContext ctx) { - final boolean versioned = !( ctx.getChild( 1 ) instanceof HqlParser.TargetEntityContext ); - final int dmlTargetIndex = versioned ? 2 : 1; - final HqlParser.TargetEntityContext dmlTargetContext = (HqlParser.TargetEntityContext) ctx.getChild( dmlTargetIndex ); + final boolean versioned = ctx.VERSIONED() != null; + final HqlParser.TargetEntityContext dmlTargetContext = ctx.targetEntity(); final SqmRoot root = visitTargetEntity( dmlTargetContext ); if ( root.getModel() instanceof SqmPolymorphicRootDescriptor ) { throw new SemanticException( @@ -577,12 +559,12 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem try { updateStatement.versioned( versioned ); - final HqlParser.SetClauseContext setClauseCtx = (HqlParser.SetClauseContext) ctx.getChild( dmlTargetIndex + 1 ); + final HqlParser.SetClauseContext setClauseCtx = ctx.setClause(); for ( ParseTree subCtx : setClauseCtx.children ) { if ( subCtx instanceof HqlParser.AssignmentContext ) { final HqlParser.AssignmentContext assignmentContext = (HqlParser.AssignmentContext) subCtx; //noinspection unchecked - final SqmPath targetPath = (SqmPath) consumeDomainPath( (HqlParser.SimplePathContext) assignmentContext.getChild( 0 ) ); + final SqmPath targetPath = (SqmPath) consumeDomainPath( assignmentContext.simplePath() ); final Class targetPathJavaType = targetPath.getJavaType(); final boolean isEnum = targetPathJavaType != null && targetPathJavaType.isEnum(); final ParseTree rightSide = assignmentContext.getChild( 2 ); @@ -604,10 +586,9 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem } } - if ( dmlTargetIndex + 2 <= ctx.getChildCount() ) { - updateStatement.applyPredicate( - visitWhereClause( (HqlParser.WhereClauseContext) ctx.getChild( dmlTargetIndex + 2 ) ) - ); + final HqlParser.WhereClauseContext whereClauseContext = ctx.whereClause(); + if ( whereClauseContext != null ) { + updateStatement.applyPredicate( visitWhereClause( whereClauseContext ) ); } return updateStatement; @@ -619,14 +600,7 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem @Override public SqmDeleteStatement visitDeleteStatement(HqlParser.DeleteStatementContext ctx) { - final int dmlTargetIndex; - if ( ctx.getChild( 1 ) instanceof HqlParser.TargetEntityContext ) { - dmlTargetIndex = 1; - } - else { - dmlTargetIndex = 2; - } - final HqlParser.TargetEntityContext dmlTargetContext = (HqlParser.TargetEntityContext) ctx.getChild( dmlTargetIndex ); + final HqlParser.TargetEntityContext dmlTargetContext = ctx.targetEntity(); final SqmRoot root = visitTargetEntity( dmlTargetContext ); final SqmDeleteStatement deleteStatement = new SqmDeleteStatement<>( root, SqmQuerySource.HQL, creationContext.getNodeBuilder() ); @@ -642,10 +616,9 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem processingStateStack.push( sqmDeleteCreationState ); try { - if ( dmlTargetIndex + 1 <= ctx.getChildCount() ) { - deleteStatement.applyPredicate( - visitWhereClause( (HqlParser.WhereClauseContext) ctx.getChild( dmlTargetIndex + 1 ) ) - ); + final HqlParser.WhereClauseContext whereClauseContext = ctx.whereClause(); + if ( whereClauseContext != null ) { + deleteStatement.applyPredicate( visitWhereClause( whereClauseContext ) ); } return deleteStatement; @@ -2472,50 +2445,37 @@ public class SemanticQueryBuilder extends HqlParserBaseVisitor implem HqlParser.ExpressionContext rightExpressionContext) { final SqmExpression right; final SqmExpression left; - switch ( comparisonOperator ) { - case EQUAL: - case NOT_EQUAL: - case DISTINCT_FROM: - case NOT_DISTINCT_FROM: { - Map, Enum> possibleEnumValues; - if ( ( possibleEnumValues = getPossibleEnumValues( leftExpressionContext ) ) != null ) { - right = (SqmExpression) rightExpressionContext.accept( this ); - left = resolveEnumShorthandLiteral( - leftExpressionContext, - possibleEnumValues, - right.getJavaType() - ); - break; - } - else if ( ( possibleEnumValues = getPossibleEnumValues( rightExpressionContext ) ) != null ) { - left = (SqmExpression) leftExpressionContext.accept( this ); - right = resolveEnumShorthandLiteral( - rightExpressionContext, - possibleEnumValues, - left.getJavaType() - ); - break; - } - final SqmExpression l = (SqmExpression) leftExpressionContext.accept( this ); - final SqmExpression r = (SqmExpression) rightExpressionContext.accept( this ); - if ( l instanceof AnyDiscriminatorSqmPath && r instanceof SqmLiteralEntityType ) { - left = l; - right = createDiscriminatorValue( (AnyDiscriminatorSqmPath) left, rightExpressionContext ); - } - else if ( r instanceof AnyDiscriminatorSqmPath && l instanceof SqmLiteralEntityType ) { - left = createDiscriminatorValue( (AnyDiscriminatorSqmPath) r, leftExpressionContext ); - right = r; - } - else { - left = l; - right = r; - } - break; + Map, Enum> possibleEnumValues; + if ( ( possibleEnumValues = getPossibleEnumValues( leftExpressionContext ) ) != null ) { + right = (SqmExpression) rightExpressionContext.accept( this ); + left = resolveEnumShorthandLiteral( + leftExpressionContext, + possibleEnumValues, + right.getJavaType() + ); + } + else if ( ( possibleEnumValues = getPossibleEnumValues( rightExpressionContext ) ) != null ) { + left = (SqmExpression) leftExpressionContext.accept( this ); + right = resolveEnumShorthandLiteral( + rightExpressionContext, + possibleEnumValues, + left.getJavaType() + ); + } + else { + final SqmExpression l = (SqmExpression) leftExpressionContext.accept( this ); + final SqmExpression r = (SqmExpression) rightExpressionContext.accept( this ); + if ( l instanceof AnyDiscriminatorSqmPath && r instanceof SqmLiteralEntityType ) { + left = l; + right = createDiscriminatorValue( (AnyDiscriminatorSqmPath) left, rightExpressionContext ); } - default: { - left = (SqmExpression) leftExpressionContext.accept( this ); - right = (SqmExpression) rightExpressionContext.accept( this ); - break; + else if ( r instanceof AnyDiscriminatorSqmPath && l instanceof SqmLiteralEntityType ) { + left = createDiscriminatorValue( (AnyDiscriminatorSqmPath) r, leftExpressionContext ); + right = r; + } + else { + left = l; + right = r; } } SqmCriteriaNodeBuilder.assertComparable( left, right ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java index aaa44d1722..909565cf2b 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java @@ -43,7 +43,9 @@ import org.hibernate.id.enhanced.Optimizer; import org.hibernate.internal.CoreLogging; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.metamodel.mapping.EntityIdentifierMapping; +import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.mapping.internal.SingleAttributeIdentifierMapping; +import org.hibernate.metamodel.model.domain.DomainType; import org.hibernate.metamodel.model.domain.EntityDomainType; import org.hibernate.persister.entity.AbstractEntityPersister; import org.hibernate.persister.entity.EntityPersister; @@ -79,6 +81,7 @@ import org.hibernate.query.spi.ScrollableResultsImplementor; import org.hibernate.query.spi.SelectQueryPlan; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SortOrder; +import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.SqmPathSource; import org.hibernate.query.sqm.internal.SqmInterpretationsKey.InterpretationsKeySource; import org.hibernate.query.sqm.mutation.spi.SqmMultiTableMutationStrategy; @@ -113,6 +116,8 @@ import jakarta.persistence.LockModeType; import jakarta.persistence.Parameter; import jakarta.persistence.PersistenceException; import jakarta.persistence.TemporalType; +import org.hibernate.type.descriptor.java.JavaType; +import org.hibernate.type.descriptor.jdbc.JdbcType; import static org.hibernate.jpa.HibernateHints.HINT_CACHEABLE; import static org.hibernate.jpa.HibernateHints.HINT_CACHE_MODE; @@ -130,6 +135,7 @@ import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.createInter import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.generateNonSelectKey; import static org.hibernate.query.sqm.internal.SqmUtil.isSelect; import static org.hibernate.query.sqm.internal.SqmUtil.verifyIsNonSelectStatement; +import static org.hibernate.type.descriptor.java.JavaTypeHelper.isUnknown; /** * {@link Query} implementation based on an SQM @@ -351,17 +357,13 @@ public class QuerySqmImpl final SqmAssignment assignment = assignments.get( i ); final SqmPath targetPath = assignment.getTargetPath(); final SqmExpression expression = assignment.getValue(); - if ( targetPath.getNodeJavaType() == null || expression.getNodeJavaType() == null ) { - continue; - } - if ( targetPath.getNodeJavaType() != expression.getNodeJavaType() - && !targetPath.getNodeJavaType().isWider( expression.getNodeJavaType() ) ) { + if ( !isAssignable( targetPath, expression ) ) { throw new SemanticException( String.format( - "The assignment expression type [%s] did not match the assignment path type [%s] for the path [%s]", + "Cannot assign expression of type '%s' to target path '%s' of type '%s'", expression.getNodeJavaType().getJavaType().getTypeName(), - targetPath.getNodeJavaType().getJavaType().getTypeName(), - targetPath.toHqlString() + targetPath.toHqlString(), + targetPath.getNodeJavaType().getJavaType().getTypeName() ), hqlString, null @@ -370,6 +372,34 @@ public class QuerySqmImpl } } + /** + * @see SqmCriteriaNodeBuilder#areTypesComparable(SqmExpressible, SqmExpressible) + */ + public static boolean isAssignable(SqmPath targetPath, SqmExpression expression) { + DomainType lhsDomainType = targetPath.getExpressible().getSqmType(); + DomainType rhsDomainType = expression.getExpressible().getSqmType(); + if ( lhsDomainType instanceof JdbcMapping && rhsDomainType instanceof JdbcMapping ) { + JdbcType lhsJdbcType = ((JdbcMapping) lhsDomainType).getJdbcType(); + JdbcType rhsJdbcType = ((JdbcMapping) rhsDomainType).getJdbcType(); + if ( lhsJdbcType.getJdbcTypeCode() == rhsJdbcType.getJdbcTypeCode() + || lhsJdbcType.isStringLike() && rhsJdbcType.isStringLike() + || lhsJdbcType.isInteger() && rhsJdbcType.isInteger() ) { + return true; + } + } + + JavaType targetType = targetPath.getNodeJavaType(); + JavaType assignedType = expression.getNodeJavaType(); + return targetType == assignedType + // If we don't know the java types, let's just be lenient + || isUnknown( targetType) + || isUnknown( assignedType ) + // Assume we can coerce one to another + || targetType.isWider( assignedType ) + // Enum assignment, other strange user type mappings + || targetType.getJavaTypeClass().isAssignableFrom( assignedType.getJavaTypeClass() ); + } + private void verifyInsertTypesMatch(String hqlString, SqmInsertStatement sqmStatement) { final List> insertionTargetPaths = sqmStatement.getInsertionTargetPaths(); if ( sqmStatement instanceof SqmInsertValuesStatement ) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java index f217fc570a..3f4c917498 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java @@ -40,6 +40,7 @@ import org.hibernate.internal.CoreMessageLogger; import org.hibernate.internal.SessionFactoryRegistry; import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.collections.CollectionHelper; +import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.model.domain.BasicDomainType; import org.hibernate.metamodel.model.domain.DomainType; import org.hibernate.metamodel.model.domain.JpaMetamodel; @@ -156,7 +157,7 @@ import org.hibernate.type.BasicType; import org.hibernate.type.StandardBasicTypes; import org.hibernate.type.descriptor.java.EnumJavaType; import org.hibernate.type.descriptor.java.JavaType; -import org.hibernate.type.descriptor.java.JavaTypeHelper; +import org.hibernate.type.descriptor.jdbc.JdbcType; import org.hibernate.type.spi.TypeConfiguration; import jakarta.persistence.Tuple; @@ -178,6 +179,7 @@ import jakarta.persistence.metamodel.EntityType; import static java.util.Arrays.asList; import static org.hibernate.query.internal.QueryHelper.highestPrecedenceType; import static org.hibernate.query.sqm.TrimSpec.fromCriteriaTrimSpec; +import static org.hibernate.type.descriptor.java.JavaTypeHelper.isUnknown; /** * Acts as a JPA {@link jakarta.persistence.criteria.CriteriaBuilder} by @@ -240,8 +242,12 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return sessionFactory.get(); } + /** + * @see QuerySqmImpl#isAssignable(SqmPath, SqmExpression) + */ public static boolean areTypesComparable(SqmExpressible lhsType, SqmExpressible rhsType) { - if ( lhsType == null || rhsType == null || lhsType == rhsType + if ( lhsType == null || rhsType == null + || lhsType == rhsType || isDiscriminatorComparison( lhsType, rhsType ) // Allow comparing an embeddable against a tuple literal || lhsType instanceof EmbeddedSqmPathSource && rhsType instanceof TupleType @@ -252,21 +258,34 @@ public class SqmCriteriaNodeBuilder implements NodeBuilder, SqmCreationContext, return true; } + DomainType lhsDomainType = lhsType.getSqmType(); + DomainType rhsDomainType = rhsType.getSqmType(); + if ( lhsDomainType instanceof JdbcMapping && rhsDomainType instanceof JdbcMapping ) { + JdbcType lhsJdbcType = ((JdbcMapping) lhsDomainType).getJdbcType(); + JdbcType rhsJdbcType = ((JdbcMapping) rhsDomainType).getJdbcType(); + if ( lhsJdbcType.getJdbcTypeCode() == rhsJdbcType.getJdbcTypeCode() + || lhsJdbcType.isStringLike() && rhsJdbcType.isStringLike() + || lhsJdbcType.isTemporal() && rhsJdbcType.isTemporal() + || lhsJdbcType.isNumber() && rhsJdbcType.isNumber() ) { + return true; + } + } + final JavaType lhsJavaType = lhsType.getExpressibleJavaType(); final JavaType rhsJavaType = rhsType.getExpressibleJavaType(); - return lhsJavaType == rhsJavaType - // If we don't know the java types, let's just be lenient - || JavaTypeHelper.isUnknown( lhsJavaType ) - || JavaTypeHelper.isUnknown( rhsJavaType ) - // Allow comparing two temporal expressions regardless of their concrete java types - || JavaTypeHelper.isTemporal( lhsJavaType ) && JavaTypeHelper.isTemporal( rhsJavaType ) - // Assume we can coerce one to another - || lhsJavaType.isWider( rhsJavaType ) - || rhsJavaType.isWider( lhsJavaType ) - // Polymorphic entity comparison - || lhsJavaType.getJavaTypeClass().isAssignableFrom( rhsJavaType.getJavaTypeClass() ) - || rhsJavaType.getJavaTypeClass().isAssignableFrom( lhsJavaType.getJavaTypeClass() ); + // If we don't know the java types, let's just be lenient + || isUnknown( lhsJavaType ) + || isUnknown( rhsJavaType ) + // Allow comparing two temporal expressions regardless of their concrete java types + || lhsJavaType.isTemporalType() && rhsJavaType.isTemporalType() + // Assume we can coerce one to another + || lhsJavaType.isWider( rhsJavaType ) + || rhsJavaType.isWider( lhsJavaType ) + // Enum comparison, other strange user type mappings, + // Polymorphic entity comparison + || lhsJavaType.getJavaTypeClass().isAssignableFrom( rhsJavaType.getJavaTypeClass() ) + || rhsJavaType.getJavaTypeClass().isAssignableFrom( lhsJavaType.getJavaTypeClass() ); } private static boolean isDiscriminatorComparison(SqmExpressible lhsType, SqmExpressible rhsType) { diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/produce/function/ArgumentTypesValidator.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/produce/function/ArgumentTypesValidator.java index 84f76163ac..133d987687 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/produce/function/ArgumentTypesValidator.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/produce/function/ArgumentTypesValidator.java @@ -10,10 +10,10 @@ import java.lang.reflect.Type; import java.sql.Types; import java.util.List; -import org.hibernate.QueryException; import org.hibernate.metamodel.MappingMetamodel; import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.mapping.JdbcMappingContainer; +import org.hibernate.metamodel.model.domain.DomainType; import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.tree.SqmTypedNode; import org.hibernate.query.sqm.tree.expression.SqmCollation; @@ -95,16 +95,7 @@ public class ArgumentTypesValidator implements ArgumentsValidator { if ( nodeType != null ) { JavaType javaType = nodeType.getRelationalJavaType(); if (javaType != null) { - try { - checkType( - count, functionName, type, - getJdbcType( indicators, javaType ), - javaType.getJavaTypeClass() - ); - } - catch (JdbcTypeRecommendationException e) { - // it's a converter or something like that, and we will check it later - } + checkArgumentType( functionName, count, argument, indicators, type, javaType ); } switch (type) { case TEMPORAL_UNIT: @@ -134,9 +125,39 @@ public class ArgumentTypesValidator implements ArgumentsValidator { } } + private void checkArgumentType( + String functionName, + int count, + SqmTypedNode argument, + JdbcTypeIndicators indicators, + FunctionParameterType type, + JavaType javaType) { + DomainType domainType = argument.getExpressible().getSqmType(); + if ( domainType instanceof JdbcMapping ) { + checkArgumentType( + count, functionName, type, + ((JdbcMapping) domainType).getJdbcType().getDefaultSqlTypeCode(), + javaType.getJavaTypeClass() + ); + } + else { + //TODO: this branch is now probably obsolete and can be deleted! + try { + checkArgumentType( + count, functionName, type, + getJdbcType( indicators, javaType ), + javaType.getJavaTypeClass() + ); + } + catch (JdbcTypeRecommendationException e) { + // it's a converter or something like that, and we will check it later + } + } + } + private int getJdbcType(JdbcTypeIndicators indicators, JavaType javaType) { if ( javaType.getJavaTypeClass().isEnum() ) { - // magic value indicates it can be coerced STRING or ORDINAL + // we can't tell if the enum is mapped STRING or ORDINAL return ENUM_UNKNOWN_JDBC_TYPE; } else { @@ -177,7 +198,7 @@ public class ArgumentTypesValidator implements ArgumentsValidator { final JdbcMapping mapping = expressionType.getJdbcMapping( i ); FunctionParameterType type = count < types.length ? types[count++] : types[types.length - 1]; if (type != null) { - checkType( + checkArgumentType( count, functionName, type, @@ -189,19 +210,21 @@ public class ArgumentTypesValidator implements ArgumentsValidator { return count; } - private void checkType(int count, String functionName, FunctionParameterType type, int code, Type javaType) { + private void checkArgumentType(int count, String functionName, FunctionParameterType type, int code, Type javaType) { switch (type) { case COMPARABLE: - if ( !isCharacterType(code) && !isTemporalType(code) && !isNumericType(code) && code != UUID ) { - if ( javaType == java.util.UUID.class && ( code == Types.BINARY || isCharacterType( code ) ) ) { - // We also consider UUID to be comparable when it's a character or binary type - return; - } + if ( !isCharacterType(code) && !isTemporalType(code) && !isNumericType(code) && !isEnumType( code ) + // both Java and the database consider UUIDs + // comparable, so go ahead and accept them + && code != UUID + // as a special case, we consider a binary column + // comparable when it is mapped by a Java UUID + && !( javaType == java.util.UUID.class && code == Types.BINARY ) ) { throwError(type, javaType, functionName, count); } break; case STRING: - if ( !isCharacterType(code) && !isEnumType(code) && code != ENUM_UNKNOWN_JDBC_TYPE) { + if ( !isCharacterType(code) && !isEnumType(code) ) { throwError(type, javaType, functionName, count); } break; @@ -216,7 +239,7 @@ public class ArgumentTypesValidator implements ArgumentsValidator { } break; case INTEGER: - if ( !isIntegral(code) && code != ENUM_UNKNOWN_JDBC_TYPE ) { + if ( !isIntegral(code) ) { throwError(type, javaType, functionName, count); } break; diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CurrencyJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CurrencyJavaType.java index 9ea26939e7..50ea82344e 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CurrencyJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/CurrencyJavaType.java @@ -60,15 +60,4 @@ public class CurrencyJavaType extends AbstractClassJavaType { public long getDefaultSqlLength(Dialect dialect, JdbcType jdbcType) { return 3; } - - @Override - public boolean isWider(JavaType javaType) { - // This is necessary to allow comparing/assigning a currency attribute against a literal of the JdbcType - switch ( javaType.getJavaType().getTypeName() ) { - case "java.lang.String": - return true; - default: - return false; - } - } } diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java index af653c4e6a..136b539b3b 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java @@ -230,7 +230,7 @@ public class EnumJavaType> extends AbstractClassJavaType { } /** - * Interpret a String value as the named value of the enum type + * Interpret a string value as the named value of the enum type */ public T fromName(String relationalForm) { if ( relationalForm == null ) { @@ -239,26 +239,6 @@ public class EnumJavaType> extends AbstractClassJavaType { return Enum.valueOf( getJavaTypeClass(), relationalForm.trim() ); } - @Override - public boolean isWider(JavaType javaType) { - // This is necessary to allow comparing/assigning an enum attribute against a literal of the JdbcType - switch ( javaType.getJavaType().getTypeName() ) { - case "byte": - case "java.lang.Byte": - case "short": - case "java.lang.Short": - case "int": - case "java.lang.Integer": - case "long": - case "java.lang.Long": - case "java.lang.String": - case "java.lang.Character": - return true; - default: - return false; - } - } - @Override public String getCheckCondition(String columnName, JdbcType jdbcType, BasicValueConverter converter, Dialect dialect) { if ( converter != null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java index 452b4fb6e1..66d5890558 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/JdbcTimestampJavaType.java @@ -10,7 +10,6 @@ import java.sql.Timestamp; import java.sql.Types; import java.time.Instant; import java.time.LocalDateTime; -import java.time.LocalDateTime; import java.time.ZoneId; import java.time.ZoneOffset; import java.time.format.DateTimeFormatter; diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/JdbcType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/JdbcType.java index 330746ba48..fda5267cb8 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/JdbcType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/jdbc/JdbcType.java @@ -223,6 +223,12 @@ public interface JdbcType extends Serializable { return isCharacterOrClobType( getDdlTypeCode() ); } + default boolean isStringLike() { + int ddlTypeCode = getDdlTypeCode(); + return isCharacterOrClobType( ddlTypeCode ) + || isEnumType( ddlTypeCode ); + } + default boolean isTemporal() { return isTemporalType( getDdlTypeCode() ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/CompositeIdFkUpdateTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/CompositeIdFkUpdateTest.java index 9c4fafbdef..bd0eae4579 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/CompositeIdFkUpdateTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/CompositeIdFkUpdateTest.java @@ -41,7 +41,7 @@ public class CompositeIdFkUpdateTest { } catch (IllegalArgumentException ex) { assertThat( ex.getCause() ).isInstanceOf( SemanticException.class ); - assertThat( ex.getCause() ).hasMessageContaining( "did not match" ); + assertThat( ex.getCause() ).hasMessageContaining( "Cannot assign expression of type" ); } } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/EnumComparisonTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/EnumComparisonTest.java new file mode 100644 index 0000000000..57c870e4d0 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/EnumComparisonTest.java @@ -0,0 +1,63 @@ +package org.hibernate.orm.test.query.hql; + +import jakarta.persistence.Entity; +import jakarta.persistence.EnumType; +import jakarta.persistence.Enumerated; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import org.hibernate.query.SemanticException; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +@SessionFactory +@DomainModel(annotatedClasses = EnumComparisonTest.WithEnum.class) +public class EnumComparisonTest { + @Test + void test(SessionFactoryScope scope) { + scope.inTransaction(session -> { + session.persist(new WithEnum()); + assertEquals(1, + session.createSelectionQuery("from WithEnum where stringEnum > X").getResultList().size()); + assertEquals(1, + session.createSelectionQuery("from WithEnum where ordinalEnum > X").getResultList().size()); + assertEquals(1, + session.createSelectionQuery("from WithEnum where stringEnum > 'X'").getResultList().size()); + assertEquals(1, + session.createSelectionQuery("from WithEnum where ordinalEnum > 1").getResultList().size()); + try { + session.createSelectionQuery("from WithEnum where ordinalEnum > 'X'").getResultList(); + fail(); + } + catch (SemanticException se) { + } + try { + session.createSelectionQuery("from WithEnum where stringEnum > 1").getResultList(); + fail(); + } + catch (SemanticException se) { + } + session.createSelectionQuery("select max(ordinalEnum) from WithEnum").getSingleResult(); + session.createSelectionQuery("select max(stringEnum) from WithEnum").getSingleResult(); + }); + } + + enum Enum { X, Y, Z } + + @Entity(name = "WithEnum") + static class WithEnum { + @Id + @GeneratedValue + long id; + + @Enumerated(EnumType.STRING) + Enum stringEnum = Enum.Y; + + @Enumerated(EnumType.ORDINAL) + Enum ordinalEnum = Enum.Z; + } +}