HHH-17033 Fix invalid SQL being generated for implicit join in entity join on clause

hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java

@@ -3409,6 +3409,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				tableGroup,
 				predicate.get()
 		);
+		lhsTableGroup.addTableGroupJoin( tableGroupJoin );
 
 		if ( sqmJoin.getJoinPredicate() != null ) {
 			final SqmJoin<?, ?> oldJoin = currentlyProcessingJoin;
@@ -3423,9 +3424,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 			);
 		}
 
-		// Note that we add the entity join after processing the predicate because implicit joins needed in there
-		// can be just ordered right before the entity join without changing the semantics
-		lhsTableGroup.addTableGroupJoin( tableGroupJoin );
 		if ( transitive ) {
 			consumeExplicitJoins( sqmJoin, tableGroupJoin.getJoinedGroup() );
 		}
@@ -3465,6 +3463,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				queryPartTableGroup,
 				null
 		);
+		parentTableGroup.addTableGroupJoin( tableGroupJoin );
 
 		// add any additional join restrictions
 		if ( sqmJoin.getJoinPredicate() != null ) {
@@ -3474,9 +3473,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 			currentlyProcessingJoin = oldJoin;
 		}
 
-		// Note that we add the entity join after processing the predicate because implicit joins needed in there
-		// can be just ordered right before the entity join without changing the semantics
-		parentTableGroup.addTableGroupJoin( tableGroupJoin );
 		if ( transitive ) {
 			consumeExplicitJoins( sqmJoin, queryPartTableGroup );
 		}
@@ -3499,6 +3495,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				tableGroup,
 				null
 		);
+		parentTableGroup.addTableGroupJoin( tableGroupJoin );
 
 		// add any additional join restrictions
 		if ( sqmJoin.getJoinPredicate() != null ) {
@@ -3508,9 +3505,6 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 			currentlyProcessingJoin = oldJoin;
 		}
 
-		// Note that we add the entity join after processing the predicate because implicit joins needed in there
-		// can be just ordered right before the entity join without changing the semantics
-		parentTableGroup.addTableGroupJoin( tableGroupJoin );
 		if ( transitive ) {
 			consumeExplicitJoins( sqmJoin, tableGroup );
 		}
@@ -3760,12 +3754,8 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 							false,
 							this
 					);
-					// Implicit joins in the ON clause of attribute joins need to be added as nested table group joins
+					// Implicit joins in the ON clause need to be added as nested table group joins
-					// We don't have to do that for entity joins etc. as these do not have an inherent dependency on the lhs.
+					final boolean nested = currentClauseStack.getCurrent() == Clause.FROM;
-					// We can just add the implicit join before the currently processing join
-					// See consumeEntityJoin for details
-					final boolean nested = currentClauseStack.getCurrent() == Clause.FROM
-							&& currentlyProcessingJoin instanceof SqmAttributeJoin<?, ?>;
 					if ( nested ) {
 						parentTableGroup.addNestedTableGroupJoin( tableGroupJoin );
 					}

hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroup.java

@@ -191,6 +191,11 @@ public interface TableGroup extends SqlAstNode, ColumnReferenceQualifier, SqmPat
 				return join;
 			}
 		}
+		for ( TableGroupJoin join : getNestedTableGroupJoins() ) {
+			if ( join.getJoinedGroup() == tableGroup ) {
+				return join;
+			}
+		}
 		return null;
 	}
 

hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/EntityJoinTest.java

@@ -208,14 +208,15 @@ public class EntityJoinTest {
                     assertThat( roots.size(), is( 1 ) );
 
                     final TableGroup rootTableGroup = roots.get( 0 );
-                    assertThat( rootTableGroup.getTableGroupJoins().size(), is( 2 ) );
+                    assertThat( rootTableGroup.getTableGroupJoins().size(), is( 1 ) );
+                    assertThat( rootTableGroup.getNestedTableGroupJoins().size(), is( 1 ) );
 
-                    // The first table group is an uninitialized lazy table group for the path in the on clause
+                    // An uninitialized lazy table group for the path in the on clause
-                    final TableGroupJoin firstTableGroupJoin = rootTableGroup.getTableGroupJoins().get( 0 );
+                    final TableGroupJoin nestedTableGroupJoin = rootTableGroup.getNestedTableGroupJoins().get( 0 );
-                    assertThat( firstTableGroupJoin.getJoinedGroup(), instanceOf( LazyTableGroup.class ) );
+                    assertThat( nestedTableGroupJoin.getJoinedGroup(), instanceOf( LazyTableGroup.class ) );
-                    assertThat( ((LazyTableGroup) firstTableGroupJoin.getJoinedGroup()).getUnderlyingTableGroup(), is( CoreMatchers.nullValue() ) );
+                    assertThat( ((LazyTableGroup) nestedTableGroupJoin.getJoinedGroup()).getUnderlyingTableGroup(), is( CoreMatchers.nullValue() ) );
 
-                    final TableGroupJoin tableGroupJoin = rootTableGroup.getTableGroupJoins().get( 1 );
+                    final TableGroupJoin tableGroupJoin = rootTableGroup.getTableGroupJoins().get( 0 );
                     assertThat( tableGroupJoin.getJoinedGroup().getModelPart(), is( customerEntityDescriptor ) );
                 }
         );

hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ImplicitJoinInOnClauseTest.java

@@ -0,0 +1,102 @@
+/*
+ * 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.query.hql;
+
+import java.util.List;
+
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.JiraKey;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.ManyToOne;
+
+/**
+ * @author Christian Beikov
+ */
+@DomainModel( annotatedClasses = {
+        ImplicitJoinInOnClauseTest.RootEntity.class,
+        ImplicitJoinInOnClauseTest.FirstLevelReferencedEntity.class,
+        ImplicitJoinInOnClauseTest.SecondLevelReferencedEntityA.class,
+        ImplicitJoinInOnClauseTest.SecondLevelReferencedEntityB.class,
+        ImplicitJoinInOnClauseTest.ThirdLevelReferencedEntity.class,
+        ImplicitJoinInOnClauseTest.UnrelatedEntity.class
+})
+@SessionFactory
+@JiraKey( "HHH-17033" )
+public class ImplicitJoinInOnClauseTest {
+
+    @Test
+    public void testImplicitJoinInEntityJoinPredicate(SessionFactoryScope scope) {
+        scope.inTransaction(
+                (session) -> {
+                        // this should get financial records which have a lastUpdateBy user set
+                        List<Object[]> result = session.createQuery(
+                                "select r.id, flr.id, ur1.id, ur2.id, ur3.id from RootEntity r " +
+                                        "inner join r.firstLevelReference as flr " +
+                                        "left join UnrelatedEntity ur1 on ur1.id = flr.secondLevelReferenceA.id " +
+                                        "left join UnrelatedEntity ur2 on ur2.id = flr.secondLevelReferenceB.id " +
+                                        "left join UnrelatedEntity ur3 on ur3.id = flr.secondLevelReferenceB.thirdLevelReference.id",
+                                Object[].class
+                        ).list();
+                }
+        );
+    }
+
+
+    @Entity(name = "RootEntity")
+    public static class RootEntity {
+        @Id
+        private Long id;
+        @ManyToOne
+        private FirstLevelReferencedEntity firstLevelReference;
+
+    }
+
+    @Entity(name = "FirstLevelReferencedEntity")
+    public static class FirstLevelReferencedEntity {
+        @Id
+        private Long id;
+        @ManyToOne
+        private SecondLevelReferencedEntityA secondLevelReferenceA;
+        @ManyToOne
+        private SecondLevelReferencedEntityB secondLevelReferenceB;
+
+    }
+    @Entity(name = "SecondLevelReferencedEntityA")
+    public static class SecondLevelReferencedEntityA {
+        @Id
+        private Long id;
+        private String name;
+    }
+
+    @Entity(name = "SecondLevelReferencedEntityB")
+    public static class SecondLevelReferencedEntityB {
+        @Id
+        private Long id;
+        @ManyToOne
+        private ThirdLevelReferencedEntity thirdLevelReference;
+    }
+
+    @Entity(name = "ThirdLevelReferencedEntity")
+    public static class ThirdLevelReferencedEntity {
+        @Id
+        private Long id;
+        private String name;
+
+    }
+    @Entity(name = "UnrelatedEntity")
+    public static class UnrelatedEntity {
+        @Id
+        private Long id;
+        private String name;
+    }
+
+}