From 666dea3c7a4abd710ba4cdd3f3e20ab0cf510ad3 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 11 Dec 2023 13:49:39 +0100 Subject: [PATCH 1/2] HHH-17483 Add test for issue --- .../ManyToOneInheritanceSubTypeTest.java | 216 +++++++++++++----- 1 file changed, 159 insertions(+), 57 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/inheritance/ManyToOneInheritanceSubTypeTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/inheritance/ManyToOneInheritanceSubTypeTest.java index 6eff309baec7..9511fe28a81d 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/inheritance/ManyToOneInheritanceSubTypeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/inheritance/ManyToOneInheritanceSubTypeTest.java @@ -14,55 +14,95 @@ import org.hibernate.testing.orm.junit.Jira; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; import jakarta.persistence.DiscriminatorColumn; import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; import jakarta.persistence.Id; import jakarta.persistence.Inheritance; import jakarta.persistence.InheritanceType; import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; import jakarta.persistence.OneToMany; import static org.assertj.core.api.Assertions.assertThat; /** * @author Laurent Almeras + * @author Marco Belladelli */ @DomainModel( annotatedClasses = { - ManyToOneInheritanceSubTypeTest.SuperType.class, - ManyToOneInheritanceSubTypeTest.TypeA.class, - ManyToOneInheritanceSubTypeTest.TypeB.class, - ManyToOneInheritanceSubTypeTest.LinkedEntity.class + ManyToOneInheritanceSubTypeTest.LinkedEntity.class, + ManyToOneInheritanceSubTypeTest.SingleTableEntity.class, + ManyToOneInheritanceSubTypeTest.SingleA.class, + ManyToOneInheritanceSubTypeTest.SubSingleA.class, + ManyToOneInheritanceSubTypeTest.SingleB.class, + ManyToOneInheritanceSubTypeTest.JoinedEntity.class, + ManyToOneInheritanceSubTypeTest.JoinedA.class, + ManyToOneInheritanceSubTypeTest.SubJoinedA.class, + ManyToOneInheritanceSubTypeTest.JoinedB.class, + ManyToOneInheritanceSubTypeTest.UnionEntity.class, + ManyToOneInheritanceSubTypeTest.UnionA.class, + ManyToOneInheritanceSubTypeTest.SubUnionA.class, + ManyToOneInheritanceSubTypeTest.UnionB.class, } ) @SessionFactory( useCollectingStatementInspector = true ) @Jira( "https://hibernate.atlassian.net/browse/HHH-16616" ) +@Jira( "https://hibernate.atlassian.net/browse/HHH-17483" ) public class ManyToOneInheritanceSubTypeTest { @BeforeAll public void setUp(SessionFactoryScope scope) { scope.inTransaction( session -> { - final SuperType superType = new SuperType( 1 ); - final TypeB typeB = new TypeB( 2, "typeB" ); - final TypeA typeA = new TypeA( 3, "typeA" ); - final LinkedEntity entity = new LinkedEntity( 4 ); - entity.addTypeA( typeA ); - session.persist( superType ); - session.persist( typeB ); - session.persist( typeA ); + final LinkedEntity entity = new LinkedEntity( 1 ); + final SingleA singleA = new SingleA(); + session.persist( singleA ); + entity.getSingle().add( singleA ); + final SubSingleA subSingleA = new SubSingleA(); + session.persist( subSingleA ); + entity.getSingle().add( subSingleA ); + session.persist( new SingleB() ); + final JoinedA joinedA = new JoinedA(); + session.persist( joinedA ); + entity.getJoined().add( joinedA ); + final SubJoinedA subJoinedA = new SubJoinedA(); + session.persist( subJoinedA ); + entity.getJoined().add( subJoinedA ); + session.persist( new JoinedB() ); + final UnionA unionA = new UnionA(); + unionA.setLinkedEntity( entity ); + session.persist( unionA ); + final SubUnionA subUnionA = new SubUnionA(); + subUnionA.setLinkedEntity( entity ); + session.persist( subUnionA ); + session.persist( new UnionB() ); session.persist( entity ); } ); } + @AfterAll + public void tearDown(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.createMutationQuery( "delete from SingleTableEntity" ).executeUpdate(); + session.createMutationQuery( "delete from JoinedEntity" ).executeUpdate(); + session.createMutationQuery( "delete from UnionEntity" ).executeUpdate(); + session.createMutationQuery( "delete from LinkedEntity" ).executeUpdate(); + } ); + } + @Test public void testFind(SessionFactoryScope scope) { final SQLStatementInspector inspector = scope.getCollectingStatementInspector(); inspector.clear(); scope.inTransaction( session -> { - final LinkedEntity entity = session.find( LinkedEntity.class, 4 ); - assertThat( entity.getTypeAS() ).hasSize( 1 ); - inspector.assertExecutedCount( 2 ); - inspector.assertNumberOfOccurrenceInQueryNoSpace( 1, "disc_col", 1 ); + final LinkedEntity entity = session.find( LinkedEntity.class, 1 ); + inspector.clear(); + assertThat( entity.getSingle() ).hasSize( 2 ); + inspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "disc_col", 2 ); + assertThat( entity.getJoined() ).hasSize( 2 ); + assertThat( entity.getUnion() ).hasSize( 2 ); } ); } @@ -72,78 +112,140 @@ public void testJoinFetch(SessionFactoryScope scope) { inspector.clear(); scope.inTransaction( session -> { final LinkedEntity entity = session.createQuery( - "from LinkedEntity e left join fetch e.typeAS", + "from LinkedEntity e join fetch e.single", + LinkedEntity.class + ).getSingleResult(); + assertThat( entity.getSingle() ).hasSize( 2 ); + inspector.assertExecutedCount( 1 ); + inspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "disc_col", 2 ); + } ); + inspector.clear(); + scope.inTransaction( session -> { + final LinkedEntity entity = session.createQuery( + "from LinkedEntity e join fetch e.joined", + LinkedEntity.class + ).getSingleResult(); + assertThat( entity.getJoined() ).hasSize( 2 ); + inspector.assertExecutedCount( 1 ); + } ); + inspector.clear(); + scope.inTransaction( session -> { + final LinkedEntity entity = session.createQuery( + "from LinkedEntity e join fetch e.union", LinkedEntity.class ).getSingleResult(); - assertThat( entity.getTypeAS() ).hasSize( 1 ); + assertThat( entity.getUnion() ).hasSize( 2 ); inspector.assertExecutedCount( 1 ); - inspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "disc_col", 1 ); } ); } - @Entity( name = "SuperType" ) - @Inheritance( strategy = InheritanceType.SINGLE_TABLE ) - @DiscriminatorColumn( name = "disc_col" ) - public static class SuperType { + @Entity( name = "LinkedEntity" ) + public static class LinkedEntity { @Id private Integer id; - public SuperType() { + @OneToMany + @JoinColumn( name = "single_id" ) + private List single = new ArrayList<>(); + + @OneToMany + @JoinColumn( name = "joined_id" ) + private List joined = new ArrayList<>(); + + @OneToMany( mappedBy = "linkedEntity" ) + private List union = new ArrayList<>(); + + public LinkedEntity() { } - public SuperType(Integer id) { + public LinkedEntity(Integer id) { this.id = id; } - } - @Entity( name = "TypeA" ) - public static class TypeA extends SuperType { - private String typeAName; + public List getSingle() { + return single; + } - public TypeA() { + public List getJoined() { + return joined; } - public TypeA(Integer id, String typeAName) { - super( id ); - this.typeAName = typeAName; + public List getUnion() { + return union; } } - @Entity( name = "TypeB" ) - public static class TypeB extends SuperType { - private String typeBName; + // InheritanceType.SINGLE_TABLE - public TypeB() { - } + @Entity( name = "SingleTableEntity" ) + @Inheritance( strategy = InheritanceType.SINGLE_TABLE ) + @DiscriminatorColumn( name = "disc_col" ) + public static class SingleTableEntity { + @Id + @GeneratedValue + private Integer id; + } - public TypeB(Integer id, String typeBName) { - super( id ); - this.typeBName = typeBName; - } + @Entity( name = "SingleA" ) + public static class SingleA extends SingleTableEntity { } - @Entity( name = "LinkedEntity" ) - public static class LinkedEntity { + @Entity( name = "SubSingleA" ) + public static class SubSingleA extends SingleA { + } + + @Entity( name = "SingleB" ) + public static class SingleB extends SingleTableEntity { + } + + // InheritanceType.JOINED + + @Entity( name = "JoinedEntity" ) + @Inheritance( strategy = InheritanceType.JOINED ) + public static class JoinedEntity { @Id + @GeneratedValue private Integer id; + } - @OneToMany - @JoinColumn( name = "linked_id" ) - private List typeAS = new ArrayList<>(); + @Entity( name = "JoinedA" ) + public static class JoinedA extends JoinedEntity { + } - public LinkedEntity() { - } + @Entity( name = "SubJoinedA" ) + public static class SubJoinedA extends JoinedA { + } - public LinkedEntity(Integer id) { - this.id = id; - } + @Entity( name = "JoinedB" ) + public static class JoinedB extends JoinedEntity { + } - public List getTypeAS() { - return typeAS; - } + // InheritanceType.TABLE_PER_CLASS + + @Entity( name = "UnionEntity" ) + @Inheritance( strategy = InheritanceType.TABLE_PER_CLASS ) + public static class UnionEntity { + @Id + @GeneratedValue + private Integer id; + } + + @Entity( name = "UnionA" ) + public static class UnionA extends UnionEntity { + @ManyToOne + @JoinColumn( name = "linked_id" ) + private LinkedEntity linkedEntity; - public void addTypeA(TypeA typeA) { - this.typeAS.add( typeA ); + public void setLinkedEntity(LinkedEntity linkedEntity) { + this.linkedEntity = linkedEntity; } } + + @Entity( name = "SubUnionA" ) + public static class SubUnionA extends UnionA { + } + + @Entity( name = "UnionB" ) + public static class UnionB extends UnionEntity { + } } From 370d30f903e07319dc9d52d3872858697dd06465 Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Mon, 11 Dec 2023 11:06:58 +0100 Subject: [PATCH 2/2] HHH-17483 Fix applyDiscriminator treat for nested inheritance subtypes Also small fix to joined-inheritance pruning. --- .../entity/AbstractEntityPersister.java | 22 +++++++++++-- .../entity/JoinedSubclassEntityPersister.java | 16 ++++++---- .../entity/SingleTableEntityPersister.java | 2 +- .../entity/UnionSubclassEntityPersister.java | 2 +- .../sqm/sql/BaseSqmToSqlAstConverter.java | 32 +++++++++++++------ 5 files changed, 53 insertions(+), 21 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 066196692d00..2f1014830eab 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -2999,7 +2999,7 @@ protected void logStaticSQL() { public abstract Map getSubclassByDiscriminatorValue(); - protected abstract boolean needsDiscriminator(); + public abstract boolean needsDiscriminator(); protected boolean isDiscriminatorFormula() { return false; @@ -3092,7 +3092,25 @@ public void applyDiscriminator( TableGroup tableGroup, SqlAstCreationState creationState) { if ( needsDiscriminator() ) { - pruneForSubclasses( tableGroup, Collections.singletonMap( getEntityName(), EntityNameUse.TREAT ) ); + assert !creationState.supportsEntityNameUsage() : "Entity name usage should have been used instead"; + final Map entityNameUseMap; + final Collection subMappingTypes = getSubMappingTypes(); + if ( subMappingTypes.isEmpty() ) { + entityNameUseMap = Collections.singletonMap( getEntityName(), EntityNameUse.TREAT ); + } + else { + entityNameUseMap = new HashMap<>( 1 + subMappingTypes.size() ); + entityNameUseMap.put( getEntityName(), EntityNameUse.TREAT ); + // We need to register TREAT uses for all subtypes when pruning + for ( EntityMappingType subMappingType : subMappingTypes ) { + entityNameUseMap.put( subMappingType.getEntityName(), EntityNameUse.TREAT ); + } + if ( isInherited() ) { + // Make sure the table group includes the root table when needed for TREAT + tableGroup.resolveTableReference( getRootTableName() ); + } + } + pruneForSubclasses( tableGroup, entityNameUseMap ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/JoinedSubclassEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/JoinedSubclassEntityPersister.java index 612174ed5e0f..3077e1c95d86 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/JoinedSubclassEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/JoinedSubclassEntityPersister.java @@ -742,7 +742,7 @@ private void associateSubclassNamesToSubclassTableIndex( } @Override - protected boolean needsDiscriminator() { + public boolean needsDiscriminator() { return forceDiscriminator; } @@ -1323,15 +1323,17 @@ public void pruneForSubclasses(TableGroup tableGroup, Map final String[] subclassTableNames = persister.getSubclassTableNames(); // Build the intersection of all tables names that are of the class or super class // These are the tables that can be safely inner joined - if ( tablesToInnerJoin.isEmpty() ) { - for ( int i = 0; i < subclassTableNames.length; i++ ) { - if ( persister.isClassOrSuperclassTable[i] ) { - tablesToInnerJoin.add( subclassTableNames[i] ); - } + final Set classOrSuperclassTables = new HashSet<>( subclassTableNames.length ); + for ( int i = 0; i < subclassTableNames.length; i++ ) { + if ( persister.isClassOrSuperclassTable[i] ) { + classOrSuperclassTables.add( subclassTableNames[i] ); } } + if ( tablesToInnerJoin.isEmpty() ) { + tablesToInnerJoin.addAll( classOrSuperclassTables ); + } else { - tablesToInnerJoin.retainAll( Arrays.asList( subclassTableNames ) ); + tablesToInnerJoin.retainAll( classOrSuperclassTables ); } if ( useKind == EntityNameUse.UseKind.FILTER && explicitDiscriminatorColumnName == null ) { // If there is no discriminator column, diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java index c046aa64a0f1..6cff99cb9927 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java @@ -548,7 +548,7 @@ public String fromTableFragment(String name) { } @Override - protected boolean needsDiscriminator() { + public boolean needsDiscriminator() { return forceDiscriminator || isInherited(); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/UnionSubclassEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/UnionSubclassEntityPersister.java index d98449cdf4c7..9880922620b3 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/UnionSubclassEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/UnionSubclassEntityPersister.java @@ -286,7 +286,7 @@ public TableGroup createRootTableGroup( } @Override - protected boolean needsDiscriminator() { + public boolean needsDiscriminator() { return false; } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index aca56bf3eec4..34154c542c9c 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -3118,20 +3118,30 @@ private void registerEntityNameUsage( ); } } - if ( useKind == EntityNameUse.UseKind.TREAT || useKind == EntityNameUse.UseKind.PROJECTION ) { - // If we encounter a treat use, we also want register the use for all subtypes. - // We do this here to not have to expand entity name uses during pruning later on + // If we encounter a treat or projection use, we also want register the use for all subtypes. + // We do this here to not have to expand entity name uses during pruning later on + if ( useKind == EntityNameUse.UseKind.TREAT ) { for ( EntityMappingType subType : persister.getSubMappingTypes() ) { entityNameUses.compute( subType.getEntityName(), (s, existingUse) -> finalEntityNameUse.stronger( existingUse ) ); - if ( useKind == EntityNameUse.UseKind.PROJECTION ) { - actualTableGroup.resolveTableReference( - null, - subType.getEntityPersister().getMappedTableDetails().getTableName() - ); - } + } + if ( persister.isInherited() && persister.needsDiscriminator() ) { + // Make sure the table group includes the root table when needed for TREAT + actualTableGroup.resolveTableReference( persister.getRootTableName() ); + } + } + else if ( useKind == EntityNameUse.UseKind.PROJECTION ) { + for ( EntityMappingType subType : persister.getSubMappingTypes() ) { + entityNameUses.compute( + subType.getEntityName(), + (s, existingUse) -> finalEntityNameUse.stronger( existingUse ) + ); + actualTableGroup.resolveTableReference( + null, + subType.getEntityPersister().getMappedTableDetails().getTableName() + ); } } } @@ -8238,7 +8248,9 @@ else if ( getLoadQueryInfluencers().hasEnabledFetchProfiles() ) { if ( entityMappingType.getSuperMappingType() != null ) { // A joined table group was created by an enabled entity graph or fetch profile, // and it's of an inheritance subtype, so we should apply the discriminator - entityMappingType.applyDiscriminator( null, null, actualTableGroup, this ); + getCurrentClauseStack().push( Clause.FROM ); + registerEntityNameUsage( actualTableGroup, EntityNameUse.TREAT, entityMappingType.getEntityName() ); + getCurrentClauseStack().pop(); } } }