From bc901f516265b0ee6ded0419f083d2c184662c6f Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 29 Jun 2023 11:54:45 +0200 Subject: [PATCH] HHH-16875 improve typechecking for comparisons of embeddables, tuples, entities --- .../metamodel/internal/AttributeFactory.java | 1 - .../metamodel/internal/MetadataContext.java | 14 +- .../model/domain/internal/BasicTypeImpl.java | 30 +- .../internal/DiscriminatorSqmPathSource.java | 8 + .../internal/PrimitiveBasicTypeImpl.java | 13 +- .../hql/internal/SemanticQueryBuilder.java | 7 +- .../org/hibernate/query/sqm/NodeBuilder.java | 1 - .../query/sqm/internal/QuerySqmImpl.java | 79 +--- .../sqm/internal/SqmCriteriaNodeBuilder.java | 174 ++------- .../query/sqm/internal/TypecheckUtil.java | 360 ++++++++++++++++++ .../tree/predicate/SqmInListPredicate.java | 5 +- .../descriptor/java/spi/JavaTypeBaseline.java | 2 - .../hibernate/type/spi/TypeConfiguration.java | 2 +- .../orm/test/hql/BulkManipulationTest.java | 4 +- .../ObjectParameterTypeForEmbeddableTest.java | 11 +- .../converter/literal/QueryLiteralTest.java | 7 +- 16 files changed, 484 insertions(+), 234 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/query/sqm/internal/TypecheckUtil.java diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java index 7b38fa325a..9905911f24 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/AttributeFactory.java @@ -39,7 +39,6 @@ import org.hibernate.metamodel.model.domain.AbstractIdentifiableType; import org.hibernate.metamodel.model.domain.DomainType; import org.hibernate.metamodel.model.domain.EmbeddableDomainType; -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.PersistentAttribute; diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/MetadataContext.java b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/MetadataContext.java index 0a5a9d41e0..f4e0b31bbb 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/internal/MetadataContext.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/internal/MetadataContext.java @@ -28,7 +28,6 @@ import org.hibernate.mapping.MappedSuperclass; import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; -import org.hibernate.mapping.Value; import org.hibernate.metamodel.MappingMetamodel; import org.hibernate.metamodel.model.domain.AbstractIdentifiableType; import org.hibernate.metamodel.model.domain.BasicDomainType; @@ -51,6 +50,7 @@ import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.descriptor.java.spi.EntityJavaType; import org.hibernate.type.descriptor.java.spi.JavaTypeRegistry; +import org.hibernate.type.descriptor.jdbc.JdbcType; import org.hibernate.type.spi.TypeConfiguration; import jakarta.persistence.metamodel.Attribute; @@ -806,12 +806,14 @@ public BasicDomainType resolveBasicType(Class javaType) { return (BasicDomainType) basicDomainTypeMap.computeIfAbsent( javaType, jt -> { + // we cannot use getTypeConfiguration().standardBasicTypeForJavaType(javaType) + // because that doesn't return the right thing for primitive types final JavaTypeRegistry registry = getTypeConfiguration().getJavaTypeRegistry(); - - if ( javaType.isPrimitive() ) { - return new PrimitiveBasicTypeImpl<>( registry.resolveDescriptor( javaType ), javaType ); - } - return new BasicTypeImpl<>( registry.resolveDescriptor( javaType ) ); + JavaType javaTypeDescriptor = registry.resolveDescriptor( javaType ); + JdbcType jdbcType = javaTypeDescriptor.getRecommendedJdbcType( typeConfiguration.getCurrentBaseSqlTypeIndicators() ); + return javaType.isPrimitive() + ? new PrimitiveBasicTypeImpl<>( javaTypeDescriptor, jdbcType , javaType ) + : new BasicTypeImpl<>( javaTypeDescriptor, jdbcType ); } ); } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicTypeImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicTypeImpl.java index 8441fa5acb..a81b55a3fe 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicTypeImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/BasicTypeImpl.java @@ -11,18 +11,23 @@ import java.sql.SQLException; import org.hibernate.engine.spi.SharedSessionContractImplementor; +import org.hibernate.metamodel.mapping.JdbcMapping; import org.hibernate.metamodel.model.domain.BasicDomainType; +import org.hibernate.type.descriptor.ValueBinder; +import org.hibernate.type.descriptor.ValueExtractor; import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.descriptor.jdbc.JdbcType; /** * @author Emmanuel Bernard */ -public class BasicTypeImpl implements BasicDomainType, Serializable { +public class BasicTypeImpl implements BasicDomainType, JdbcMapping, Serializable { private final JavaType javaType; + private final JdbcType jdbcType; - public BasicTypeImpl(JavaType javaType) { + public BasicTypeImpl(JavaType javaType, JdbcType jdbcType) { this.javaType = javaType; + this.jdbcType = jdbcType; } public PersistenceType getPersistenceType() { @@ -41,12 +46,12 @@ public Class getJavaType() { @Override public boolean canDoExtraction() { - throw new UnsupportedOperationException(); + return true; } @Override public JdbcType getJdbcType() { - throw new UnsupportedOperationException(); + return jdbcType; } @Override @@ -54,7 +59,7 @@ public J extract( CallableStatement statement, int paramIndex, SharedSessionContractImplementor session) throws SQLException { - throw new UnsupportedOperationException(); + return jdbcType.getExtractor( javaType ).extract( statement, paramIndex, session ); } @Override @@ -62,6 +67,21 @@ public J extract( CallableStatement statement, String paramName, SharedSessionContractImplementor session) throws SQLException { + return jdbcType.getExtractor( javaType ).extract( statement, paramName, session ); + } + + @Override + public JavaType getJavaTypeDescriptor() { throw new UnsupportedOperationException(); } + + @Override + public ValueExtractor getJdbcValueExtractor() { + return jdbcType.getExtractor( javaType ); + } + + @Override + public ValueBinder getJdbcValueBinder() { + return jdbcType.getBinder( javaType ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/DiscriminatorSqmPathSource.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/DiscriminatorSqmPathSource.java index 2aaafbae38..4f3795ad1f 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/DiscriminatorSqmPathSource.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/DiscriminatorSqmPathSource.java @@ -34,6 +34,14 @@ public DiscriminatorSqmPathSource( this.entityMapping = entityMapping; } + public EntityDomainType getEntityDomainType() { + return entityDomainType; + } + + public EntityMappingType getEntityMapping() { + return entityMapping; + } + @Override public SqmPath createSqmPath(SqmPath lhs, SqmPathSource intermediatePathSource) { final NavigablePath navigablePath; diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/PrimitiveBasicTypeImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/PrimitiveBasicTypeImpl.java index 4b4c8e64af..72c7bfdcde 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/PrimitiveBasicTypeImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/model/domain/internal/PrimitiveBasicTypeImpl.java @@ -7,18 +7,19 @@ package org.hibernate.metamodel.model.domain.internal; import org.hibernate.type.descriptor.java.JavaType; +import org.hibernate.type.descriptor.jdbc.JdbcType; public class PrimitiveBasicTypeImpl extends BasicTypeImpl { - private final Class javaTypeClass; + private final Class primitiveClass; - public PrimitiveBasicTypeImpl(JavaType javaType, Class javaTypeClass) { - super( javaType ); - assert javaTypeClass.isPrimitive(); - this.javaTypeClass = javaTypeClass; + public PrimitiveBasicTypeImpl(JavaType javaType, JdbcType jdbcType, Class primitiveClass) { + super( javaType, jdbcType ); + assert primitiveClass.isPrimitive(); + this.primitiveClass = primitiveClass; } @Override public Class getJavaType() { - return javaTypeClass; + return primitiveClass; } } 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 4c52d6a4fd..141353fc58 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 @@ -248,6 +248,7 @@ import static org.hibernate.query.sqm.TemporalUnit.TIMEZONE_MINUTE; import static org.hibernate.query.sqm.TemporalUnit.WEEK_OF_MONTH; import static org.hibernate.query.sqm.TemporalUnit.WEEK_OF_YEAR; +import static org.hibernate.query.sqm.internal.TypecheckUtil.assertComparable; import static org.hibernate.type.descriptor.DateTimeUtils.DATE_TIME; import static org.hibernate.type.spi.TypeConfiguration.isJdbcTemporalType; @@ -2478,7 +2479,7 @@ else if ( r instanceof AnyDiscriminatorSqmPath && l instanceof SqmLiteralEntityT right = r; } } - SqmCriteriaNodeBuilder.assertComparable( left, right ); + assertComparable( left, right, creationContext.getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( left, comparisonOperator, @@ -2676,8 +2677,8 @@ else if ( inListContext instanceof HqlParser.ParamInListContext ) { } else if ( inListContext instanceof HqlParser.SubqueryInListContext ) { final HqlParser.SubqueryInListContext subQueryOrParamInListContext = (HqlParser.SubqueryInListContext) inListContext; - final SqmSubQuery subquery = visitSubquery(subQueryOrParamInListContext.subquery()); - SqmCriteriaNodeBuilder.assertComparable( testExpression, subquery ); + final SqmSubQuery subquery = visitSubquery( subQueryOrParamInListContext.subquery() ); + assertComparable( testExpression, subquery, creationContext.getNodeBuilder().getSessionFactory() ); return new SqmInSubQueryPredicate( testExpression, subquery, diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/NodeBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/NodeBuilder.java index f93480a47f..34e943fb88 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/NodeBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/NodeBuilder.java @@ -22,7 +22,6 @@ import org.hibernate.query.criteria.HibernateCriteriaBuilder; import org.hibernate.query.criteria.JpaCoalesce; import org.hibernate.query.criteria.JpaCompoundSelection; -import org.hibernate.query.criteria.JpaCriteriaQuery; import org.hibernate.query.criteria.JpaExpression; import org.hibernate.query.criteria.JpaParameterExpression; import org.hibernate.query.criteria.JpaSearchedCase; 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 909565cf2b..853ba355c0 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,9 +43,7 @@ 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; @@ -81,7 +79,6 @@ 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; @@ -116,8 +113,6 @@ 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; @@ -135,7 +130,7 @@ 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; +import static org.hibernate.query.sqm.internal.TypecheckUtil.assertAssignable; /** * {@link Query} implementation based on an SQM @@ -357,48 +352,10 @@ private void verifyUpdateTypesMatch(String hqlString, SqmUpdateStatement sqmS final SqmAssignment assignment = assignments.get( i ); final SqmPath targetPath = assignment.getTargetPath(); final SqmExpression expression = assignment.getValue(); - if ( !isAssignable( targetPath, expression ) ) { - throw new SemanticException( - String.format( - "Cannot assign expression of type '%s' to target path '%s' of type '%s'", - expression.getNodeJavaType().getJavaType().getTypeName(), - targetPath.toHqlString(), - targetPath.getNodeJavaType().getJavaType().getTypeName() - ), - hqlString, - null - ); - } + assertAssignable( hqlString, targetPath, expression, getSessionFactory() ); } } - /** - * @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(); @@ -439,21 +396,23 @@ private void verifyInsertTypesMatch( } for ( int i = 0; i < expressionsSize; i++ ) { final SqmTypedNode expression = expressions.get( i ); - if ( expression.getNodeJavaType() == null ) { - continue; - } - if ( insertionTargetPaths.get( i ).getJavaTypeDescriptor() != expression.getNodeJavaType() ) { - throw new SemanticException( - String.format( - "Expected insert attribute type [%s] did not match Query selection type [%s] at selection index [%d]", - insertionTargetPaths.get( i ).getJavaTypeDescriptor().getJavaType().getTypeName(), - expression.getNodeJavaType().getJavaType().getTypeName(), - i - ), - hqlString, - null - ); - } + final SqmPath targetPath = insertionTargetPaths.get(i); + assertAssignable( hqlString, targetPath, expression, getSessionFactory() ); +// if ( expression.getNodeJavaType() == null ) { +// continue; +// } +// if ( insertionTargetPaths.get( i ).getJavaTypeDescriptor() != expression.getNodeJavaType() ) { +// throw new SemanticException( +// String.format( +// "Expected insert attribute type [%s] did not match Query selection type [%s] at selection index [%d]", +// insertionTargetPaths.get( i ).getJavaTypeDescriptor().getJavaType().getTypeName(), +// expression.getNodeJavaType().getJavaType().getTypeName(), +// i +// ), +// hqlString, +// null +// ); +// } } } 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 3f4c917498..192786f941 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,14 +40,10 @@ 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; -import org.hibernate.metamodel.model.domain.TupleType; import org.hibernate.metamodel.model.domain.internal.BasicTypeImpl; -import org.hibernate.metamodel.model.domain.internal.DiscriminatorSqmPathSource; -import org.hibernate.metamodel.model.domain.internal.EmbeddedSqmPathSource; import org.hibernate.metamodel.model.domain.spi.JpaMetamodelImplementor; import org.hibernate.metamodel.spi.MappingMetamodelImplementor; import org.hibernate.query.BindableType; @@ -174,12 +170,11 @@ import jakarta.persistence.criteria.SetJoin; import jakarta.persistence.criteria.Subquery; import jakarta.persistence.metamodel.Bindable; -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; +import static org.hibernate.query.sqm.internal.TypecheckUtil.assertComparable; /** * Acts as a JPA {@link jakarta.persistence.criteria.CriteriaBuilder} by @@ -242,81 +237,7 @@ public SessionFactoryImplementor getSessionFactory() { return sessionFactory.get(); } - /** - * @see QuerySqmImpl#isAssignable(SqmPath, SqmExpression) - */ - public static boolean areTypesComparable(SqmExpressible lhsType, SqmExpressible 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 - || rhsType instanceof EmbeddedSqmPathSource && lhsType instanceof TupleType - // Since we don't know any better, we just allow any comparison with multivalued parameters - || lhsType instanceof MultiValueParameterType - || rhsType instanceof MultiValueParameterType) { - 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 - || 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) { - final SqmExpressible nonDiscriminator; - if ( lhsType instanceof DiscriminatorSqmPathSource ) { - nonDiscriminator = rhsType; - } - else if ( rhsType instanceof DiscriminatorSqmPathSource ) { - nonDiscriminator = lhsType; - } - else { - return false; - } - - // Comparing the discriminator against an entity type is fine - if ( nonDiscriminator instanceof EntityType ) { - return true; - } - final JavaType nonDiscriminatorJavaType = nonDiscriminator.getExpressibleJavaType(); - // Comparing the discriminator against the discriminator value is fine - switch ( nonDiscriminatorJavaType.getJavaTypeClass().getTypeName() ) { - case "java.lang.String": - case "char": - case "java.lang.Character": - case "int": - case "java.lang.Integer": - return true; - default: - return false; - } - } @Override public BasicType getBooleanType() { @@ -1246,10 +1167,13 @@ else if ( value == null ) { return null; } else { - Class type = value.getClass(); - BasicType result = typeConfiguration.getBasicTypeForJavaType( type ); + final Class type = value.getClass(); + final BasicType result = typeConfiguration.getBasicTypeForJavaType( type ); if ( result == null && value instanceof Enum ) { - return new BasicTypeImpl<>( new EnumJavaType<>( type ) ); + final EnumJavaType javaType = new EnumJavaType<>( type ); + final JdbcType jdbcType = + javaType.getRecommendedJdbcType( typeConfiguration.getCurrentBaseSqlTypeIndicators() ); + return new BasicTypeImpl<>( javaType, jdbcType ); } else { return result; @@ -1306,12 +1230,12 @@ public List> literals(List values) { @Override public SqmExpression nullLiteral(Class resultClass) { - final TypeConfiguration typeConfiguration = getTypeConfiguration(); - final BasicType basicTypeForJavaType = typeConfiguration.getBasicTypeForJavaType( resultClass ); + final BasicType basicTypeForJavaType = getTypeConfiguration().getBasicTypeForJavaType( resultClass ); + // if there's no basic type, it might be an entity type final SqmExpressible sqmExpressible = basicTypeForJavaType == null ? getDomainModel().managedType( resultClass ) : basicTypeForJavaType; - return new SqmLiteralNull<>(sqmExpressible, this ); + return new SqmLiteralNull<>( sqmExpressible, this ); } class MultiValueParameterType implements SqmExpressible { @@ -1674,7 +1598,7 @@ public JpaExpression localTime() { @Override public SqmFunction function(String name, Class type, Expression[] args) { SqmFunctionDescriptor functionTemplate = getFunctionDescriptor( name ); - final BasicType resultType = getTypeConfiguration().getBasicTypeForJavaType( type ); + final BasicType resultType = getTypeConfiguration().standardBasicTypeForJavaType( type ); if ( functionTemplate == null ) { functionTemplate = new NamedSqmFunctionDescriptor( name, @@ -2043,8 +1967,8 @@ public SqmPredicate isNotNull(Expression x) { @Override public > SqmPredicate between(Expression value, Expression lower, Expression upper) { - assertComparable( value, lower ); - assertComparable( value, upper ); + assertComparable( value, lower, getNodeBuilder().getSessionFactory() ); + assertComparable( value, upper, getNodeBuilder().getSessionFactory() ); return new SqmBetweenPredicate( (SqmExpression) value, (SqmExpression) lower, @@ -2059,8 +1983,8 @@ public > SqmPredicate between(Expression valueExpression = (SqmExpression) value; final SqmExpression lowerExpr = value( lower, valueExpression ); final SqmExpression upperExpr = value( upper, valueExpression ); - assertComparable( valueExpression, lowerExpr ); - assertComparable( valueExpression, upperExpr ); + assertComparable( valueExpression, lowerExpr, getNodeBuilder().getSessionFactory() ); + assertComparable( valueExpression, upperExpr, getNodeBuilder().getSessionFactory() ); return new SqmBetweenPredicate( valueExpression, lowerExpr, @@ -2070,26 +1994,6 @@ public > SqmPredicate between(Expression x, Expression y) { - SqmExpression left = (SqmExpression) x; - SqmExpression right = (SqmExpression) y; - if ( left.getTupleLength() != null && right.getTupleLength() != null - && left.getTupleLength().intValue() != right.getTupleLength().intValue() ) { - throw new SemanticException( "Cannot compare tuples of different lengths" ); - } - final SqmExpressible leftType = left.getNodeType(); - final SqmExpressible rightType = right.getNodeType(); - if ( !areTypesComparable( leftType, rightType ) ) { - throw new SemanticException( - String.format( - "Cannot compare left expression of type '%s' with right expression of type '%s'", - leftType.getTypeName(), - rightType.getTypeName() - ) - ); - } - } - public static void assertNumeric(SqmExpression expression, BinaryArithmeticOperator op) { final SqmExpressible nodeType = expression.getNodeType(); if ( nodeType != null ) { @@ -2130,7 +2034,7 @@ public static void assertNumeric(SqmExpression expression, UnaryArithmeticOpe @Override public SqmPredicate equal(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.EQUAL, @@ -2142,7 +2046,7 @@ public SqmPredicate equal(Expression x, Expression y) { @Override public SqmPredicate equal(Expression x, Object y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.EQUAL, @@ -2153,7 +2057,7 @@ public SqmPredicate equal(Expression x, Object y) { @Override public SqmPredicate notEqual(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.NOT_EQUAL, @@ -2165,7 +2069,7 @@ public SqmPredicate notEqual(Expression x, Expression y) { @Override public SqmPredicate notEqual(Expression x, Object y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.NOT_EQUAL, @@ -2176,7 +2080,7 @@ public SqmPredicate notEqual(Expression x, Object y) { @Override public SqmPredicate distinctFrom(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.DISTINCT_FROM, @@ -2188,7 +2092,7 @@ public SqmPredicate distinctFrom(Expression x, Expression y) { @Override public SqmPredicate distinctFrom(Expression x, Object y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.DISTINCT_FROM, @@ -2199,7 +2103,7 @@ public SqmPredicate distinctFrom(Expression x, Object y) { @Override public SqmPredicate notDistinctFrom(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.NOT_DISTINCT_FROM, @@ -2211,7 +2115,7 @@ public SqmPredicate notDistinctFrom(Expression x, Expression y) { @Override public SqmPredicate notDistinctFrom(Expression x, Object y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.NOT_DISTINCT_FROM, @@ -2222,7 +2126,7 @@ public SqmPredicate notDistinctFrom(Expression x, Object y) { @Override public > SqmPredicate greaterThan(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN, @@ -2234,7 +2138,7 @@ public > SqmPredicate greaterThan(Expression> SqmPredicate greaterThan(Expression x, Y y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN, @@ -2245,7 +2149,7 @@ public > SqmPredicate greaterThan(Expression> SqmPredicate greaterThanOrEqualTo(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN_OR_EQUAL, @@ -2257,7 +2161,7 @@ public > SqmPredicate greaterThanOrEqualTo(Expre @Override public > SqmPredicate greaterThanOrEqualTo(Expression x, Y y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN_OR_EQUAL, @@ -2268,7 +2172,7 @@ public > SqmPredicate greaterThanOrEqualTo(Expre @Override public > SqmPredicate lessThan(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN, @@ -2280,7 +2184,7 @@ public > SqmPredicate lessThan(Expression> SqmPredicate lessThan(Expression x, Y y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN, @@ -2302,7 +2206,7 @@ public > SqmPredicate lessThanOrEqualTo(Expressi @Override public > SqmPredicate lessThanOrEqualTo(Expression x, Y y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN_OR_EQUAL, @@ -2313,7 +2217,7 @@ public > SqmPredicate lessThanOrEqualTo(Expressi @Override public SqmPredicate gt(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN, @@ -2325,7 +2229,7 @@ public SqmPredicate gt(Expression x, Expression x, Number y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN, @@ -2336,7 +2240,7 @@ public SqmPredicate gt(Expression x, Number y) { @Override public SqmPredicate ge(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN_OR_EQUAL, @@ -2348,7 +2252,7 @@ public SqmPredicate ge(Expression x, Expression x, Number y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.GREATER_THAN_OR_EQUAL, @@ -2359,7 +2263,7 @@ public SqmPredicate ge(Expression x, Number y) { @Override public SqmPredicate lt(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN, @@ -2371,7 +2275,7 @@ public SqmPredicate lt(Expression x, Expression x, Number y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN, @@ -2382,7 +2286,7 @@ public SqmPredicate lt(Expression x, Number y) { @Override public SqmPredicate le(Expression x, Expression y) { - assertComparable( x, y ); + assertComparable( x, y, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN_OR_EQUAL, @@ -2394,7 +2298,7 @@ public SqmPredicate le(Expression x, Expression x, Number y) { final SqmExpression yExpr = value( y, (SqmExpression) x ); - assertComparable( x, yExpr ); + assertComparable( x, yExpr, getNodeBuilder().getSessionFactory() ); return new SqmComparisonPredicate( (SqmExpression) x, ComparisonOperator.LESS_THAN_OR_EQUAL, @@ -2726,7 +2630,7 @@ public SqmFunction sql(String pattern, Class type, Expression... ar sqmArguments.add( 0, literal( pattern ) ); return getFunctionDescriptor( "sql" ).generateSqmExpression( sqmArguments, - getTypeConfiguration().getBasicTypeForJavaType( type ), + getTypeConfiguration().standardBasicTypeForJavaType( type ), queryEngine ); } 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 new file mode 100644 index 0000000000..8acdfd484f --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/TypecheckUtil.java @@ -0,0 +1,360 @@ +/* + * 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.internal; + +import jakarta.persistence.criteria.Expression; +import jakarta.persistence.metamodel.EntityType; +import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.metamodel.mapping.JdbcMapping; +import org.hibernate.metamodel.model.domain.DomainType; +import org.hibernate.metamodel.model.domain.TupleType; +import org.hibernate.metamodel.model.domain.internal.DiscriminatorSqmPathSource; +import org.hibernate.metamodel.model.domain.internal.EmbeddedSqmPathSource; +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.query.SemanticException; +import org.hibernate.query.sqm.SqmExpressible; +import org.hibernate.query.sqm.SqmPathSource; +import org.hibernate.query.sqm.tree.SqmTypedNode; +import org.hibernate.query.sqm.tree.domain.SqmPath; +import org.hibernate.query.sqm.tree.expression.SqmExpression; +import org.hibernate.query.sqm.tree.expression.SqmLiteralNull; +import org.hibernate.type.BasicType; +import org.hibernate.type.descriptor.jdbc.JdbcType; + +/** + * Functions for typechecking comparison expressions and assignments in the SQM tree. + * A comparison expression is any predicate like {@code x = y} or {@code x > y}. An + * assignment is an element of the {@code set} clause in an {@code update} query, or + * an element of the {@code values} list in an {@code insert query}. + *

+ * The rules here are not the same as the rules for the Java language, nor are they + * identical to the rules for SQL. For example: + *

    + *
  • In Java, a comparison expression like {@code '1'.equals(1)} is well-typed, and + * evaluates to {@code false}. In most SQL dialects, this expression evaluates to + * {@code true}, via implicit type conversions. Here we reject such + * comparison expressions. + *
  • In Java, if two classes are related by inheritance, then one is assignable to + * the other. But here, this assignment is only legal if the two classes are entity + * types belonging to the same mapped entity inheritance hierarchy. + *
  • On the other hand, in Java, {@code java.sql.Date} and {@code java.time.LocalDate} + * may not be compared nor assigned. But here they're considered inter-comparable + * and inter-assignable. + *
+ *

+ * Two basic types are considered comparable if they map to the same "family" of + * {@linkplain org.hibernate.type.SqlTypes JDBC type}s. Here we allow some latitude + * so that different numeric types are comparable, and different string types are + * comparable. However, we do not allow comparisons between types which involve more + * questionable/unportable implicit type conversions (between integers and strings, + * for example). This means that we accept comparisons between basic types which map + * completely unrelated types in Java. + *

+ * Entity types have identity equality. That is, two entities are considered equal if + * their primary keys are equal. + *

+ * Embeddable and tuple types have value equality. That is, they're considered equal + * if their members are equal. For convenience, an embeddable object may be compared + * directly to a tuple constructor. + *

+ * Comparison of discriminators (that is, of literal entity types and {@code type()} + * function application) is legal only when the entity types belong to the same mapped + * entity hierarchy. + * + * @see #assertComparable(Expression, Expression, SessionFactoryImplementor) + * @see #assertAssignable(String, SqmPath, SqmTypedNode, SessionFactoryImplementor) + * + * @author Gavin King + */ +public class TypecheckUtil { + + /** + * @implNote The code below will reject some things that are accepted in H5, + * and perhaps even a few things which were accepted in H6.1/6.2. Therefore, + * users will report "bugs". The correct resolution of such bugs is not to + * go naively inserting special cases and secret escape hatches in this code! + * Instead, it's important to practice saying "no" and rejecting the majority + * of such "bug" reports. In cases where a fix really is required, it should + * be done within the framework laid out below, not by adding ad-hoc special + * rules and holes which undermine the type system. It's much more important + * that HQL has simple, predictable, and understandable rules than it is that + * every single user be able to do every single little wierd thing that used + * to work in Hibernate 5. + * + * @param lhsType the type of the expression on the LHS of the comparison operator + * @param rhsType the type of the expression on the RHS of the comparison operator + * + * @see #isTypeAssignable(SqmPathSource, SqmExpressible, SessionFactoryImplementor) + */ + private static boolean areTypesComparable( + SqmExpressible lhsType, SqmExpressible rhsType, + SessionFactoryImplementor factory) { + + if ( lhsType == null || rhsType == null || lhsType == rhsType ) { + return true; + } + + // since we can't so anything meaningful here, just allow + // any comparison with multivalued parameters + + if ( lhsType instanceof SqmCriteriaNodeBuilder.MultiValueParameterType + || rhsType instanceof SqmCriteriaNodeBuilder.MultiValueParameterType) { + // TODO: do something meaningful here + return true; + } + + // for embeddables, the embeddable class must match exactly + + if ( lhsType instanceof EmbeddedSqmPathSource && rhsType instanceof EmbeddedSqmPathSource ) { + return areEmbeddableTypesComparable( (EmbeddedSqmPathSource) lhsType, (EmbeddedSqmPathSource) rhsType ); + } + + // for tuple constructors, we must check each element + + if ( lhsType instanceof TupleType && rhsType instanceof TupleType ) { + return areTupleTypesComparable( factory, (TupleType) lhsType, (TupleType) rhsType ); + } + + // allow comparing an embeddable against a tuple literal + + if ( lhsType instanceof EmbeddedSqmPathSource && rhsType instanceof TupleType + || rhsType instanceof EmbeddedSqmPathSource && lhsType instanceof TupleType ) { + // TODO: do something meaningful here + return true; + } + + // entities can be compared if they belong to the same inheritance hierarchy + + if ( lhsType instanceof EntityType && rhsType instanceof EntityType ) { + return areEntityTypesComparable( (EntityType) lhsType, (EntityType) rhsType, factory ); + } + + // entities can be compared to discriminators if they belong to + // the same inheritance hierarchy + + if ( lhsType instanceof DiscriminatorSqmPathSource) { + return isDiscriminatorTypeComparable( (DiscriminatorSqmPathSource) lhsType, rhsType, factory ); + } + if ( rhsType instanceof DiscriminatorSqmPathSource ) { + return isDiscriminatorTypeComparable( (DiscriminatorSqmPathSource) rhsType, lhsType, factory ); + } + + // Treat the expressions as comparable if they belong to the same + // "family" of JDBC type. One could object to this approach since + // JDBC types can vary between databases, however it's the only + // decent approach which allows comparison between literals and + // enums, user-defined types, etc. + + 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() + // "families" of implicitly-convertible JDBC types + // (this list might need to be extended in future) + || lhsJdbcType.isStringLike() && rhsJdbcType.isStringLike() + || lhsJdbcType.isTemporal() && rhsJdbcType.isTemporal() + || lhsJdbcType.isNumber() && rhsJdbcType.isNumber() ) { + return true; + } + } + + // Workaround: these are needed for a handful of slightly "weird" cases + // involving Java field literals and converters, where we don't have + // access to the correct JDBC type above. However, this is exactly the + // sort of hole warned about above, and accepts many things which are + // not well-typed. + + // TODO: sort all this out, and remove this branch + if ( isSameJavaType( lhsType, rhsType ) ) { + return true; + } + + return false; + } + + private static boolean areEmbeddableTypesComparable(EmbeddedSqmPathSource lhsType, EmbeddedSqmPathSource rhsType) { + // no polymorphism for embeddable types + return rhsType.getNodeJavaType() == lhsType.getNodeJavaType(); + } + + private static boolean areTupleTypesComparable(SessionFactoryImplementor factory, TupleType lhsTuple, TupleType rhsTuple) { + if ( rhsTuple.componentCount() != lhsTuple.componentCount() ) { + return false; + } + else { + for ( int i = 0; i < lhsTuple.componentCount(); i++ ) { + if ( !areTypesComparable( lhsTuple.get(i), rhsTuple.get(i), factory ) ) { + return false; + } + } + } + return true; + } + + private static boolean areEntityTypesComparable( + EntityType lhsType, EntityType rhsType, + SessionFactoryImplementor factory) { + EntityPersister lhsEntity = getEntityDescriptor( factory, lhsType.getName() ); + EntityPersister rhsEntity = getEntityDescriptor( factory, rhsType.getName() ); + return lhsEntity.getRootEntityName().equals( rhsEntity.getRootEntityName() ); + } + + private static boolean isDiscriminatorTypeComparable( + DiscriminatorSqmPathSource lhsDiscriminator, SqmExpressible rhsType, + SessionFactoryImplementor factory) { + String entityName = lhsDiscriminator.getEntityDomainType().getHibernateEntityName(); + EntityPersister lhsEntity = factory.getMappingMetamodel().getEntityDescriptor( entityName ); + if ( rhsType instanceof EntityType ) { + String rhsEntityName = ((EntityType) rhsType).getName(); + EntityPersister rhsEntity = getEntityDescriptor( factory, rhsEntityName ); + return lhsEntity.getRootEntityName().equals( rhsEntity.getRootEntityName() ); + } + else if ( rhsType instanceof DiscriminatorSqmPathSource ) { + DiscriminatorSqmPathSource discriminator = (DiscriminatorSqmPathSource) rhsType; + String rhsEntityName = discriminator.getEntityDomainType().getHibernateEntityName(); + EntityPersister rhsEntity = factory.getMappingMetamodel().getEntityDescriptor( rhsEntityName ); + return rhsEntity.getRootEntityName().equals( lhsEntity.getRootEntityName() ); + } + else { + BasicType discriminatorType = (BasicType) + lhsDiscriminator.getEntityMapping().getDiscriminatorMapping().getMappedType(); + return areTypesComparable( discriminatorType, rhsType, factory ); + } + } + + /** + * @param targetType the type of the path expression to which a value is assigned + * @param expressionType the type of the value expression being assigned to the path + * + * @see #areTypesComparable(SqmExpressible, SqmExpressible, SessionFactoryImplementor) + */ + private static boolean isTypeAssignable( + SqmPathSource targetType, SqmExpressible expressionType, + SessionFactoryImplementor factory) { + + if ( targetType == null || expressionType == null || targetType == expressionType ) { + return true; + } + + // entities can be assigned if they belong to the same inheritance hierarchy + + if ( targetType instanceof EntityType && expressionType instanceof EntityType ) { + return isEntityTypeAssignable( (EntityType) targetType, (EntityType) expressionType, factory ); + } + + // Treat the expression as assignable to the target path if they belong + // to the same "family" of JDBC type. One could object to this approach + // since JDBC types can vary between databases, however it's the only + // decent approach which allows comparison between literals and enums, + // user-defined types, etc. + + DomainType lhsDomainType = targetType.getSqmType(); + DomainType rhsDomainType = expressionType.getSqmType(); + if ( lhsDomainType instanceof JdbcMapping && rhsDomainType instanceof JdbcMapping ) { + JdbcType lhsJdbcType = ((JdbcMapping) lhsDomainType).getJdbcType(); + JdbcType rhsJdbcType = ((JdbcMapping) rhsDomainType).getJdbcType(); + if ( lhsJdbcType.getJdbcTypeCode() == rhsJdbcType.getJdbcTypeCode() + // "families" of implicitly-convertible JDBC types + // (this list might need to be extended in future) + || lhsJdbcType.isStringLike() && rhsJdbcType.isStringLike() + || lhsJdbcType.isInteger() && rhsJdbcType.isInteger() + || lhsJdbcType.isFloat() && rhsJdbcType.isFloat() + || lhsJdbcType.isFloat() && rhsJdbcType.isInteger() ) { + return true; + } + } + + // Workaround: these are needed for a handful of slightly "weird" cases + // involving Java field literals and converters, where we don't have + // access to the correct JDBC type above. However, this is exactly the + // sort of hole warned about above, and accepts many things which are + // not well-typed. + + // TODO: sort all this out, and remove this branch + if ( isSameJavaType( targetType, expressionType ) ) { + return true; + } + + return false; + } + + private static boolean isSameJavaType(SqmExpressible leftType, SqmExpressible rightType) { + return leftType.getRelationalJavaType() == rightType.getRelationalJavaType() + || leftType.getExpressibleJavaType() == rightType.getExpressibleJavaType() + || leftType.getBindableJavaType() == rightType.getBindableJavaType(); + } + + private static boolean isEntityTypeAssignable( + EntityType lhsType, EntityType rhsType, + SessionFactoryImplementor factory) { + EntityPersister lhsEntity = getEntityDescriptor( factory, lhsType.getName() ); + EntityPersister rhsEntity = getEntityDescriptor( factory, rhsType.getName() ); + return lhsEntity.isSubclassEntityName( rhsEntity.getEntityName() ); + } + + private static EntityPersister getEntityDescriptor(SessionFactoryImplementor factory, String name) { + return factory.getMappingMetamodel() + .getEntityDescriptor( factory.getJpaMetamodel().qualifyImportableName( name ) ); + } + + /** + * @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() ) { + throw new SemanticException( "Cannot compare tuples of different lengths" ); + } + // allow comparing literal null to things + if ( !(left instanceof SqmLiteralNull) && !(right instanceof SqmLiteralNull) ) { + final SqmExpressible leftType = left.getNodeType(); + final SqmExpressible rightType = right.getNodeType(); + if ( !areTypesComparable( leftType, rightType, factory ) ) { + throw new SemanticException( + String.format( + "Cannot compare left expression of type '%s' with right expression of type '%s'", + leftType.getTypeName(), + rightType.getTypeName() + ) + ); + } + } + } + + /** + * @see TypecheckUtil#assertComparable(Expression, Expression, SessionFactoryImplementor) + */ + public static void assertAssignable( + String hqlString, + SqmPath targetPath, SqmTypedNode expression, + SessionFactoryImplementor factory) { + // allow assigning literal null to things + if ( expression instanceof SqmLiteralNull ) { + // TODO: check that the target path is nullable + } + else { + SqmPathSource targetType = targetPath.getNodeType(); + SqmExpressible expressionType = expression.getNodeType(); + if ( !isTypeAssignable( targetType, expressionType, factory ) ) { + throw new SemanticException( + String.format( + "Cannot assign expression of type '%s' to target path '%s' of type '%s'", + expressionType.getTypeName(), + targetPath.toHqlString(), + targetType.getTypeName() + ), + hqlString, + null + ); + } + } + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmInListPredicate.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmInListPredicate.java index 7db208d14b..cf57530a42 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmInListPredicate.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/predicate/SqmInListPredicate.java @@ -15,12 +15,13 @@ import org.hibernate.query.internal.QueryHelper; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.query.sqm.SemanticQueryWalker; -import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.tree.SqmCopyContext; import org.hibernate.query.sqm.tree.expression.SqmExpression; import jakarta.persistence.criteria.Expression; +import static org.hibernate.query.sqm.internal.TypecheckUtil.assertComparable; + /** * @author Steve Ebersole */ @@ -135,7 +136,7 @@ public void addExpression(SqmExpression expression) { } private void implyListElementType(SqmExpression expression) { - nodeBuilder().assertComparable( getTestExpression(), expression ); + assertComparable( getTestExpression(), expression, nodeBuilder().getSessionFactory() ); expression.applyInferableType( QueryHelper.highestPrecedenceType2( getTestExpression().getExpressible(), expression.getExpressible() ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeBaseline.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeBaseline.java index 9ed11138fc..d247013953 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeBaseline.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/spi/JavaTypeBaseline.java @@ -35,10 +35,8 @@ import org.hibernate.type.descriptor.java.BlobJavaType; import org.hibernate.type.descriptor.java.BooleanJavaType; import org.hibernate.type.descriptor.java.BooleanPrimitiveArrayJavaType; -import org.hibernate.type.descriptor.java.ByteArrayJavaType; import org.hibernate.type.descriptor.java.ByteJavaType; import org.hibernate.type.descriptor.java.CalendarJavaType; -import org.hibernate.type.descriptor.java.CharacterArrayJavaType; import org.hibernate.type.descriptor.java.CharacterJavaType; import org.hibernate.type.descriptor.java.ClassJavaType; import org.hibernate.type.descriptor.java.ClobJavaType; diff --git a/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java b/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java index 6dfe4e04c3..970a070c24 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java +++ b/hibernate-core/src/main/java/org/hibernate/type/spi/TypeConfiguration.java @@ -693,7 +693,7 @@ public BasicType getBasicTypeForJavaType(Type javaType) { return (BasicType) existing; } - final BasicType registeredType = getBasicTypeRegistry().getRegisteredType( javaType ); + final BasicType registeredType = basicTypeRegistry.getRegisteredType( javaType ); if ( registeredType != null ) { basicTypeByJavaType.put( javaType, registeredType ); return registeredType; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/BulkManipulationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/BulkManipulationTest.java index 316a7db7bf..66d67a2e92 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/BulkManipulationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/BulkManipulationTest.java @@ -404,8 +404,8 @@ public void testSimpleInsertTypeMismatchException() { int lt = m.indexOf( "java.lang.Long" ); assertTrue( "type causing error not reported", st > -1 ); assertTrue( "type causing error not reported", lt > -1 ); - assertTrue( lt > st ); - assertTrue( "wrong position of type error reported", m.indexOf( "index [2]" ) > -1 ); + assertTrue( lt < st ); +// assertTrue( "wrong position of type error reported", m.indexOf( "index [2]" ) > -1 ); } finally { s.close(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/ObjectParameterTypeForEmbeddableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/ObjectParameterTypeForEmbeddableTest.java index 1b55e288e8..51dcb121ae 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/ObjectParameterTypeForEmbeddableTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/criteria/ObjectParameterTypeForEmbeddableTest.java @@ -1,7 +1,9 @@ package org.hibernate.orm.test.jpa.criteria; +import org.hibernate.query.SemanticException; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.FailureExpected; import org.hibernate.testing.orm.junit.Jpa; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -37,7 +39,7 @@ public void setUp(EntityManagerFactoryScope scope) { ); } - @Test + @Test @FailureExpected(reason = "This query is in my opinion not well-typed, and should be rejected") public void testSettingParameterOfTypeObject(EntityManagerFactoryScope scope) { scope.inTransaction( entityManager -> { @@ -75,8 +77,8 @@ public void testSettingParameterOfTypeAnEmbeddable(EntityManagerFactoryScope sco @Test public void testSettingParameterOfTypeWrongType(EntityManagerFactoryScope scope) { - IllegalArgumentException thrown = assertThrows( - IllegalArgumentException.class, () -> + SemanticException thrown = assertThrows( + SemanticException.class, () -> scope.inTransaction( entityManager -> { final CriteriaBuilder cb = entityManager.getCriteriaBuilder(); @@ -92,8 +94,7 @@ public void testSettingParameterOfTypeWrongType(EntityManagerFactoryScope scope) ) ); - assertThat( thrown.getMessage() ).startsWith( - "Argument [org.hibernate.orm.test.jpa.criteria.ObjectParameterTypeForEmbeddableTest" ); + assertThat( thrown.getMessage() ).startsWith( "Cannot compare left expression" ); } @Entity(name = "TestEntity") diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/converted/converter/literal/QueryLiteralTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/converted/converter/literal/QueryLiteralTest.java index 3211f2561e..e2d81272d8 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/converted/converter/literal/QueryLiteralTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/converted/converter/literal/QueryLiteralTest.java @@ -20,6 +20,7 @@ import org.hibernate.engine.spi.SessionImplementor; import org.hibernate.internal.util.ExceptionHelper; import org.hibernate.query.Query; +import org.hibernate.query.SemanticException; import org.hibernate.sql.ast.SqlTreeCreationException; import org.hibernate.testing.orm.junit.DomainModel; @@ -90,11 +91,7 @@ public void testIntegerWrapperThrowsException(SessionFactoryScope scope) { fail( "Should throw Exception!" ); } catch (Exception e) { - final Throwable rootCause = ExceptionHelper.getRootCause( e ); - assertThat( rootCause, instanceOf( SqlTreeCreationException.class ) ); - assertThat( rootCause.getMessage(), startsWith( "QueryLiteral type [" ) ); - assertThat( rootCause.getMessage(), containsString( "] did not match domain Java-type [" ) ); - assertThat( rootCause.getMessage(), containsString( "] nor JDBC Java-type [" ) ); + assertThat( ExceptionHelper.getRootCause( e ), instanceOf( SemanticException.class ) ); } }