HHH-18929 generate check constraints for discriminator columns

hibernate-core/src/main/java/org/hibernate/annotations/DiscriminatorOptions.java

@@ -28,6 +28,10 @@
 	 * instances of a root entity and its subtypes. This is useful if
 	 * there are discriminator column values which do <em>not</em>
 	 * map to any subtype of the root entity type.
+	 * <p>
+	 * 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

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<String, PersistentClass> 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<String> discriminatorValues(PersistentClass rootClass) {
+		final List<String> 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;
+	}
+}

hibernate-core/src/main/java/org/hibernate/boot/model/internal/EntityBinder.java

@@ -958,7 +958,9 @@ private void bindDiscriminatorColumnToRootPersistentClass(
 			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() ) );
 		}
 	}
 

hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java

@@ -189,6 +189,7 @@
 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 String getCheckCondition(String columnName, Long[] values) {
 	}
 
 	/**
-	 * 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 String getCheckCondition(String columnName, Set<?> valueSet, JdbcType jdb
 				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 = ",";
 		}

hibernate-core/src/main/java/org/hibernate/dialect/DialectDelegateWrapper.java

@@ -12,6 +12,7 @@
 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 String getCheckCondition(String columnName, Long[] values) {
 	}
 
 	@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 );
 	}
 

hibernate-core/src/main/java/org/hibernate/persister/entity/DiscriminatorHelper.java

@@ -37,26 +37,26 @@ 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() );
 		}
 	}
 
 	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 @@ else if ( persistentClass.isDiscriminatorValueNotNull() ) {
 	}
 
 	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 @@ private static <T> String discriminatorSqlLiteral(
 		);
 	}
 
-	public static <T> String jdbcLiteral(
-			T value,
-			JdbcLiteralFormatter<T> formatter,
-			Dialect dialect) {
+	public static <T> String jdbcLiteral(T value, JdbcLiteralFormatter<T> formatter, Dialect dialect) {
 		try {
 			return formatter.toJdbcLiteral( value, dialect, null );
 		}
@@ -119,26 +118,12 @@ public static <T> String jdbcLiteral(
 	 * domain types, or to {@link StandardBasicTypes#CLASS Class} for non-inherited ones.
 	 */
 	public static <T> SqmExpressible<? super T> getDiscriminatorType(
-			SqmPathSource<T> domainType,
-			NodeBuilder nodeBuilder) {
+			SqmPathSource<T> 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<? super T>) type;
 	}
-
-	static String discriminatorLiteral(JdbcLiteralFormatter<Object> formatter, Dialect dialect, Object value) {
-		return value == NULL_DISCRIMINATOR || value == NOT_NULL_DISCRIMINATOR
-				? null
-				: jdbcLiteral( value, formatter, dialect );
-	}
-
 }

hibernate-core/src/test/java/org/hibernate/orm/test/boot/models/xml/complete/DiscriminatorValueTest.java

@@ -39,14 +39,14 @@ void testDiscriminatorValue(ServiceRegistryScope registryScope) {
 
 		{
 			final ClassDetails rootClassDetails = classDetailsRegistry.getClassDetails( Root.class.getName() );
-			assertThat( rootClassDetails.hasDirectAnnotationUsage( DiscriminatorValue.class ) ).isFalse();
+			assertThat( rootClassDetails.hasDirectAnnotationUsage( DiscriminatorValue.class ) ).isTrue();
 			assertThat( rootClassDetails.hasDirectAnnotationUsage( DiscriminatorFormula.class ) ).isFalse();
 
 			final DiscriminatorColumn discriminatorColumn = rootClassDetails.getDirectAnnotationUsage(
 					DiscriminatorColumn.class );
 			assertThat( discriminatorColumn ).isNotNull();
 			assertThat( discriminatorColumn.name() ).isEqualTo( "TYPE_COLUMN" );
-			assertThat( discriminatorColumn.discriminatorType() ).isEqualTo( DiscriminatorType.INTEGER );
+			assertThat( discriminatorColumn.discriminatorType() ).isEqualTo( DiscriminatorType.CHAR );
 
 			final ClassDetails subClassDetails = classDetailsRegistry.getClassDetails( Sub.class.getName() );
 			assertThat( subClassDetails.hasDirectAnnotationUsage( DiscriminatorColumn.class ) ).isFalse();

hibernate-core/src/test/resources/mappings/models/complete/discriminator-value.xml

@@ -11,7 +11,8 @@
 
     <entity class="Root" metadata-complete="true" access="FIELD">
         <inheritance strategy="JOINED"/>
-        <discriminator-column name="TYPE_COLUMN" discriminator-type="INTEGER"/>
+        <discriminator-value>P</discriminator-value>
+        <discriminator-column name="TYPE_COLUMN" discriminator-type="CHAR"/>
         <attributes>
             <id name="id">
                 <column name="pk"/>