diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java index 3d6628cea3..6777300dc3 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java @@ -914,6 +914,10 @@ public class ToOneAttributeMapping return cardinality; } + public boolean hasJoinTable() { + return hasJoinTable; + } + @Override public EntityMappingType getMappedType() { return getEntityMappingType(); diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java index 3ca7677fae..6892b2d74a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java @@ -34,6 +34,7 @@ import org.hibernate.metamodel.mapping.MappingModelExpressible; import org.hibernate.metamodel.mapping.ModelPart; import org.hibernate.metamodel.mapping.ModelPartContainer; import org.hibernate.metamodel.mapping.PluralAttributeMapping; +import org.hibernate.metamodel.mapping.internal.ToOneAttributeMapping; import org.hibernate.metamodel.model.domain.BasicDomainType; import org.hibernate.metamodel.model.domain.EntityDomainType; import org.hibernate.metamodel.model.domain.IdentifiableDomainType; @@ -257,7 +258,13 @@ public class SqmUtil { * or one that has an explicit on clause predicate. */ public static boolean isFkOptimizationAllowed(SqmPath sqmPath, EntityAssociationMapping associationMapping) { - if ( associationMapping.isFkOptimizationAllowed() && sqmPath instanceof SqmJoin sqmJoin ) { + // By default, never allow the FK optimization if the path is a join, unless the association has a join table + // Hibernate ORM has no way for users to refer to collection/join table rows, + // so referring the columns of these rows by default when requesting FK column attributes is sensible. + // Users that need to refer to the actual target table columns will have to add an explicit entity join. + if ( associationMapping.isFkOptimizationAllowed() + && sqmPath instanceof SqmJoin sqmJoin + && hasJoinTable( associationMapping ) ) { switch ( sqmJoin.getSqmJoinType() ) { case LEFT: if ( isFiltered( associationMapping ) ) { @@ -273,6 +280,16 @@ public class SqmUtil { return false; } + private static boolean hasJoinTable(EntityAssociationMapping associationMapping) { + if ( associationMapping instanceof CollectionPart collectionPart ) { + return !collectionPart.getCollectionAttribute().getCollectionDescriptor().isOneToMany(); + } + else if ( associationMapping instanceof ToOneAttributeMapping toOneAttributeMapping ) { + return toOneAttributeMapping.hasJoinTable(); + } + return false; + } + private static boolean isFiltered(EntityAssociationMapping associationMapping) { final EntityMappingType entityMappingType = associationMapping.getAssociatedEntityMappingType(); return !associationMapping.isFkOptimizationAllowed() diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ManyToOneJoinReuseTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ManyToOneJoinReuseTest.java index dfe02c70c9..80a351af4b 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ManyToOneJoinReuseTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ManyToOneJoinReuseTest.java @@ -75,7 +75,7 @@ public class ManyToOneJoinReuseTest { query.where( cb.and( root.get( "book" ).isNotNull(), - join.isNotNull() + cb.fk( root.get( "book" ) ).isNotNull() ) ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/InheritanceQueryGroupByTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/InheritanceQueryGroupByTest.java index 85c0d0b2eb..1beaedcb4a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/InheritanceQueryGroupByTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/InheritanceQueryGroupByTest.java @@ -150,8 +150,8 @@ public class InheritanceQueryGroupByTest { Long.class ).getSingleResult(); assertThat( sum ).isEqualTo( 3L ); - // When not selected, group by should only use the foreign key (parent_id) - statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, parentFkName, 2 ); + // Association is joined, so every use of the join alias will make use of target table columns + statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, parentFkName, 1 ); statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "child_one_col", childPropCount ); statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "child_two_col", childPropCount ); } ); @@ -234,8 +234,8 @@ public class InheritanceQueryGroupByTest { Long.class ).getSingleResult(); assertThat( sum ).isEqualTo( 3L ); - // When not selected, group by should only use the foreign key (parent_id) - statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, parentFkName, 3 ); + // Association is joined, so every use of the join alias will make use of target table columns + statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, parentFkName, 1 ); statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "child_one_col", childPropCount ); statementInspector.assertNumberOfOccurrenceInQueryNoSpace( 0, "child_two_col", childPropCount ); } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java index 7d6a0811f9..a197550ce6 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java @@ -50,13 +50,27 @@ public class MapIssueTest { } @Test - public void testMapKeyJoinIsOmitted(SessionFactoryScope scope) { + public void testMapKeyJoinIsNotOmitted(SessionFactoryScope scope) { SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); statementInspector.clear(); scope.inTransaction( s -> { s.createQuery( "select c from MapOwner as o join o.contents c join c.relationship r where r.id is not null" ).list(); statementInspector.assertExecutedCount( 1 ); + // Assert 3 joins, collection table, collection element and collection key (relationship) + statementInspector.assertNumberOfJoins( 0, 3 ); + } + ); + } + + @Test + public void testMapKeyJoinIsOmitted2(SessionFactoryScope scope) { + SQLStatementInspector statementInspector = scope.getCollectingStatementInspector(); + statementInspector.clear(); + scope.inTransaction( + s -> { + s.createQuery( "select c from MapOwner as o join o.contents c where c.relationship.id is not null" ).list(); + statementInspector.assertExecutedCount( 1 ); // Assert 2 joins, collection table and collection element. No need to join the relationship because it is not nullable statementInspector.assertNumberOfJoins( 0, 2 ); }