From 3d6f8eb0ff11adba1018c55fc978aa10ab297fdc Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 1 Feb 2017 13:39:04 +0100 Subject: [PATCH] HHH-11437 - Entity joins are not polymorphic --- .../engine/internal/JoinSequence.java | 6 +- .../ast/tree/EntityJoinFromElement.java | 80 ++++++++++++++++--- .../hibernate/test/hql/JoinOnClauseTest.java | 2 +- 3 files changed, 74 insertions(+), 14 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java index 50c93b9de7..c27af02ec6 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/JoinSequence.java @@ -89,6 +89,10 @@ public class JoinSequence { this.treatAsDeclarations.addAll( treatAsDeclarations ); } + protected Set getTreatAsDeclarations() { + return treatAsDeclarations; + } + /** * Create a full, although shallow, copy. @@ -436,7 +440,7 @@ public class JoinSequence { ); } - private boolean isIncluded(String alias) { + protected boolean isIncluded(String alias) { return selector != null && selector.includeSubclasses( alias ); } diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/EntityJoinFromElement.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/EntityJoinFromElement.java index d90bc916e9..3f09232112 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/EntityJoinFromElement.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/EntityJoinFromElement.java @@ -8,6 +8,7 @@ package org.hibernate.hql.internal.ast.tree; import java.util.Collections; import java.util.Map; +import java.util.Set; import org.hibernate.AssertionFailure; import org.hibernate.MappingException; @@ -16,6 +17,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes; import org.hibernate.hql.internal.ast.HqlSqlWalker; import org.hibernate.internal.util.StringHelper; +import org.hibernate.persister.entity.AbstractEntityPersister; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.persister.entity.Joinable; import org.hibernate.persister.entity.Queryable; @@ -126,11 +128,34 @@ public class EntityJoinFromElement extends FromElement { } final StringBuilder buffer = new StringBuilder(); - buffer.append( joinString ) - .append( entityTableText ) + final AbstractEntityPersister joinable = (AbstractEntityPersister) entityType.getAssociatedJoinable(factory); + + buffer.append( joinString ); + + Set treatAsDeclarations = getTreatAsDeclarations(); + final boolean include = includeAllSubclassJoins && isIncluded( entityTableAlias ); + String fromFragment = joinable.fromJoinFragment( entityTableAlias, true, include, treatAsDeclarations ); + String whereFragment = joinable.whereJoinFragment( entityTableAlias, true, include, treatAsDeclarations ); + + // We need a table group only when having e.g. a left join of a polymorphic entity + // fromFragment is empty if the entity is non-polymorphic + // Inner joined entity joins can avoid using the table grouping since the condition can be moved to the where clause + boolean renderTableGroup = !fromFragment.isEmpty() && joinType != JoinType.INNER_JOIN; + + if ( renderTableGroup ) { + buffer.append( '(' ); + } + + buffer.append( entityTableText ) .append( ' ' ) - .append( entityTableAlias ) - .append( " on " ); + .append( entityTableAlias ); + + if ( renderTableGroup ) { + buffer.append( fromFragment ) + .append( ')' ); + } + + buffer.append( " on " ); final String filters = entityType.getOnCondition( entityTableAlias, @@ -139,23 +164,54 @@ public class EntityJoinFromElement extends FromElement { Collections.emptySet() ); - buffer.append( filters ); - if ( withClauseFragment != null ) { - if ( StringHelper.isNotEmpty( filters ) ) { - buffer.append( " and " ); + if ( fromFragment.isEmpty() || renderTableGroup ) { + buffer.append( filters ); + if ( withClauseFragment != null ) { + if ( StringHelper.isNotEmpty( filters ) ) { + buffer.append( " and " ); + } + buffer.append( withClauseFragment ); } - buffer.append( withClauseFragment ); + } + else { + // We know there is a fromFragment and that we shouldn't render a table group + // This means the entity is polymorphic and the entity join is an inner join + // We move the with clause stuff to the where clause but still need to have a valid on condition + buffer.append( "1=1" ); + buffer.append( fromFragment ); + + // Proper capacity to avoid resizing + StringBuilder whereBuffer = new StringBuilder( + 10 + + whereFragment.length() + + filters.length() + + withClauseFragment.length() + ); + whereBuffer.append(whereFragment); + if ( !filters.isEmpty() ) { + whereBuffer.append( " and " ); + whereBuffer.append( filters ); + } + if ( !withClauseFragment.isEmpty() ) { + whereBuffer.append( " and " ); + whereBuffer.append( withClauseFragment ); + } + + whereFragment = whereBuffer.toString(); } - return new EntityJoinJoinFragment( buffer.toString() ); + return new EntityJoinJoinFragment( buffer.toString(), whereFragment ); } + } private static class EntityJoinJoinFragment extends JoinFragment { private final String fragmentString; + private final String whereFragment; - public EntityJoinJoinFragment(String fragmentString) { + public EntityJoinJoinFragment(String fragmentString, String whereFragment) { this.fragmentString = fragmentString; + this.whereFragment = whereFragment; } @Override @@ -192,7 +248,7 @@ public class EntityJoinFromElement extends FromElement { @Override public String toWhereFragmentString() { - return null; + return whereFragment; } @Override diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/JoinOnClauseTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/JoinOnClauseTest.java index fa219f93b1..fbdb537fd3 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/hql/JoinOnClauseTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/hql/JoinOnClauseTest.java @@ -60,7 +60,7 @@ public class JoinOnClauseTest extends BaseEntityManagerFunctionalTestCase { @Test @TestForIssue(jiraKey = "HHH-11437") - public void testOnClauseUsesNonDrivingTableAlias() { + public void testOnClauseUsesSuperclassAttribute() { doInJPA( this::entityManagerFactory, entityManager -> { List result = entityManager.createQuery( "SELECT DISTINCT b1 FROM Book b1 JOIN Book b2 ON b1.price = b2.price", Book.class ) .getResultList();