HHH-17706 Optimize FK comparison to eliminate unnecessary left join

Fix regression introduced by ef155c2
hibernate · Feb 4, 2024 · f58d4a0 · f58d4a0
1 parent dc9a997
commit f58d4a0
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 6 deletions.
diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java
@@ -79,6 +79,7 @@
  * Helper utilities for dealing with SQM
  *
  * @author Steve Ebersole
+ * @author Yanming Zhou
  */
 public class SqmUtil {
 	private SqmUtil() {
@@ -130,6 +131,9 @@ public static IllegalQueryOperationException expectingNonSelect(SqmStatement<?>
 	 * dereferenced using the target table mapping, i.e. when the path's lhs is an explicit join.
 	 */
 	public static boolean needsTargetTableMapping(SqmPath<?> sqmPath, ModelPartContainer modelPartContainer) {
+		if ( isFkOptimizationAllowed(sqmPath.getLhs()) ) {
+			return false;
+		}
 		return modelPartContainer.getPartMappingType() != modelPartContainer
 				&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
 				&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType;

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java
@@ -59,8 +59,8 @@ public void testMapKeyJoinIsIncluded(SessionFactoryScope scope) {
 				s -> {
 					s.createQuery( "select c from MapOwner as o join o.contents c join c.relationship r where r.id is not null" ).list();
 					statementInspector.assertExecutedCount( 1 );
-					// Assert 3 joins, collection table, collection element and relationship
-					statementInspector.assertNumberOfJoins( 0, 3 );
+					// Assert 2 joins, collection table, collection element
+					statementInspector.assertNumberOfJoins( 0, 2 );
 				}
 		);
 	}

diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/manytoone/ManyToOneTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/manytoone/ManyToOneTest.java
@@ -9,14 +9,16 @@
 import jakarta.persistence.Entity;
 import jakarta.persistence.FetchType;
 import jakarta.persistence.Id;
-import jakarta.persistence.JoinColumn;
-import jakarta.persistence.JoinTable;
 import jakarta.persistence.ManyToOne;
 import jakarta.persistence.Table;
+import jakarta.persistence.criteria.CriteriaBuilder;
+import jakarta.persistence.criteria.CriteriaQuery;
+import jakarta.persistence.criteria.JoinType;
+import jakarta.persistence.criteria.Root;
 
 import org.hibernate.Hibernate;
 import org.hibernate.stat.spi.StatisticsImplementor;
-
+import org.hibernate.testing.jdbc.SQLStatementInspector;
 import org.hibernate.testing.orm.junit.DomainModel;
 import org.hibernate.testing.orm.junit.ServiceRegistry;
 import org.hibernate.testing.orm.junit.SessionFactory;
@@ -34,6 +36,7 @@
 
 /**
  * @author Andrea Boriero
+ * @author Yanming Zhou
  */
 @DomainModel(
 		annotatedClasses = {
@@ -304,6 +307,23 @@ public void testDelete(SessionFactoryScope scope) {
 		);
 	}
 
+	@Test
+	public void testFkOptimizationWithLeftJoin(SessionFactoryScope scope) {
+		SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
+		statementInspector.clear();
+		scope.inTransaction(
+				session -> {
+					CriteriaBuilder cb = session.getCriteriaBuilder();
+					CriteriaQuery<OtherEntity> cq = cb.createQuery( OtherEntity.class );
+					Root<OtherEntity> root = cq.from( OtherEntity.class );
+					cq.select(root).where( cb.equal( root.join("simpleEntity", JoinType.LEFT ).get( "id" ), 1 ) );
+					assertThat( session.createQuery( cq ).getResultList().size(), is(1) );
+					statementInspector.assertExecutedCount( 1 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 0 );
+				}
+		);
+	}
+
 	@BeforeEach
 	public void setUp(SessionFactoryScope scope) {
 		scope.inTransaction(

diff --git a/...onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java b/...onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java
@@ -153,7 +153,7 @@ public void testHqlSelectSon(SessionFactoryScope scope) {
 
 					statementInspector.assertExecutedCount( 2 );
 					// The join to the target table PARENT for Male#parent is added since it's explicitly joined in HQL
-					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 2 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 );
 					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 3 );
 					assertThat( son.getParent(), CoreMatchers.notNullValue() );