Skip to content

HHH-19542 Ensure columns in embeddables default to correct table #10476

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}

/**
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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?" );
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.HashMap;
import java.util.Map;

import org.checkerframework.checker.nullness.qual.Nullable;
import org.hibernate.AnnotationException;
import org.hibernate.boot.spi.MetadataBuildingContext;
import org.hibernate.boot.spi.PropertyData;
Expand Down Expand Up @@ -220,27 +221,32 @@ public String getEntityName() {
}

@Override
public void addProperty(Property property, MemberDetails attributeMemberDetails, AnnotatedColumns columns, ClassDetails declaringClass) {
//Ejb3Column.checkPropertyConsistency( ); //already called earlier
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
if ( columns != null ) {
final Table table = columns.getTable();
if ( !table.equals( getTable() ) ) {
if ( component.getPropertySpan() == 0 ) {
component.setTable( table );
}
else {
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)"
);
}
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 );
}
else {
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 );
}
}
addProperty( property, attributeMemberDetails, declaringClass );
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -352,15 +352,15 @@ private static PropertyBinder createEmbeddedProperty(
final PropertyBinder binder = new PropertyBinder();
binder.setDeclaringClass( inferredData.getDeclaringClass() );
binder.setName( inferredData.getPropertyName() );
binder.setValue(component);
binder.setValue( component );
binder.setMemberDetails( inferredData.getAttributeMember() );
binder.setAccessType( inferredData.getDefaultAccess() );
binder.setEmbedded(isComponentEmbedded);
binder.setHolder(propertyHolder);
binder.setId(isId);
binder.setEntityBinder(entityBinder);
binder.setInheritanceStatePerClass(inheritanceStatePerClass);
binder.setBuildingContext(context);
binder.setEmbedded( isComponentEmbedded );
binder.setHolder( propertyHolder );
binder.setId( isId );
binder.setEntityBinder( entityBinder );
binder.setInheritanceStatePerClass( inheritanceStatePerClass );
binder.setBuildingContext( context );
binder.makePropertyAndBind();
return binder;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class EmbeddableA {
@AttributeOverrides({@AttributeOverride(name = "embedAttrB" , column = @Column(table = "TableB"))})
private EmbeddableB embedB;

@Column(table = "TableB")
private String embedAttrA;

public EmbeddableB getEmbedB() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import jakarta.persistence.Table;
import org.hibernate.boot.MetadataSources;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.NotImplementedYet;
import org.hibernate.testing.orm.junit.ServiceRegistry;
import org.hibernate.testing.orm.junit.ServiceRegistryScope;
import org.junit.jupiter.api.Test;
Expand All @@ -32,7 +31,6 @@
class NestedEmbeddedObjectWithASecondaryTableTest {

@Test
@NotImplementedYet
void testNestedEmbeddedAndSecondaryTables(ServiceRegistryScope registryScope) {
final MetadataSources metadataSources = new MetadataSources( registryScope.getRegistry() )
.addAnnotatedClasses( Book.class, Author.class, House.class );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
* SPDX-License-Identifier: Apache-2.0
* Copyright Red Hat Inc. and Hibernate Authors
*/
package org.hibernate.orm.test.records;

import jakarta.persistence.Column;
import jakarta.persistence.Embeddable;
import jakarta.persistence.Entity;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.Id;
import jakarta.persistence.SecondaryTable;
import jakarta.persistence.Table;
import org.hibernate.AnnotationException;
import org.hibernate.boot.MetadataSources;
import org.hibernate.boot.registry.StandardServiceRegistry;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.ServiceRegistryScope;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

@JiraKey("HHH-19542")
@DomainModel(annotatedClasses = {
RecordNestedEmbeddedWithASecondaryTableTest.UserEntity.class
})
@SessionFactory
class RecordNestedEmbeddedWithASecondaryTableTest {

private UserEntity user;

@BeforeAll
void prepare(SessionFactoryScope scope) {
scope.inTransaction( session -> {
Person person = new Person( new FullName( "Sylvain", "Lecoy" ), 38 );
user = new UserEntity( person );
session.persist( user );
} );
}

@Test
void test(SessionFactoryScope scope) {
scope.inTransaction(session -> {
UserEntity entity = session.find( UserEntity.class, user.id );
assertThat( entity ).isNotNull();
assertThat( entity.id ).isEqualTo( user.id );
assertThat( entity.person ).isNotNull();
assertThat( entity.person.age ).isEqualTo( 38 );
assertThat( entity.person.fullName.firstName ).isEqualTo( "Sylvain" );
assertThat( entity.person.fullName.lastName ).isEqualTo( "Lecoy" );
});
}

@Test
void test(ServiceRegistryScope scope) {
final StandardServiceRegistry registry = scope.getRegistry();
final MetadataSources sources = new MetadataSources( registry ).addAnnotatedClass( UserEntity1.class );

try {
sources.buildMetadata();
fail( "Expecting to fail" );
} catch (AnnotationException expected) {
assertThat( expected ).hasMessageContaining( "all properties of the embeddable class must map to the same table" );
}
}

@Entity
@Table(name = "UserEntity")
@SecondaryTable(name = "Person")
static class UserEntity {
@Id
@GeneratedValue
private Integer id;
private Person person;

public UserEntity(
final Person person) {
this.person = person;
}

protected UserEntity() {

}
}

@Embeddable
record Person(
FullName fullName,
@Column(table = "Person")
Integer age) {

}

@Embeddable
record FullName(
@Column(table = "Person")
String firstName,
@Column(table = "Person")
String lastName) {

}

@Entity
@Table(name = "UserEntity")
@SecondaryTable(name = "Person")
public static class UserEntity1 {
@Id
@GeneratedValue
private Integer id;
private Person1 person;

public UserEntity1(
final Person1 person) {
this.person = person;
}

protected UserEntity1() {

}
}

@Embeddable
public record Person1(
FullName1 fullName,
@Column(table = "Person")
Integer age) {

}

@Embeddable
public record FullName1(
@Column(table = "Person")
String firstName,
String lastName) {

}
}
Loading