HHH-14201 fix HQL JOIN order issue

hibernate-core/src/main/antlr/sql-gen.g

@@ -314,12 +314,13 @@ fromTable
 	// Write the table node (from fragment) and all the join fragments associated with it.
 	: #( a:FROM_FRAGMENT  { out(a); } (tableJoin [ a ])* { fromFragmentSeparator(a); } )
 	| #( b:JOIN_FRAGMENT  { out(b); } (tableJoin [ b ])* { fromFragmentSeparator(b); } )
-	| #( e:ENTITY_JOIN    { out(e); } (tableJoin [ e ])* { fromFragmentSeparator(e); } )
+	| #( c:ENTITY_JOIN    { out(c); } (tableJoin [ c ])* { fromFragmentSeparator(c); } )
 	;
 
 tableJoin [ AST parent ]
-	: #( c:JOIN_FRAGMENT { out(" "); out(c); } (tableJoin [ c ] )* )
+	: #( d:JOIN_FRAGMENT { out(" "); out(d); } (tableJoin [ d ] )* )
-	| #( d:FROM_FRAGMENT { nestedFromFragment(d,parent); } (tableJoin [ d ] )* )
+	| #( e:FROM_FRAGMENT { nestedFromFragment(e,parent); } (tableJoin [ e ] )* )
+	| #( f:ENTITY_JOIN   { out(" "); out(f); } (tableJoin [ f ] )* )
 	;
 
 booleanOp[ boolean parens ]

hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromClause.java

@@ -6,9 +6,9 @@
  */
 package org.hibernate.hql.internal.ast.tree;
 
-import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -34,7 +34,7 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
 	public static final int ROOT_LEVEL = 1;
 
 	private int level = ROOT_LEVEL;
-	private Set<FromElement> fromElements = new HashSet<FromElement>();
+	private Set<FromElement> fromElements = new LinkedHashSet<FromElement>();
 	private Map<String,FromElement> fromElementByClassAlias = new HashMap<String,FromElement>();
 	private Map<String,FromElement> fromElementByTableAlias = new HashMap<String,FromElement>();
 	private Map<String,FromElement> fromElementsByPath = new HashMap<String,FromElement>();
@@ -61,8 +61,6 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
 	 */
 	private List impliedElements = new LinkedList();
 
-	private List<EntityJoinFromElement> entityJoinFromElements;
-
 	/**
 	 * Adds a new from element to the from node.
 	 *
@@ -91,25 +89,18 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
 		if ( tableAlias != null ) {
 			fromElementByTableAlias.put( tableAlias, element );
 		}
-
-		if ( element instanceof EntityJoinFromElement ) {
-			if ( entityJoinFromElements == null ) {
-				entityJoinFromElements = new ArrayList<EntityJoinFromElement>();
-			}
-			entityJoinFromElements.add( (EntityJoinFromElement) element );
-		}
 	}
 
 	public void finishInit() {
-		if ( entityJoinFromElements == null ) {
+		// Insert EntityJoinFromElements while maintaining their original position during depth-first traversal.
-			return;
+		FromElement lastFromElement = null;
+		for ( FromElement fromElement : fromElements ) {
+			if ( fromElement instanceof EntityJoinFromElement ) {
+				assert lastFromElement != null;
+				ASTUtil.insertChild( lastFromElement, fromElement );
+			}
+			lastFromElement = fromElement;
 		}
-
-		for ( EntityJoinFromElement entityJoinFromElement : entityJoinFromElements ) {
-			ASTUtil.appendChild( this, entityJoinFromElement );
-		}
-
-		entityJoinFromElements.clear();
 	}
 
 	void addDuplicateAlias(String alias, FromElement element) {
@@ -412,4 +403,5 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
 	public String toString() {
 		return "FromClause{level=" + level + "}";
 	}
+
 }

hibernate-core/src/test/java/org/hibernate/query/JoinOrderTest.java

@@ -0,0 +1,86 @@
+package org.hibernate.query;
+
+import java.util.Map;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.ManyToOne;
+
+import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.jdbc.SQLStatementInterceptor;
+import org.junit.Test;
+
+import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Christian Beikov
+ * @author Nathan Xu
+ */
+@TestForIssue( jiraKey = "HHH-14201" )
+public class JoinOrderTest extends BaseEntityManagerFunctionalTestCase {
+
+	private SQLStatementInterceptor sqlStatementInterceptor;
+
+	@Override
+	protected void addConfigOptions(Map options) {
+		sqlStatementInterceptor = new SQLStatementInterceptor( options );
+	}
+
+	@Override
+	public Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] {
+				EntityA.class,
+				EntityB.class,
+				EntityC.class
+		};
+	}
+
+	@Test
+	public void testJoinOrder() {
+		doInJPA( this::entityManagerFactory, entityManager -> {
+
+			sqlStatementInterceptor.clear();
+
+			final String hql =
+					"SELECT 1 " +
+					"FROM EntityA a " +
+					"JOIN EntityB b ON b.a = a " +
+					"JOIN a.c c ON c.b = b";
+			entityManager.createQuery( hql ).getResultList();
+
+			sqlStatementInterceptor.assertExecutedCount( 1 );
+			final String sql = sqlStatementInterceptor.getSqlQueries().getFirst();
+
+			assertTrue( sql.matches( "^.+(?: join EntityB ).+(?: join EntityC ).+$" ) );
+		} );
+	}
+
+	@Entity(name = "EntityA")
+	public static class EntityA {
+		@Id
+		int id;
+
+		@ManyToOne
+		EntityC c;
+	}
+
+	@Entity(name = "EntityB")
+	public static class EntityB {
+		@Id
+		int id;
+
+		@ManyToOne
+		EntityA a;
+	}
+
+	@Entity(name = "EntityC")
+	public static class EntityC {
+		@Id
+		int id;
+
+		@ManyToOne
+		EntityB b;
+	}
+}