From ebb3e36db6a5793bb4a3615f2e7ab74839936be7 Mon Sep 17 00:00:00 2001 From: Andrea Boriero Date: Thu, 7 Nov 2019 18:42:11 +0000 Subject: [PATCH] Fix mixed inheritance issue --- .../entity/AbstractEntityPersister.java | 36 ++- .../entity/JoinedSubclassEntityPersister.java | 22 +- .../{ => joined}/JoinedInheritanceTest.java | 2 +- ...JoinedInheritanceWithConcreteRootTest.java | 2 +- .../mapping/joined/MixedInheritanceTest.java | 286 ++++++++++++++++++ 5 files changed, 326 insertions(+), 22 deletions(-) rename hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/{ => joined}/JoinedInheritanceTest.java (99%) rename hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/{ => joined}/JoinedInheritanceWithConcreteRootTest.java (99%) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/MixedInheritanceTest.java 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 a99442e0c9..1a405fabb5 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 @@ -1287,7 +1287,7 @@ public abstract class AbstractEntityPersister createTableReferenceJoin( i, primaryTableReference, - baseJoinType, + determineSubclassTableJoinType( i, true, true, null ), sqlAliasBase, sqlExpressionResolver ) @@ -4251,18 +4251,36 @@ public abstract class AbstractEntityPersister return join; } - protected JoinType determineSubclassTableJoinType( + protected org.hibernate.sql.ast.JoinType determineSubclassTableJoinType( int subclassTableNumber, boolean canInnerJoin, boolean includeSubclasses, Set treatAsDeclarations) { - return determineSubclassTableJoinType( - subclassTableNumber, - canInnerJoin, - includeSubclasses, - treatAsDeclarations, - null - ); + if ( isClassOrSuperclassTable( subclassTableNumber ) ) { + final boolean shouldInnerJoin = canInnerJoin + && !isInverseTable( subclassTableNumber ) + && !isNullableTable( subclassTableNumber ); + // the table is either this persister's driving table or (one of) its super class persister's driving + // tables which can be inner joined as long as the `shouldInnerJoin` condition resolves to true + return shouldInnerJoin ? org.hibernate.sql.ast.JoinType.INNER : org.hibernate.sql.ast.JoinType.LEFT; + } + + // otherwise we have a subclass table and need to look a little deeper... + + // IMPL NOTE : By default includeSubclasses indicates that all subclasses should be joined and that each + // subclass ought to be joined by outer-join. However, TREAT-AS always requires that an inner-join be used + // so we give TREAT-AS higher precedence... + + if ( isSubclassTableIndicatedByTreatAsDeclarations( subclassTableNumber, treatAsDeclarations ) ) { + return org.hibernate.sql.ast.JoinType.INNER; + } + + if ( includeSubclasses + && !isSubclassTableSequentialSelect( subclassTableNumber ) + && !isSubclassTableLazy( subclassTableNumber ) ) { + return org.hibernate.sql.ast.JoinType.LEFT; + } + return org.hibernate.sql.ast.JoinType.INNER; } protected JoinType determineSubclassTableJoinType( 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 28bfdd1aaa..016ccea7ee 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 @@ -1223,9 +1223,6 @@ public class JoinedSubclassEntityPersister extends AbstractEntityPersister { SqlExpressionResolver sqlExpressionResolver, Supplier> additionalPredicateCollectorAccess, SqlAstCreationContext creationContext) { - if ( hasSubclasses() ) { - tableReferenceJoinType = JoinType.LEFT; - } return super.createRootTableGroup( navigablePath, explicitSourceAlias, @@ -1267,14 +1264,17 @@ public class JoinedSubclassEntityPersister extends AbstractEntityPersister { tableReferenceJoins.forEach( tableReferenceJoin -> { final TableReference joinedTableReference = tableReferenceJoin.getJoinedTableReference(); - final ColumnReference identifierColumnReference = getIdentifierColumnReference( joinedTableReference ); - info.columnReferences.add( identifierColumnReference ); - addWhen( - caseSearchedExpression, - joinedTableReference, - identifierColumnReference, - discriminatorType - ); + if ( discriminatorValuesByTableName.containsKey( joinedTableReference.getTableExpression() ) ) { + final ColumnReference identifierColumnReference = getIdentifierColumnReference( + joinedTableReference ); + info.columnReferences.add( identifierColumnReference ); + addWhen( + caseSearchedExpression, + joinedTableReference, + identifierColumnReference, + discriminatorType + ); + } } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceTest.java similarity index 99% rename from hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceTest.java rename to hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceTest.java index 8c3f8a0c32..26b8d9d22f 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceTest.java @@ -4,7 +4,7 @@ * License: GNU Lesser General Public License (LGPL), version 2.1 or later * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html */ -package org.hibernate.orm.test.metamodel.mapping; +package org.hibernate.orm.test.metamodel.mapping.joined; import java.sql.Statement; import java.util.List; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceWithConcreteRootTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceWithConcreteRootTest.java similarity index 99% rename from hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceWithConcreteRootTest.java rename to hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceWithConcreteRootTest.java index cda42b5d08..04a972b4b2 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/JoinedInheritanceWithConcreteRootTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/JoinedInheritanceWithConcreteRootTest.java @@ -4,7 +4,7 @@ * License: GNU Lesser General Public License (LGPL), version 2.1 or later * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html */ -package org.hibernate.orm.test.metamodel.mapping; +package org.hibernate.orm.test.metamodel.mapping.joined; import java.sql.Statement; import java.util.List; diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/MixedInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/MixedInheritanceTest.java new file mode 100644 index 0000000000..dbce3ae92f --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/metamodel/mapping/joined/MixedInheritanceTest.java @@ -0,0 +1,286 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.metamodel.mapping.joined; + +import java.sql.Statement; +import java.util.List; +import javax.persistence.DiscriminatorColumn; +import javax.persistence.DiscriminatorValue; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.Inheritance; +import javax.persistence.InheritanceType; +import javax.persistence.Table; + +import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.persister.entity.JoinedSubclassEntityPersister; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Tags; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.CoreMatchers.instanceOf; +import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertTrue; + +/** + * @author Andrea Boriero + */ +@DomainModel( + annotatedClasses = { + MixedInheritanceTest.Customer.class, + MixedInheritanceTest.DomesticCustomer.class, + MixedInheritanceTest.ForeignCustomer.class, + MixedInheritanceTest.ItalianForeignCustomer.class + } +) +@ServiceRegistry +@SessionFactory +@Tags({ + @Tag("RunnableIdeTest"), +}) +public class MixedInheritanceTest { + @Test + public void basicTest(SessionFactoryScope scope) { + final EntityPersister customerDescriptor = scope.getSessionFactory() + .getMetamodel() + .findEntityDescriptor( Customer.class ); + final EntityPersister domesticCustomerDescriptor = scope.getSessionFactory() + .getMetamodel() + .findEntityDescriptor( DomesticCustomer.class ); + final EntityPersister foreignCustomerDescriptor = scope.getSessionFactory() + .getMetamodel() + .findEntityDescriptor( ForeignCustomer.class ); + + assert customerDescriptor instanceof JoinedSubclassEntityPersister; + + assert customerDescriptor.isTypeOrSuperType( customerDescriptor ); + assert !customerDescriptor.isTypeOrSuperType( domesticCustomerDescriptor ); + assert !customerDescriptor.isTypeOrSuperType( foreignCustomerDescriptor ); + + assert domesticCustomerDescriptor instanceof JoinedSubclassEntityPersister; + + assert domesticCustomerDescriptor.isTypeOrSuperType( customerDescriptor ); + assert domesticCustomerDescriptor.isTypeOrSuperType( domesticCustomerDescriptor ); + assert !domesticCustomerDescriptor.isTypeOrSuperType( foreignCustomerDescriptor ); + + assert foreignCustomerDescriptor instanceof JoinedSubclassEntityPersister; + + assert foreignCustomerDescriptor.isTypeOrSuperType( customerDescriptor ); + assert !foreignCustomerDescriptor.isTypeOrSuperType( domesticCustomerDescriptor ); + assert foreignCustomerDescriptor.isTypeOrSuperType( foreignCustomerDescriptor ); + } + + @Test + public void rootQueryExecutionTest(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + { + // [name, taxId, vat] + final List results = session.createQuery( + "select c from Customer c", + Customer.class + ).list(); + + assertThat( results.size(), is( 2 ) ); + boolean foundDomesticCustomer = false; + boolean foundForeignCustomer = false; + for ( Customer result : results ) { + if ( result.getId() == 1 ) { + assertThat( result, instanceOf( DomesticCustomer.class ) ); + final DomesticCustomer customer = (DomesticCustomer) result; + assertThat( customer.getName(), is( "domestic" ) ); + assertThat( ( customer ).getTaxId(), is( "123" ) ); + foundDomesticCustomer = true; + } + else { + assertThat( result.getId(), is( 2 ) ); + final ForeignCustomer customer = (ForeignCustomer) result; + assertThat( customer.getName(), is( "foreign" ) ); + assertThat( ( customer ).getVat(), is( "987" ) ); + foundForeignCustomer = true; + } + } + assertTrue( foundDomesticCustomer ); + assertTrue( foundForeignCustomer ); + } + } + ); + } + + @Test + public void subclassQueryExecutionTest(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + { + final DomesticCustomer result = session.createQuery( + "select c from DomesticCustomer c", + DomesticCustomer.class + ).uniqueResult(); + + assertThat( result, notNullValue() ); + assertThat( result.getId(), is( 1 ) ); + assertThat( result.getName(), is( "domestic" ) ); + assertThat( result.getTaxId(), is( "123" ) ); + } + + { + final ForeignCustomer result = session.createQuery( + "select c from ForeignCustomer c", + ForeignCustomer.class + ).uniqueResult(); + + assertThat( result, notNullValue() ); + assertThat( result.getId(), is( 2 ) ); + assertThat( result.getName(), is( "foreign" ) ); + assertThat( result.getVat(), is( "987" ) ); + } + } + ); + } + + @BeforeEach + public void createTestData(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + session.persist( new DomesticCustomer( 1, "domestic", "123" ) ); + session.persist( new ForeignCustomer( 2, "foreign", "987" ) ); + } + ); + } + + @AfterEach + public void cleanupTestData(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + session.doWork( + work -> { + Statement statement = work.createStatement(); + try { + statement.execute( "delete from DomesticCustomer" ); + statement.execute( "delete from ForeignCustomer" ); + statement.execute( "delete from Customer" ); + } + finally { + statement.close(); + } + } + ); + } + ); + } + + @Entity(name = "Customer") + @Inheritance(strategy = InheritanceType.JOINED) + @Table(name = "Customer") + public static abstract class Customer { + private Integer id; + private String name; + + public Customer() { + } + + public Customer(Integer id, String name) { + this.id = id; + this.name = name; + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "DomesticCustomer") + @Table(name = "DomesticCustomer") + public static class DomesticCustomer extends Customer { + private String taxId; + + public DomesticCustomer() { + } + + public DomesticCustomer(Integer id, String name, String taxId) { + super( id, name ); + this.taxId = taxId; + } + + public String getTaxId() { + return taxId; + } + + public void setTaxId(String taxId) { + this.taxId = taxId; + } + } + + @Entity(name = "ForeignCustomer") + @Table(name = "ForeignCustomer") + @Inheritance(strategy = InheritanceType.SINGLE_TABLE) + @DiscriminatorColumn( name = "cust_type" ) + @DiscriminatorValue("FC") + public static class ForeignCustomer extends Customer { + private String vat; + + public ForeignCustomer() { + } + + public ForeignCustomer(Integer id, String name, String vat) { + super( id, name ); + this.vat = vat; + } + + public String getVat() { + return vat; + } + + public void setVat(String vat) { + this.vat = vat; + } + } + + @Entity(name = "ItalianForeignCustomer") + @DiscriminatorValue("IFC") + public static class ItalianForeignCustomer extends ForeignCustomer{ + private String code; + + public ItalianForeignCustomer() { + } + + public ItalianForeignCustomer(Integer id, String name, String vat, String code) { + super( id, name, vat ); + this.code = code; + } + + public String getCode() { + return code; + } + + public void setCode(String code) { + this.code = code; + } + } +}