HHH-18932 Use target table columns for SQM joins, except for join table associations

Author: beikov

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java

@@ -918,6 +918,10 @@ public Cardinality getCardinality() {
 		return cardinality;
 	}
 
+	public boolean hasJoinTable() {
+		return hasJoinTable;
+	}
+
 	@Override
 	public EntityMappingType getMappedType() {
 		return getEntityMappingType();

hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java

@@ -38,6 +38,7 @@
 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.DomainType;
 import org.hibernate.metamodel.model.domain.EntityDomainType;
@@ -286,7 +287,13 @@ public static boolean isFkOptimizationAllowed(SqmPath<?> sqmPath) {
 	 * or one that has an explicit on clause predicate.
 	 */
 	public static boolean isFkOptimizationAllowed(SqmPath<?> sqmPath, EntityAssociationMapping associationMapping) {
-		if ( associationMapping.isFkOptimizationAllowed() && sqmPath instanceof 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<?, ?>
+			&& hasJoinTable( associationMapping ) ) {
 			final SqmJoin<?, ?> sqmJoin = (SqmJoin<?, ?>) sqmPath;
 			switch ( sqmJoin.getSqmJoinType() ) {
 				case LEFT:
@@ -304,6 +311,16 @@ public static boolean isFkOptimizationAllowed(SqmPath<?> sqmPath, EntityAssociat
 		return false;
 	}
 
+	private static boolean hasJoinTable(EntityAssociationMapping associationMapping) {
+		if ( associationMapping instanceof CollectionPart ) {
+			return !( (CollectionPart) associationMapping ).getCollectionAttribute().getCollectionDescriptor().isOneToMany();
+		}
+		else if ( associationMapping instanceof ToOneAttributeMapping ) {
+			return ( (ToOneAttributeMapping) associationMapping ).hasJoinTable();
+		}
+		return false;
+	}
+
 	private static boolean isFiltered(EntityAssociationMapping associationMapping) {
 			final EntityMappingType entityMappingType = associationMapping.getAssociatedEntityMappingType();
 			return !associationMapping.isFkOptimizationAllowed()

hibernate-core/src/test/java/org/hibernate/orm/test/hql/ManyToOneJoinReuseTest.java

@@ -78,7 +78,7 @@ public void joinAndImplicitPath(SessionFactoryScope scope) {
 					query.where(
 							cb.and(
 									root.get( "book" ).isNotNull(),
-									join.isNotNull()
+									cb.fk( root.get( "book" ) ).isNotNull()
 							)
 					);
 

hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/InheritanceQueryGroupByTest.java

@@ -152,8 +152,8 @@ private void testGroupByNotSelected(
 					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 );
 		} );
@@ -236,8 +236,8 @@ private void testGroupByAndOrderByNotSelected(
 					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 );
 		} );

hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ql/MapIssueTest.java

@@ -52,13 +52,27 @@ public void testOnlyCollectionTableJoined(SessionFactoryScope scope) {
 	}
 
 	@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 );
 				}