Fix mixed inheritance issue

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<String> treatAsDeclarations) {
-		return determineSubclassTableJoinType(
+		if ( isClassOrSuperclassTable( subclassTableNumber ) ) {
-				subclassTableNumber,
+			final boolean shouldInnerJoin = canInnerJoin
-				canInnerJoin,
+					&& !isInverseTable( subclassTableNumber )
-				includeSubclasses,
+					&& !isNullableTable( subclassTableNumber );
-				treatAsDeclarations,
+			// the table is either this persister's driving table or (one of) its super class persister's driving
-				null
+			// 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(

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

@@ -1223,9 +1223,6 @@ public class JoinedSubclassEntityPersister extends AbstractEntityPersister {
 			SqlExpressionResolver sqlExpressionResolver,
 			Supplier<Consumer<Predicate>> 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 );
+					if ( discriminatorValuesByTableName.containsKey( joinedTableReference.getTableExpression() ) ) {
-					info.columnReferences.add( identifierColumnReference );
+						final ColumnReference identifierColumnReference = getIdentifierColumnReference(
-					addWhen(
+								joinedTableReference );
-							caseSearchedExpression,
+						info.columnReferences.add( identifierColumnReference );
-							joinedTableReference,
+						addWhen(
-							identifierColumnReference,
+								caseSearchedExpression,
-							discriminatorType
+								joinedTableReference,
-					);
+								identifierColumnReference,
+								discriminatorType
+						);
+					}
 				}
 		);
 

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;

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;

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<Customer> 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;
+		}
+	}
+}