diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index 8e46b9c1b8..3c9607e2af 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -49,6 +49,7 @@ import org.hibernate.spi.EntityIdentifierNavigablePath; import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstJoinType; +import org.hibernate.sql.ast.internal.TableGroupJoinHelper; import org.hibernate.sql.ast.spi.AliasCollector; import org.hibernate.sql.ast.spi.FromClauseAccess; import org.hibernate.sql.ast.spi.SimpleFromClauseAccessImpl; @@ -711,13 +712,7 @@ private void applyFiltering( final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup ); assert pluralTableGroupJoin != null; - final TableGroupJoin joinForPredicate; - if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { - joinForPredicate = pluralTableGroupJoin; - } - else { - joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); - } + final TableGroupJoin joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( pluralTableGroupJoin ); pluralAttributeMapping.applyBaseRestrictions( joinForPredicate::applyPredicate, diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java index 456ac39e09..0a7aff3a13 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java @@ -49,6 +49,7 @@ import org.hibernate.property.access.spi.PropertyAccess; import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.SqlAstJoinType; +import org.hibernate.sql.ast.internal.TableGroupJoinHelper; import org.hibernate.sql.ast.spi.FromClauseAccess; import org.hibernate.sql.ast.spi.SqlAliasBase; import org.hibernate.sql.ast.spi.SqlAliasStemHelper; @@ -752,13 +753,7 @@ public TableGroupJoin createTableGroupJoin( collectionPredicateCollector.getPredicate() ); if ( predicateCollector != collectionPredicateCollector ) { - final TableGroupJoin joinForPredicate; - if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { - joinForPredicate = tableGroupJoin; - } - else { - joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); - } + final TableGroupJoin joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( tableGroupJoin ); joinForPredicate.applyPredicate( predicateCollector.getPredicate() ); } return tableGroupJoin; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index 304d187a21..cb7558ee35 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -267,6 +267,7 @@ import org.hibernate.sql.ast.SqlAstJoinType; import org.hibernate.sql.ast.SqlTreeCreationException; import org.hibernate.sql.ast.SqlTreeCreationLogger; +import org.hibernate.sql.ast.internal.TableGroupJoinHelper; import org.hibernate.sql.ast.spi.FromClauseAccess; import org.hibernate.sql.ast.spi.SqlAliasBase; import org.hibernate.sql.ast.spi.SqlAliasBaseGenerator; @@ -3364,13 +3365,9 @@ private TableGroup consumeAttributeJoin( } } + // Implicit joins in the predicate might alter the nested table group joins, + // so defer determination of the join for predicate until after the predicate was visited final TableGroupJoin joinForPredicate; - if ( !joinedTableGroup.getNestedTableGroupJoins().isEmpty() || joinedTableGroup.getTableGroupJoins().isEmpty() ) { - joinForPredicate = joinedTableGroupJoin; - } - else { - joinForPredicate = joinedTableGroup.getTableGroupJoins().get( joinedTableGroup.getTableGroupJoins().size() - 1 ); - } // add any additional join restrictions if ( sqmJoin.getJoinPredicate() != null ) { @@ -3381,6 +3378,7 @@ private TableGroup consumeAttributeJoin( final SqmJoin oldJoin = currentlyProcessingJoin; currentlyProcessingJoin = sqmJoin; final Predicate predicate = visitNestedTopLevelPredicate( sqmJoin.getJoinPredicate() ); + joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( joinedTableGroupJoin ); // If translating the join predicate didn't initialize the table group, // we can safely apply it on the collection table group instead if ( joinForPredicate.getJoinedGroup().isInitialized() ) { @@ -3391,6 +3389,9 @@ private TableGroup consumeAttributeJoin( } currentlyProcessingJoin = oldJoin; } + else { + joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( joinedTableGroupJoin ); + } // Since joins on treated paths will never cause table pruning, we need to add a join condition for the treat if ( sqmJoin.getLhs() instanceof SqmTreatedPath ) { final SqmTreatedPath treatedPath = (SqmTreatedPath) sqmJoin.getLhs(); @@ -8312,13 +8313,7 @@ public ImmutableFetchList visitFetches(FetchParent fetchParent) { final TableGroupJoin pluralTableGroupJoin = parentTableGroup.findTableGroupJoin( tableGroup ); assert pluralTableGroupJoin != null; - final TableGroupJoin joinForPredicate; - if ( !tableGroup.getNestedTableGroupJoins().isEmpty() || tableGroup.getTableGroupJoins().isEmpty() ) { - joinForPredicate = pluralTableGroupJoin; - } - else { - joinForPredicate = tableGroup.getTableGroupJoins().get( tableGroup.getTableGroupJoins().size() - 1 ); - } + final TableGroupJoin joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( pluralTableGroupJoin ); joinForPredicate.applyPredicate( predicate ); }, tableGroup, diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/TableGroupJoinHelper.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/TableGroupJoinHelper.java new file mode 100644 index 0000000000..ae08030149 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/internal/TableGroupJoinHelper.java @@ -0,0 +1,46 @@ +/* + * 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.sql.ast.internal; + +import org.hibernate.metamodel.mapping.internal.EmbeddedCollectionPart; +import org.hibernate.sql.ast.tree.from.TableGroup; +import org.hibernate.sql.ast.tree.from.TableGroupJoin; +import org.hibernate.sql.ast.tree.from.VirtualTableGroup; + +public class TableGroupJoinHelper { + + /** + * Determine the {@link TableGroupJoin} to which a custom {@code ON} clause predicate should be applied to. + * This is supposed to be called right after construction of a {@link TableGroupJoin}. + * This should also be called after a {@link org.hibernate.query.sqm.tree.predicate.SqmPredicate} is translated to a + * {@link org.hibernate.sql.ast.tree.predicate.Predicate}, because that translation might cause nested joins to be + * added to the table group of the join. + */ + public static TableGroupJoin determineJoinForPredicateApply(TableGroupJoin mainTableGroupJoin) { + final TableGroup mainTableGroup = mainTableGroupJoin.getJoinedGroup(); + if ( !mainTableGroup.getNestedTableGroupJoins().isEmpty() || mainTableGroup.getTableGroupJoins().isEmpty() ) { + // Always apply a predicate on the main table group join if it has nested table group joins or no joins + return mainTableGroupJoin; + } + else { + // If the main table group has just regular table group joins, + // prefer to apply predicates on the last table group join + final TableGroupJoin lastTableGroupJoin = mainTableGroup.getTableGroupJoins() + .get( mainTableGroup.getTableGroupJoins().size() - 1 ); + if ( lastTableGroupJoin.getJoinedGroup().getModelPart() instanceof EmbeddedCollectionPart ) { + // If the table group join refers to an embedded collection part, + // then the underlying table group *is* the main table group. + // Applying predicates on the join referring to the virtual table group would be a problem though, + // because these predicates will never be rendered. So use the main table group join in that case + assert lastTableGroupJoin.getJoinedGroup() instanceof VirtualTableGroup + && ( (VirtualTableGroup) lastTableGroupJoin.getJoinedGroup() ).getUnderlyingTableGroup() == mainTableGroup; + return mainTableGroupJoin; + } + return lastTableGroupJoin; + } + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java index 4fd3a75b75..9f79f5066e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/JoinTableOptimizationTest.java @@ -11,11 +11,15 @@ import org.hibernate.testing.TestForIssue; import org.hibernate.testing.jdbc.SQLStatementInspector; 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.Assertions; import org.junit.jupiter.api.Test; +import jakarta.persistence.CollectionTable; +import jakarta.persistence.ElementCollection; +import jakarta.persistence.Embeddable; import jakarta.persistence.Entity; import jakarta.persistence.Id; import jakarta.persistence.JoinTable; @@ -123,6 +127,26 @@ public void testLeftJoinCustomOnClause(SessionFactoryScope scope) { ); } + @Test + @JiraKey("HHH-17830") + public void testElementCollectionJoinCustomOnClause(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select p.text from Document d join d.pages p on p.text is not null" ).list(); + statementInspector.assertExecutedCount( 1 ); + Assertions.assertEquals( + "select p1_0.text " + + "from Document d1_0 " + + "join document_pages p1_0 on d1_0.id=p1_0.Document_id and p1_0.text is not null", + statementInspector.getSqlQueries().get( 0 ), + "Join condition was wrongly removed" + ); + } + ); + } + @Entity(name = "Document") public static class Document { @Id @@ -131,6 +155,9 @@ public static class Document { @OneToMany @JoinTable(name = "people") Set people; + @ElementCollection + @CollectionTable(name = "document_pages") + Set pages; } @Entity(name = "Person") public static class Person { @@ -138,4 +165,8 @@ public static class Person { Long id; String name; } + @Embeddable + public static class Page { + String text; + } }