HHH-19542 Ensure columns in embeddables default to correct table

beikov · beikov · commit d9082029a673 · 2025-07-07T08:52:07.000+02:00
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotatedColumns.java
@@ -111,7 +111,7 @@ public boolean isSecondary() {
 		final String explicitTableName = firstColumn.getExplicitTableName();
 		//note: checkPropertyConsistency() is responsible for ensuring they all have the same table name
 		return isNotEmpty( explicitTableName )
-			&& !getPropertyHolder().getTable().getName().equals( explicitTableName );
+			&& !getOwnerTable().getName().equals( explicitTableName );
 	}
 
 	/**
@@ -130,10 +130,18 @@ public Table getTable() {
 			// all the columns have to be mapped to the same table
 			// even though at the annotation level it looks like
 			// they could each specify a different table
-			return isSecondary() ? getJoin().getTable() : getPropertyHolder().getTable();
+			return isSecondary() ? getJoin().getTable() : getOwnerTable();
 		}
 	}
 
+	private Table getOwnerTable() {
+		PropertyHolder holder = getPropertyHolder();
+		while ( holder instanceof ComponentPropertyHolder componentPropertyHolder ) {
+			holder = componentPropertyHolder.parent;
+		}
+		return holder.getTable();
+	}
+
 	public void setTable(Table table) {
 		this.table = table;
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ClassPropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ClassPropertyHolder.java
@@ -10,6 +10,7 @@
 import java.util.Map;
 import java.util.function.Consumer;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.AssertionFailure;
 import org.hibernate.MappingException;
 import org.hibernate.PropertyNotFoundException;
@@ -150,7 +151,7 @@ public String getEntityName() {
 	}
 
 	@Override
-	public void addProperty(Property prop, MemberDetails memberDetails, AnnotatedColumns columns, ClassDetails declaringClass) {
+	public void addProperty(Property prop, MemberDetails memberDetails, @Nullable AnnotatedColumns columns, ClassDetails declaringClass) {
 		//AnnotatedColumn.checkPropertyConsistency( ); //already called earlier
 		if ( columns != null ) {
 			if ( columns.isSecondary() ) {
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionPropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionPropertyHolder.java
@@ -8,6 +8,7 @@
 import java.util.Locale;
 import java.util.Map;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.AssertionFailure;
 import org.hibernate.annotations.CollectionType;
 import org.hibernate.annotations.ManyToAny;
@@ -301,7 +302,7 @@ public String getEntityName() {
 	}
 
 	@Override
-	public void addProperty(Property prop, MemberDetails memberDetails, AnnotatedColumns columns, ClassDetails declaringClass) {
+	public void addProperty(Property prop, MemberDetails memberDetails, @Nullable AnnotatedColumns columns, ClassDetails declaringClass) {
 		//Ejb3Column.checkPropertyConsistency( ); //already called earlier
 		throw new AssertionFailure( "addProperty to a join table of a collection: does it make sense?" );
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ComponentPropertyHolder.java
@@ -4,16 +4,13 @@
  */
 package org.hibernate.boot.model.internal;
 
-import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.AnnotationException;
 import org.hibernate.boot.spi.MetadataBuildingContext;
 import org.hibernate.boot.spi.PropertyData;
-import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.AggregateColumn;
 import org.hibernate.mapping.Component;
 import org.hibernate.mapping.Join;
@@ -72,7 +69,6 @@ public class ComponentPropertyHolder extends AbstractPropertyHolder {
 
 	private final String embeddedAttributeName;
 	private final Map<String,AttributeConversionInfo> attributeConversionInfoMap;
-	private final List<AnnotatedColumn> annotatedColumns;
 
 	public ComponentPropertyHolder(
 			Component component,
@@ -99,12 +95,6 @@ public ComponentPropertyHolder(
 			this.embeddedAttributeName = "";
 			this.attributeConversionInfoMap = processAttributeConversions( inferredData.getClassOrElementType() );
 		}
-
-		if ( parent instanceof ComponentPropertyHolder parentHolder ) {
-			this.annotatedColumns = parentHolder.annotatedColumns;
-		} else {
-			this.annotatedColumns = new ArrayList<>();
-		}
 	}
 
 	/**
@@ -231,12 +221,17 @@ public String getEntityName() {
 	}
 
 	@Override
-	public void addProperty(Property property, MemberDetails attributeMemberDetails, AnnotatedColumns columns, ClassDetails declaringClass) {
+	public void addProperty(Property property, MemberDetails attributeMemberDetails, @Nullable AnnotatedColumns columns, ClassDetails declaringClass) {
 		//AnnotatedColumns.checkPropertyConsistency( ); //already called earlier
 		// Check table matches between the component and the columns
 		// if not, change the component table if no properties are set
 		// if a property is set already the core cannot support that
-		final Table table = property.getValue().getTable();
+		assert columns == null || property.getValue().getTable() == columns.getTable();
+		setTable( property.getValue().getTable() );
+		addProperty( property, attributeMemberDetails, declaringClass );
+	}
+
+	private void setTable(Table table) {
 		if ( !table.equals( getTable() ) ) {
 			if ( component.getPropertySpan() == 0 ) {
 				component.setTable( table );
@@ -248,27 +243,8 @@ public void addProperty(Property property, MemberDetails attributeMemberDetails,
 						+ " (all properties of the embeddable class must map to the same table)"
 				);
 			}
-		}
-		if ( columns != null ) {
-			annotatedColumns.addAll( columns.getColumns() );
-		}
-		addProperty( property, attributeMemberDetails, declaringClass );
-	}
-
-	public void checkPropertyConsistency() {
-		if ( annotatedColumns.size() > 1 ) {
-			for ( int currentIndex = 1; currentIndex < annotatedColumns.size(); currentIndex++ ) {
-				final AnnotatedColumn current = annotatedColumns.get( currentIndex );
-				final AnnotatedColumn previous = annotatedColumns.get( currentIndex - 1 );
-				if ( !Objects.equals(
-						StringHelper.nullIfEmpty( current.getExplicitTableName() ),
-						StringHelper.nullIfEmpty( previous.getExplicitTableName() ) ) ) {
-					throw new AnnotationException(
-							"Embeddable class '" + component.getComponentClassName()
-							+ "' has properties mapped to two different tables"
-							+ " (all properties of the embeddable class must map to the same table)"
-					);
-				}
+			if ( parent instanceof ComponentPropertyHolder parentComponentHolder ) {
+				parentComponentHolder.setTable( table );
 			}
 		}
 	}
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java
@@ -455,7 +455,7 @@ static Component fillEmbeddable(
 		if ( LOG.isDebugEnabled() ) {
 			LOG.debug( "Binding component with path: " + subpath );
 		}
-		final ComponentPropertyHolder subholder = buildPropertyHolder(
+		final PropertyHolder subholder = buildPropertyHolder(
 				component,
 				subpath,
 				inferredData,
@@ -579,8 +579,6 @@ else if ( member.hasDirectAnnotationUsage( GeneratedValue.class ) ) {
 			}
 		}
 
-		subholder.checkPropertyConsistency();
-
 		if ( compositeUserType != null ) {
 			processCompositeUserType( component, compositeUserType );
 		}
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolder.java
@@ -4,6 +4,7 @@
  */
 package org.hibernate.boot.model.internal;
 
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.hibernate.annotations.ColumnTransformer;
 import org.hibernate.boot.model.convert.spi.ConverterDescriptor;
 import org.hibernate.mapping.Join;
@@ -33,7 +34,7 @@ public interface PropertyHolder {
 
 	void addProperty(Property prop, MemberDetails memberDetails, ClassDetails declaringClass);
 
-	void addProperty(Property prop, MemberDetails memberDetails, AnnotatedColumns columns, ClassDetails declaringClass);
+	void addProperty(Property prop, MemberDetails memberDetails, @Nullable AnnotatedColumns columns, ClassDetails declaringClass);
 
 	KeyValue getIdentifier();
 
diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyHolderBuilder.java
@@ -47,7 +47,7 @@ public static PropertyHolder buildPropertyHolder(
 	 *
 	 * @return PropertyHolder
 	 */
-	public static ComponentPropertyHolder buildPropertyHolder(
+	public static PropertyHolder buildPropertyHolder(
 			Component component,
 			String path,
 			PropertyData inferredData,

Original file line number	Diff line number	Diff line change
`@@ -8,6 +8,7 @@`
`8`	`8`	`import java.util.Locale;`
`9`	`9`	`import java.util.Map;`
`10`	`10`
	`11`	`+import org.checkerframework.checker.nullness.qual.Nullable;`
`11`	`12`	`import org.hibernate.AssertionFailure;`
`12`	`13`	`import org.hibernate.annotations.CollectionType;`
`13`	`14`	`import org.hibernate.annotations.ManyToAny;`
`@@ -301,7 +302,7 @@ public String getEntityName() {`
`301`	`302`	`}`
`302`	`303`
`303`	`304`	`@Override`
`304`		`- public void addProperty(Property prop, MemberDetails memberDetails, AnnotatedColumns columns, ClassDetails declaringClass) {`
	`305`	`+ public void addProperty(Property prop, MemberDetails memberDetails, @Nullable AnnotatedColumns columns, ClassDetails declaringClass) {`
`305`	`306`	`//Ejb3Column.checkPropertyConsistency( ); //already called earlier`
`306`	`307`	`throw new AssertionFailure( "addProperty to a join table of a collection: does it make sense?" );`
`307`	`308`	`}`
Original file line number	Diff line number	Diff line change
`@@ -455,7 +455,7 @@ static Component fillEmbeddable(`
`455`	`455`	`if ( LOG.isDebugEnabled() ) {`
`456`	`456`	`LOG.debug( "Binding component with path: " + subpath );`
`457`	`457`	`}`
`458`		`- final ComponentPropertyHolder subholder = buildPropertyHolder(`
	`458`	`+ final PropertyHolder subholder = buildPropertyHolder(`
`459`	`459`	`component,`
`460`	`460`	`subpath,`
`461`	`461`	`inferredData,`
`@@ -579,8 +579,6 @@ else if ( member.hasDirectAnnotationUsage( GeneratedValue.class ) ) {`
`579`	`579`	`}`
`580`	`580`	`}`
`581`	`581`
`582`		`- subholder.checkPropertyConsistency();`
`583`		`-`
`584`	`582`	`if ( compositeUserType != null ) {`
`585`	`583`	`processCompositeUserType( component, compositeUserType );`
`586`	`584`	`}`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ public static PropertyHolder buildPropertyHolder(`
`47`	`47`	`*`
`48`	`48`	`* @return PropertyHolder`
`49`	`49`	`*/`
`50`		`- public static ComponentPropertyHolder buildPropertyHolder(`
	`50`	`+ public static PropertyHolder buildPropertyHolder(`
`51`	`51`	`Component component,`
`52`	`52`	`String path,`
`53`	`53`	`PropertyData inferredData,`