HHH-12895 - Extra LEFT JOIN generated with @ManyToOne and @JoinTable when projecting

on main entity id

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

@@ -318,6 +318,24 @@ public abstract class AbstractEntityPersister
 
 	protected abstract boolean isClassOrSuperclassTable(int j);
 
+	protected boolean isClassOrSuperclassJoin(int j) {
+		/*
+		 * TODO:
+		 *  SingleTableEntityPersister incorrectly used isClassOrSuperclassJoin == isClassOrSuperclassTable,
+		 *  this caused HHH-12895, as this resulted in the subclass tables always being joined, even if no
+		 *  property on these tables was accessed.
+		 *
+		 *  JoinedTableEntityPersister does not use isClassOrSuperclassJoin at all, probably incorrectly so.
+		 *  I however haven't been able to reproduce any quirks regarding <join>s, secondary tables or
+		 *  @JoinTable's.
+		 *
+		 *  Probably this method needs to be properly implemented for the various entity persisters,
+		 *  but this at least fixes the SingleTableEntityPersister, while maintaining the the
+		 *  previous behaviour for other persisters.
+		 */
+		return isClassOrSuperclassTable( j );
+	}
+
 	public abstract int getSubclassTableSpan();
 
 	protected abstract int getTableSpan();
@@ -4005,7 +4023,7 @@ public abstract class AbstractEntityPersister
 			Set<String> treatAsDeclarations,
 			Set<String> referencedTables) {
 
-		if ( isClassOrSuperclassTable( subclassTableNumber ) ) {
+		if ( isClassOrSuperclassJoin( subclassTableNumber ) ) {
 			String superclassTableName = getSubclassTableName( subclassTableNumber );
 			if ( referencedTables != null && canOmitSuperclassTableJoin() && !referencedTables.contains(
 					superclassTableName ) ) {

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

@@ -75,6 +75,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 	private final boolean[] subclassTableSequentialSelect;
 	private final String[][] subclassTableKeyColumnClosure;
 	private final boolean[] isClassOrSuperclassTable;
+	private final boolean[] isClassOrSuperclassJoin;
 
 	// properties of this class, including inherited properties
 	private final int[] propertyTableNumbers;
@@ -227,6 +228,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		ArrayList<String> subclassTables = new ArrayList<String>();
 		ArrayList<String[]> joinKeyColumns = new ArrayList<String[]>();
 		ArrayList<Boolean> isConcretes = new ArrayList<Boolean>();
+		ArrayList<Boolean> isClassOrSuperclassJoins = new ArrayList<Boolean>();
 		ArrayList<Boolean> isDeferreds = new ArrayList<Boolean>();
 		ArrayList<Boolean> isInverses = new ArrayList<Boolean>();
 		ArrayList<Boolean> isNullables = new ArrayList<Boolean>();
@@ -234,6 +236,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		subclassTables.add( qualifiedTableNames[0] );
 		joinKeyColumns.add( getIdentifierColumnNames() );
 		isConcretes.add( Boolean.TRUE );
+		isClassOrSuperclassJoins.add( Boolean.TRUE );
 		isDeferreds.add( Boolean.FALSE );
 		isInverses.add( Boolean.FALSE );
 		isNullables.add( Boolean.FALSE );
@@ -241,14 +244,15 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		joinIter = persistentClass.getSubclassJoinClosureIterator();
 		while ( joinIter.hasNext() ) {
 			Join join = (Join) joinIter.next();
-			isConcretes.add( persistentClass.isClassOrSuperclassJoin( join ) );
+			isConcretes.add( persistentClass.isClassOrSuperclassTable( join.getTable() ) );
-			isDeferreds.add( join.isSequentialSelect() );
+			isClassOrSuperclassJoins.add( persistentClass.isClassOrSuperclassJoin( join ) );
 			isInverses.add( join.isInverse() );
 			isNullables.add( join.isOptional() );
 			isLazies.add( lazyAvailable && join.isLazy() );
-			if ( join.isSequentialSelect() && !persistentClass.isClassOrSuperclassJoin( join ) ) {
+
-				hasDeferred = true;
+			boolean isDeferred = join.isSequentialSelect() && ! persistentClass.isClassOrSuperclassJoin( join ) ;
-			}
+			isDeferreds.add( isDeferred );
+			hasDeferred |= isDeferred;
 
 			String joinTableName = determineTableName( join.getTable(), jdbcEnvironment );
 			subclassTables.add( joinTableName );
@@ -268,6 +272,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		subclassTableIsLazyClosure = ArrayHelper.toBooleanArray( isLazies );
 		subclassTableKeyColumnClosure = ArrayHelper.to2DStringArray( joinKeyColumns );
 		isClassOrSuperclassTable = ArrayHelper.toBooleanArray( isConcretes );
+		isClassOrSuperclassJoin = ArrayHelper.toBooleanArray( isClassOrSuperclassJoins );
 		isInverseSubclassTable = ArrayHelper.toBooleanArray( isInverses );
 		isNullableSubclassTable = ArrayHelper.toBooleanArray( isNullables );
 		hasSequentialSelects = hasDeferred;
@@ -797,6 +802,10 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		return isClassOrSuperclassTable[j];
 	}
 
+	protected boolean isClassOrSuperclassJoin(int j) {
+		return isClassOrSuperclassJoin[j];
+	}
+
 	protected boolean isSubclassTableLazy(int j) {
 		return subclassTableIsLazyClosure[j];
 	}
@@ -831,6 +840,11 @@ public class SingleTableEntityPersister extends AbstractEntityPersister {
 		}
 	}
 
+	@Override
+	public boolean canOmitSuperclassTableJoin() {
+		return true;
+	}
+
 	public boolean isMultiTable() {
 		return getTableSpan() > 1;
 	}

hibernate-core/src/test/java/org/hibernate/jpa/test/jointable/Address.java

@@ -0,0 +1,38 @@
+/*
+ * 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.jpa.test.jointable;
+
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+
+/**
+ * @author Christian Beikov
+ */
+@Entity(name="House")
+public class Address {
+	private Long id;
+	private String street;
+
+	@Id
+	@GeneratedValue
+	public Long getId() {
+		return id;
+	}
+
+	public String getStreet() {
+		return street;
+	}
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	public void setStreet(String street) {
+		this.street = street;
+	}
+}

hibernate-core/src/test/java/org/hibernate/jpa/test/jointable/ManyToOneJoinTableTest.java

@@ -0,0 +1,45 @@
+/*
+ * 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.jpa.test.jointable;
+
+import org.hibernate.engine.query.spi.HQLQueryPlan;
+import org.hibernate.hql.spi.QueryTranslator;
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
+
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+
+/**
+ * @author Christian Beikov
+ */
+public class ManyToOneJoinTableTest extends BaseCoreFunctionalTestCase {
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {
+				Person.class,
+				Address.class
+		};
+	}
+
+	@Test
+	public void testAvoidJoin() {
+		final HQLQueryPlan plan = sessionFactory().getQueryPlanCache().getHQLQueryPlan(
+				"SELECT e.id FROM Person e",
+				false,
+				Collections.EMPTY_MAP
+		);
+		assertEquals( 1, plan.getTranslators().length );
+		final QueryTranslator translator = plan.getTranslators()[0];
+		final String generatedSql = translator.getSQLString();
+		// Ideally, we could detect that *ToOne join tables aren't used, but that requires tracking the uses of properties
+		// Since *ToOne join tables are treated like secondary or subclass/superclass tables, the proper fix will allow many more optimizations
+		assertFalse( generatedSql.contains( "join" ) );
+	}
+}

hibernate-core/src/test/java/org/hibernate/jpa/test/jointable/Person.java

@@ -0,0 +1,50 @@
+/*
+ * 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.jpa.test.jointable;
+
+import javax.persistence.*;
+import java.util.Set;
+
+/**
+ *
+ * @author Christian Beikov
+ */
+@Entity
+public class Person {
+	private Long id;
+	private Address address;
+	private Set<Address> addresses;
+
+	@Id
+	public Long getId() {
+		return id;
+	}
+
+	public void setId(Long id) {
+		this.id = id;
+	}
+
+	@ManyToOne
+	@JoinTable( name = "SOME_OTHER_TABLE" )
+	public Address getAddress() {
+		return address;
+	}
+
+	public void setAddress(Address address) {
+		this.address = address;
+	}
+
+	@OneToMany
+	@JoinTable( name = "SOME_OTHER_TABLE2" )
+	public Set<Address> getAddresses() {
+		return addresses;
+	}
+
+	public void setAddresses(Set<Address> addresses) {
+		this.addresses = addresses;
+	}
+}