From 7f8b6859738b4c24afe10910410bd349a7949327 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 12 Dec 2024 20:32:05 +0100 Subject: [PATCH] HHH-18929 generate check constraints for discriminator columns --- .../annotations/DiscriminatorOptions.java | 4 + .../DiscriminatorColumnSecondPass.java | 97 +++++++++++++++++++ .../boot/model/internal/EntityBinder.java | 4 +- ...NullableDiscriminatorColumnSecondPass.java | 46 --------- .../java/org/hibernate/dialect/Dialect.java | 18 +++- .../dialect/DialectDelegateWrapper.java | 3 +- .../persister/entity/DiscriminatorHelper.java | 51 ++++------ 7 files changed, 137 insertions(+), 86 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/boot/model/internal/DiscriminatorColumnSecondPass.java delete mode 100644 hibernate-core/src/main/java/org/hibernate/boot/model/internal/NullableDiscriminatorColumnSecondPass.java diff --git a/hibernate-core/src/main/java/org/hibernate/annotations/DiscriminatorOptions.java b/hibernate-core/src/main/java/org/hibernate/annotations/DiscriminatorOptions.java index 62b37843a4..fe6ab4ed0c 100644 --- a/hibernate-core/src/main/java/org/hibernate/annotations/DiscriminatorOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/annotations/DiscriminatorOptions.java @@ -28,6 +28,10 @@ public @interface DiscriminatorOptions { * instances of a root entity and its subtypes. This is useful if * there are discriminator column values which do not * map to any subtype of the root entity type. + *

+ * This setting has the side effect of suppressing the generation + * of a {@code check} constraint in the DDL for the discriminator + * column. * * @return {@code true} if allowed discriminator values must always * be explicitly enumerated diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/DiscriminatorColumnSecondPass.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/DiscriminatorColumnSecondPass.java new file mode 100644 index 0000000000..7bfd308d48 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/DiscriminatorColumnSecondPass.java @@ -0,0 +1,97 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.boot.model.internal; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import org.hibernate.MappingException; +import org.hibernate.boot.spi.SecondPass; +import org.hibernate.dialect.Dialect; +import org.hibernate.mapping.CheckConstraint; +import org.hibernate.mapping.Column; +import org.hibernate.mapping.PersistentClass; +import org.hibernate.mapping.Selectable; +import org.hibernate.mapping.Subclass; +import org.hibernate.persister.entity.DiscriminatorHelper; + + +public class DiscriminatorColumnSecondPass implements SecondPass { + private final String rootEntityName; + private final Dialect dialect; + + public DiscriminatorColumnSecondPass(String rootEntityName, Dialect dialect) { + this.rootEntityName = rootEntityName; + this.dialect = dialect; + } + + @Override + public void doSecondPass(Map persistentClasses) throws MappingException { + final PersistentClass rootClass = persistentClasses.get( rootEntityName ); + if ( hasNullDiscriminatorValue( rootClass ) ) { + for ( Selectable selectable: rootClass.getDiscriminator().getSelectables() ) { + if ( selectable instanceof Column column ) { + column.setNullable( true ); + } + } + } + if ( !hasNotNullDiscriminatorValue( rootClass ) // a "not null" discriminator is a catch-all + && !rootClass.getDiscriminator().hasFormula() // can't add check constraints to formulas + && !rootClass.isForceDiscriminator() ) { // the usecase for "forced" discriminators is that there are some rogue values + final Column column = rootClass.getDiscriminator().getColumns().get( 0 ); + column.addCheckConstraint( new CheckConstraint( checkConstraint( rootClass, column ) ) ); + } + } + + private boolean hasNullDiscriminatorValue(PersistentClass rootClass) { + if ( rootClass.isDiscriminatorValueNull() ) { + return true; + } + for ( Subclass subclass : rootClass.getSubclasses() ) { + if ( subclass.isDiscriminatorValueNull() ) { + return true; + } + } + return false; + } + + private boolean hasNotNullDiscriminatorValue(PersistentClass rootClass) { + if ( rootClass.isDiscriminatorValueNotNull() ) { + return true; + } + for ( Subclass subclass : rootClass.getSubclasses() ) { + if ( subclass.isDiscriminatorValueNotNull() ) { + return true; + } + } + return false; + } + + private String checkConstraint(PersistentClass rootClass, Column column) { + return dialect.getCheckCondition( + column.getQuotedName( dialect ), + discriminatorValues( rootClass ), + column.getType().getJdbcType() + ); + } + + private static List discriminatorValues(PersistentClass rootClass) { + final List values = new ArrayList<>(); + if ( !rootClass.isAbstract() + && !rootClass.isDiscriminatorValueNull() + && !rootClass.isDiscriminatorValueNotNull() ) { + values.add( DiscriminatorHelper.getDiscriminatorValue( rootClass ).toString() ); + } + for ( Subclass subclass : rootClass.getSubclasses() ) { + if ( !subclass.isAbstract() + && !subclass.isDiscriminatorValueNull() + && !subclass.isDiscriminatorValueNotNull() ) { + values.add( DiscriminatorHelper.getDiscriminatorValue( subclass ).toString() ); + } + } + return values; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java index 48df4077fe..865e08ece2 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java @@ -958,7 +958,9 @@ public class EntityBinder { rootClass.setPolymorphic( true ); final String rootEntityName = rootClass.getEntityName(); LOG.tracev( "Setting discriminator for entity {0}", rootEntityName); - getMetadataCollector().addSecondPass( new NullableDiscriminatorColumnSecondPass( rootEntityName ) ); + getMetadataCollector() + .addSecondPass( new DiscriminatorColumnSecondPass( rootEntityName, + context.getMetadataCollector().getDatabase().getDialect() ) ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/NullableDiscriminatorColumnSecondPass.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/NullableDiscriminatorColumnSecondPass.java deleted file mode 100644 index 6b541af608..0000000000 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/NullableDiscriminatorColumnSecondPass.java +++ /dev/null @@ -1,46 +0,0 @@ -/* - * SPDX-License-Identifier: LGPL-2.1-or-later - * Copyright Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.boot.model.internal; - -import java.util.Map; - -import org.hibernate.MappingException; -import org.hibernate.boot.spi.SecondPass; -import org.hibernate.mapping.Column; -import org.hibernate.mapping.PersistentClass; -import org.hibernate.mapping.Selectable; -import org.hibernate.mapping.Subclass; - -public class NullableDiscriminatorColumnSecondPass implements SecondPass { - private final String rootEntityName; - - public NullableDiscriminatorColumnSecondPass(String rootEntityName) { - this.rootEntityName = rootEntityName; - } - - @Override - public void doSecondPass(Map persistentClasses) throws MappingException { - final PersistentClass rootPersistenceClass = persistentClasses.get( rootEntityName ); - if ( hasNullDiscriminatorValue( rootPersistenceClass ) ) { - for ( Selectable selectable: rootPersistenceClass.getDiscriminator().getSelectables() ) { - if ( selectable instanceof Column ) { - ( (Column) selectable ).setNullable( true ); - } - } - } - } - - private boolean hasNullDiscriminatorValue(PersistentClass rootPersistenceClass) { - if ( rootPersistenceClass.isDiscriminatorValueNull() ) { - return true; - } - for ( Subclass subclass : rootPersistenceClass.getSubclasses() ) { - if ( subclass.isDiscriminatorValueNull() ) { - return true; - } - } - return false; - } -} diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index b2969101ca..e039167494 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -189,6 +189,7 @@ import java.time.temporal.ChronoUnit; import java.time.temporal.TemporalAccessor; import java.time.temporal.TemporalAmount; import java.util.Calendar; +import java.util.Collection; import java.util.Date; import java.util.HashSet; import java.util.List; @@ -839,11 +840,17 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun } /** - * Generate a check condition for column with the given set of values. + * Generate a SQL {@code check} condition for the given column, + * constraining to the given values. * - * @apiNote Only supports TINYINT, SMALLINT and (VAR)CHAR + * @return a SQL expression that will occur in a {@code check} constraint + * + * @apiNote Only supports {@code TINYINT}, {@code SMALLINT}, {@code CHAR}, + * and {@code VARCHAR} + * + * @since 7.0 */ - public String getCheckCondition(String columnName, Set valueSet, JdbcType jdbcType) { + public String getCheckCondition(String columnName, Collection valueSet, JdbcType jdbcType) { final boolean isCharacterJdbcType = isCharacterType( jdbcType.getJdbcTypeCode() ); assert isCharacterJdbcType || isIntegral( jdbcType.getJdbcTypeCode() ); @@ -856,11 +863,12 @@ public abstract class Dialect implements ConversionContext, TypeContributor, Fun nullIsValid = true; continue; } + check.append( separator ); if ( isCharacterJdbcType ) { - check.append( separator ).append('\'').append( value ).append('\''); + check.append('\'').append( value ).append('\''); } else { - check.append( separator ).append( value ); + check.append( value ); } separator = ","; } diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/DialectDelegateWrapper.java b/hibernate-core/src/main/java/org/hibernate/dialect/DialectDelegateWrapper.java index a760f32ee6..c7963dfd6a 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/DialectDelegateWrapper.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/DialectDelegateWrapper.java @@ -12,6 +12,7 @@ import java.time.Duration; import java.time.temporal.TemporalAccessor; import java.time.temporal.TemporalAmount; import java.util.Calendar; +import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Map; @@ -1681,7 +1682,7 @@ public class DialectDelegateWrapper extends Dialect { } @Override - public String getCheckCondition(String columnName, Set valueSet, JdbcType jdbcType) { + public String getCheckCondition(String columnName, Collection valueSet, JdbcType jdbcType) { return wrapped.getCheckCondition( columnName, valueSet, jdbcType ); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/DiscriminatorHelper.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/DiscriminatorHelper.java index 8ee6f6765e..aab814dd67 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/DiscriminatorHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/DiscriminatorHelper.java @@ -37,9 +37,9 @@ public class DiscriminatorHelper { * and the {@link org.hibernate.type.descriptor.jdbc.JdbcType}. */ static BasicType getDiscriminatorType(PersistentClass persistentClass) { - Type discriminatorType = persistentClass.getDiscriminator().getType(); - if ( discriminatorType instanceof BasicType ) { - return (BasicType) discriminatorType; + final Type discriminatorType = persistentClass.getDiscriminator().getType(); + if ( discriminatorType instanceof BasicType basicType ) { + return basicType; } else { throw new MappingException( "Illegal discriminator type: " + discriminatorType.getName() ); @@ -47,16 +47,16 @@ public class DiscriminatorHelper { } public static BasicType getDiscriminatorType(Component component) { - Type discriminatorType = component.getDiscriminator().getType(); - if ( discriminatorType instanceof BasicType ) { - return (BasicType) discriminatorType; + final Type discriminatorType = component.getDiscriminator().getType(); + if ( discriminatorType instanceof BasicType basicType ) { + return basicType; } else { throw new MappingException( "Illegal discriminator type: " + discriminatorType.getName() ); } } - static String getDiscriminatorSQLValue(PersistentClass persistentClass, Dialect dialect) { + public static String getDiscriminatorSQLValue(PersistentClass persistentClass, Dialect dialect) { if ( persistentClass.isDiscriminatorValueNull() ) { return InFragment.NULL; } @@ -69,16 +69,18 @@ public class DiscriminatorHelper { } private static Object parseDiscriminatorValue(PersistentClass persistentClass) { - BasicType discriminatorType = getDiscriminatorType( persistentClass ); + final BasicType discriminatorType = getDiscriminatorType( persistentClass ); + final String discriminatorValue = persistentClass.getDiscriminatorValue(); try { - return discriminatorType.getJavaTypeDescriptor().fromString( persistentClass.getDiscriminatorValue() ); + return discriminatorType.getJavaTypeDescriptor().fromString( discriminatorValue ); } catch ( Exception e ) { - throw new MappingException( "Could not parse discriminator value", e ); + throw new MappingException( "Could not parse discriminator value '" + discriminatorValue + + "' as discriminator type '" + discriminatorType.getName() + "'", e ); } } - static Object getDiscriminatorValue(PersistentClass persistentClass) { + public static Object getDiscriminatorValue(PersistentClass persistentClass) { if ( persistentClass.isDiscriminatorValueNull() ) { return NULL_DISCRIMINATOR; } @@ -101,10 +103,7 @@ public class DiscriminatorHelper { ); } - public static String jdbcLiteral( - T value, - JdbcLiteralFormatter formatter, - Dialect dialect) { + public static String jdbcLiteral(T value, JdbcLiteralFormatter formatter, Dialect dialect) { try { return formatter.toJdbcLiteral( value, dialect, null ); } @@ -119,26 +118,12 @@ public class DiscriminatorHelper { * domain types, or to {@link StandardBasicTypes#CLASS Class} for non-inherited ones. */ public static SqmExpressible getDiscriminatorType( - SqmPathSource domainType, - NodeBuilder nodeBuilder) { + SqmPathSource domainType, NodeBuilder nodeBuilder) { final SqmPathSource subPathSource = domainType.findSubPathSource( DISCRIMINATOR_ROLE_NAME ); - final SqmExpressible type; - if ( subPathSource != null ) { - type = subPathSource.getSqmPathType(); - } - else { - type = nodeBuilder.getTypeConfiguration() - .getBasicTypeRegistry() - .resolve( StandardBasicTypes.CLASS ); - } + final SqmExpressible type = subPathSource != null + ? subPathSource.getSqmPathType() + : nodeBuilder.getTypeConfiguration().getBasicTypeRegistry().resolve( StandardBasicTypes.CLASS ); //noinspection unchecked return (SqmExpressible) type; } - - static String discriminatorLiteral(JdbcLiteralFormatter formatter, Dialect dialect, Object value) { - return value == NULL_DISCRIMINATOR || value == NOT_NULL_DISCRIMINATOR - ? null - : jdbcLiteral( value, formatter, dialect ); - } - }