From 886083ab77f6ac41711c053b5bca097868c9b7c3 Mon Sep 17 00:00:00 2001 From: Nathan Xu Date: Wed, 2 Sep 2020 15:06:27 -0400 Subject: [PATCH] HHH-14201 fix HQL JOIN order issue --- hibernate-core/src/main/antlr/sql-gen.g | 7 +- .../hql/internal/ast/tree/FromClause.java | 30 +++---- .../org/hibernate/query/JoinOrderTest.java | 86 +++++++++++++++++++ 3 files changed, 101 insertions(+), 22 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/query/JoinOrderTest.java diff --git a/hibernate-core/src/main/antlr/sql-gen.g b/hibernate-core/src/main/antlr/sql-gen.g index e2659df245..4f5f57a91e 100644 --- a/hibernate-core/src/main/antlr/sql-gen.g +++ b/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:FROM_FRAGMENT { nestedFromFragment(d,parent); } (tableJoin [ d ] )* ) + : #( d:JOIN_FRAGMENT { out(" "); out(d); } (tableJoin [ d ] )* ) + | #( e:FROM_FRAGMENT { nestedFromFragment(e,parent); } (tableJoin [ e ] )* ) + | #( f:ENTITY_JOIN { out(" "); out(f); } (tableJoin [ f ] )* ) ; booleanOp[ boolean parens ] diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromClause.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromClause.java index 5d854c6b0f..0288159da8 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromClause.java +++ b/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 fromElements = new HashSet(); + private Set fromElements = new LinkedHashSet(); private Map fromElementByClassAlias = new HashMap(); private Map fromElementByTableAlias = new HashMap(); private Map fromElementsByPath = new HashMap(); @@ -61,8 +61,6 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di */ private List impliedElements = new LinkedList(); - private List 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(); - } - entityJoinFromElements.add( (EntityJoinFromElement) element ); - } } public void finishInit() { - if ( entityJoinFromElements == null ) { - return; + // Insert EntityJoinFromElements while maintaining their original position during depth-first traversal. + 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 + "}"; } + } diff --git a/hibernate-core/src/test/java/org/hibernate/query/JoinOrderTest.java b/hibernate-core/src/test/java/org/hibernate/query/JoinOrderTest.java new file mode 100644 index 0000000000..b655d76379 --- /dev/null +++ b/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; + } +}