From 6dc2a4de133865a2fb9b32c45810c9521b844f5c Mon Sep 17 00:00:00 2001 From: Gavin King Date: Fri, 20 Jun 2025 21:46:23 +0200 Subject: [PATCH 1/7] HHH-19565 test to reproduce bug --- .../annotations/ManyToOneRestrictionTest.java | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java new file mode 100644 index 000000000000..9557d5a9a957 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java @@ -0,0 +1,68 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.where.annotations; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import org.hibernate.annotations.SQLRestriction; +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.Jpa; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.AssertionsKt.assertNull; + +@Jpa(annotatedClasses = {ManyToOneRestrictionTest.X.class, ManyToOneRestrictionTest.Y.class}) +class ManyToOneRestrictionTest { + @Test void test(EntityManagerFactoryScope scope) { + scope.inTransaction(em -> { + Y y = new Y(); + X x = new X(); + x.id = -1; + y.x = x; + em.persist(x); + em.persist(y); + }); + scope.inTransaction(em -> { + Y y = em.find(Y.class, 0L); + assertNull(y.x); + var fk = + em.createNativeQuery( "select xx from YY", long.class ) + .getSingleResultOrNull(); + assertNotNull(fk); + y.name = "hello"; + }); + scope.inTransaction(em -> { + Y y = em.find(Y.class, 0L); + assertNull(y.x); + var fk = + em.createNativeQuery( "select xx from YY", long.class ) + .getSingleResultOrNull(); + assertNotNull(fk); + }); + + } + + @Entity + @Table(name = "XX") + @SQLRestriction("id>0") + static class X { + @Id + long id; + } + @Entity + @Table(name = "YY") + static class Y { + @Id + long id; + String name; + @ManyToOne + @JoinColumn(name = "xx") + X x; + } +} From 8892e3879e6ab8fb177b354b3c34bfc0a69b5ef2 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 13:44:01 +0200 Subject: [PATCH 2/7] HHH-19565 stricter test for fix to bug --- .../annotations/ManyToOneRestrictionTest.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java index 9557d5a9a957..76eb088b2efb 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneRestrictionTest.java @@ -11,14 +11,16 @@ import jakarta.persistence.Table; import org.hibernate.annotations.SQLRestriction; import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.Jpa; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.AssertionsKt.assertNull; @Jpa(annotatedClasses = {ManyToOneRestrictionTest.X.class, ManyToOneRestrictionTest.Y.class}) class ManyToOneRestrictionTest { + @JiraKey("HHH-19565") @Test void test(EntityManagerFactoryScope scope) { scope.inTransaction(em -> { Y y = new Y(); @@ -28,24 +30,22 @@ class ManyToOneRestrictionTest { em.persist(x); em.persist(y); }); + // @SQLRestrictions should not be applied to + // foreign key associations, or the FK will + // be set to null when the entity is updated, + // leading to data loss scope.inTransaction(em -> { Y y = em.find(Y.class, 0L); - assertNull(y.x); - var fk = - em.createNativeQuery( "select xx from YY", long.class ) - .getSingleResultOrNull(); - assertNotNull(fk); + assertNotNull(y.x); + assertEquals(-1, y.x.id); y.name = "hello"; }); scope.inTransaction(em -> { Y y = em.find(Y.class, 0L); - assertNull(y.x); - var fk = - em.createNativeQuery( "select xx from YY", long.class ) - .getSingleResultOrNull(); - assertNotNull(fk); + assertNotNull(y.x); + assertEquals(-1, y.x.id); + assertEquals("hello", y.name); }); - } @Entity From d570c369a428a1180ecf32ef2ebd42fd25e27c9b Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 14:22:55 +0200 Subject: [PATCH 3/7] HHH-19565 @SQLRestriction should not be applied to a @ManyToOne At least not when mapped to a foreign key association. If we apply such a restriction, we will set the FK to null when the entity is updated, resulting in data loss. Note that this was the historical and correct behavior right up until Hibernate 6.2. Some recent tests are asserting a different and dangerous behavior, so I'm changing those tests. --- .../internal/ToOneAttributeMapping.java | 32 ++++++++++++------- ...inWithRestrictedJoinedInheritanceTest.java | 3 +- ...ereAnnotationAndJoinedInheritanceTest.java | 6 ++-- 3 files changed, 26 insertions(+), 15 deletions(-) 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 fe964e02f7cc..92269d2280ae 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 @@ -2091,10 +2091,12 @@ public TableGroupJoin createTableGroupJoin( creationState ) ); + final EntityMappingType associatedEntityMappingType = getAssociatedEntityMappingType(); + // Note specifically we only apply `@Filter` restrictions that are applyToLoadByKey = true // to make the behavior consistent with lazy loading of an association - if ( getAssociatedEntityMappingType().getEntityPersister().hasFilterForLoadByKey() ) { - getAssociatedEntityMappingType().applyBaseRestrictions( + if ( associatedEntityMappingType.getEntityPersister().hasFilterForLoadByKey() ) { + associatedEntityMappingType.applyBaseRestrictions( join::applyPredicate, tableGroup, true, @@ -2104,22 +2106,28 @@ public TableGroupJoin createTableGroupJoin( creationState ); } - getAssociatedEntityMappingType().applyWhereRestrictions( - join::applyPredicate, - tableGroup, - true, - creationState - ); - if ( getAssociatedEntityMappingType().getSuperMappingType() != null && !creationState.supportsEntityNameUsage() ) { - getAssociatedEntityMappingType().applyDiscriminator( null, null, tableGroup, creationState ); + // @SQLRestriction should not be applied when joining FK association, + // because that would result in us setting the FK to null when the + // owning entity is updated, that is, to data loss. + // But we'll let it apply on the TARGET side of a @OneToOne + if ( sideNature == ForeignKeyDescriptor.Nature.TARGET ) { + associatedEntityMappingType.applyWhereRestrictions( + join::applyPredicate, + tableGroup, + true, + creationState + ); + } + if ( associatedEntityMappingType.getSuperMappingType() != null && !creationState.supportsEntityNameUsage() ) { + associatedEntityMappingType.applyDiscriminator( null, null, tableGroup, creationState ); } - final SoftDeleteMapping softDeleteMapping = getAssociatedEntityMappingType().getSoftDeleteMapping(); + final SoftDeleteMapping softDeleteMapping = associatedEntityMappingType.getSoftDeleteMapping(); if ( softDeleteMapping != null ) { // add the restriction final TableReference tableReference = lazyTableGroup.resolveTableReference( navigablePath, - getAssociatedEntityMappingType().getSoftDeleteTableDetails().getTableName() + associatedEntityMappingType.getSoftDeleteTableDetails().getTableName() ); join.applyPredicate( softDeleteMapping.createNonDeletedRestriction( tableReference, diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/AttributeJoinWithRestrictedJoinedInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/AttributeJoinWithRestrictedJoinedInheritanceTest.java index 3a06d52a8924..218e1f5618ac 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/AttributeJoinWithRestrictedJoinedInheritanceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/inheritance/join/AttributeJoinWithRestrictedJoinedInheritanceTest.java @@ -66,7 +66,8 @@ public void testLeftJoinWithDiscriminatorFiltering(SessionFactoryScope scope) { ).getResultList(); assertEquals( 2, resultList.size() ); assertResult( resultList.get( 0 ), 1, 11, 11, "child_a_1", SubChildEntityA1.class ); - assertResult( resultList.get( 1 ), 2, 21, null, null, null ); + // @SQLRestriction should not be applied when joining FK association + assertResult( resultList.get( 1 ), 2, 21, 21, "child_a_2", SubChildEntityA2.class ); } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/where/WhereAnnotationAndJoinedInheritanceTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/where/WhereAnnotationAndJoinedInheritanceTest.java index 1b72d517a3d8..f60c6186f2a1 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/where/WhereAnnotationAndJoinedInheritanceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/where/WhereAnnotationAndJoinedInheritanceTest.java @@ -79,7 +79,8 @@ public void testCriteriaQuery(EntityManagerFactoryScope scope) { // the child List resultList = entityManager.createQuery( query ).getResultList(); - assertThat( resultList.size() ).isEqualTo( 0 ); + // @SQLRestriction should not be applied when joining FK association + assertThat( resultList.size() ).isEqualTo( 1 ); } ); } @@ -124,7 +125,8 @@ public void testCriteriaQuery2(EntityManagerFactoryScope scope) { root.get( "child" ).get( "data" ), DELETED_CHILD ) ); List resultList = entityManager.createQuery( query ).getResultList(); - assertThat( resultList.size() ).isEqualTo( 0 ); + // @SQLRestriction should not be applied when joining FK association + assertThat( resultList.size() ).isEqualTo( 1 ); builder = entityManager.getCriteriaBuilder(); query = builder.createQuery( PrimaryObject.class ); From 201a03316dbe1529e4276d13d5638e4efb8fff34 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 15:10:05 +0200 Subject: [PATCH 4/7] HHH-19565 restore application of @SQLRestriction to @ManyToOne @JoinTable In this case, we won't set the FK to null. Instead, we can handle it with an upsert (SQL MERGE). --- .../boot/model/internal/ToOneBinder.java | 5 ++- .../java/org/hibernate/mapping/ManyToOne.java | 9 +++++ .../internal/ToOneAttributeMapping.java | 40 +++++++++---------- 3 files changed, 31 insertions(+), 23 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ToOneBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ToOneBinder.java index 4ca09f8bfeb2..45fd614d8322 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ToOneBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/ToOneBinder.java @@ -259,6 +259,7 @@ private static org.hibernate.mapping.ManyToOne handleJoinTable( notFoundAction, value ) ); + value.markAsJoinTable(); return value; } else { @@ -270,7 +271,9 @@ private static org.hibernate.mapping.ManyToOne handleJoinTable( join.disableForeignKeyCreation(); } // All FK columns should be in the same table - return new org.hibernate.mapping.ManyToOne( context, joinColumns.getTable() ); + var manyToOne = new org.hibernate.mapping.ManyToOne( context, joinColumns.getTable() ); + manyToOne.markAsJoinTable(); + return manyToOne; } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java index 6c39a293cdd4..990aab263718 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java @@ -19,6 +19,7 @@ */ public final class ManyToOne extends ToOne { private boolean isLogicalOneToOne; + private boolean hasJoinTable; private NotFoundAction notFoundAction; private transient ManyToOneType resolvedType; @@ -141,6 +142,14 @@ public boolean isLogicalOneToOne() { return isLogicalOneToOne; } + public void markAsJoinTable() { + hasJoinTable = true; + } + + public boolean hasJoinTable() { + return hasJoinTable; + } + @Override public boolean isNullable() { return getReferencedPropertyName() != null || super.isNullable(); 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 92269d2280ae..6bddd55acb31 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 @@ -307,7 +307,7 @@ && equal( join.getKey(), manyToOne ) ) { this.hasJoinTable = hasJoinTable; } else { - this.hasJoinTable = false; + this.hasJoinTable = manyToOne.hasJoinTable(); bidirectionalAttributeName = findBidirectionalOneToManyAttributeName( propertyPath, declaringType, @@ -361,7 +361,7 @@ && equal( join.getKey(), manyToOne ) ) { } } isOptional = manyToOne.isIgnoreNotFound(); - isInternalLoadNullable = ( isNullable && bootValue.isForeignKeyEnabled() ) || hasNotFoundAction(); + isInternalLoadNullable = isNullable && bootValue.isForeignKeyEnabled() || hasNotFoundAction(); } else { assert bootValue instanceof OneToOne; @@ -412,12 +412,10 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does so in order to recognize the bidirectionality the "primaryKey." is removed from the otherSidePropertyName value. */ final OneToOne oneToOne = (OneToOne) bootValue; - if ( oneToOne.getMappedByProperty() == null ) { - this.bidirectionalAttributePath = SelectablePath.parse( referencedPropertyName ); - } - else { - this.bidirectionalAttributePath = SelectablePath.parse( oneToOne.getMappedByProperty() ); - } + this.bidirectionalAttributePath = + oneToOne.getMappedByProperty() == null + ? SelectablePath.parse( referencedPropertyName ) + : SelectablePath.parse( oneToOne.getMappedByProperty() ); notFoundAction = null; isKeyTableNullable = isNullable(); isOptional = !bootValue.isConstrained(); @@ -628,10 +626,9 @@ else if ( value instanceof OneToOne oneToOne ) { return null; } - private static FetchTiming adjustFetchTiming( - FetchTiming mappedFetchTiming, - ToOne bootValue) { - return bootValue instanceof ManyToOne manyToOne && manyToOne.getNotFoundAction() != null + private static FetchTiming adjustFetchTiming(FetchTiming mappedFetchTiming, ToOne bootValue) { + return bootValue instanceof ManyToOne manyToOne + && manyToOne.getNotFoundAction() != null ? FetchTiming.IMMEDIATE : mappedFetchTiming; } @@ -641,9 +638,8 @@ private static TableGroupProducer resolveDeclaringTableGroupProducer(EntityPersi NavigableRole parentRole = navigableRole.getParent(); String collectionRole = null; do { - final CollectionPart.Nature nature = CollectionPart.Nature.fromNameExact( - parentRole.getLocalName() - ); + final CollectionPart.Nature nature = + CollectionPart.Nature.fromNameExact( parentRole.getLocalName() ); if (nature != null) { collectionRole = parentRole.getParent().getFullPath(); break; @@ -1175,11 +1171,10 @@ class Mother { // If the parent is null, this is a simple collection fetch of a root, in which case the types must match if ( parentPath.getParent() == null ) { final String entityName = entityMappingType.getPartName(); - return parentPath.getFullPath().startsWith( entityName ) && ( - parentPath.getFullPath().length() == entityName.length() - // Ignore a possible alias - || parentPath.getFullPath().charAt( entityName.length() ) == '(' - ); + return parentPath.getFullPath().startsWith( entityName ) + && ( parentPath.getFullPath().length() == entityName.length() + // Ignore a possible alias + || parentPath.getFullPath().charAt( entityName.length() ) == '(' ); } // If we have a parent, we ensure that the parent is the same as the attribute name else { @@ -2109,8 +2104,9 @@ public TableGroupJoin createTableGroupJoin( // @SQLRestriction should not be applied when joining FK association, // because that would result in us setting the FK to null when the // owning entity is updated, that is, to data loss. - // But we'll let it apply on the TARGET side of a @OneToOne - if ( sideNature == ForeignKeyDescriptor.Nature.TARGET ) { + // But we let it apply on the TARGET side of a @OneToOne, and we apply + // it whenever there is a dedicated join table. + if ( hasJoinTable || sideNature == ForeignKeyDescriptor.Nature.TARGET ) { associatedEntityMappingType.applyWhereRestrictions( join::applyPredicate, tableGroup, From fff3606121ee9b4aa338bf52cf7a914c4aacdfa8 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 15:39:15 +0200 Subject: [PATCH 5/7] HHH-19565 start cleaning up very messy code in ToOneAttributeMapping after bug fix --- .../internal/ToOneAttributeMapping.java | 218 +++++++++--------- 1 file changed, 113 insertions(+), 105 deletions(-) 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 6bddd55acb31..5d6016c55c13 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 @@ -49,7 +49,6 @@ import org.hibernate.metamodel.mapping.ValuedModelPart; import org.hibernate.metamodel.mapping.VirtualModelPart; import org.hibernate.metamodel.model.domain.NavigableRole; -import org.hibernate.persister.collection.AbstractCollectionPersister; import org.hibernate.persister.entity.EntityNameUse; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.persister.entity.JoinedSubclassEntityPersister; @@ -113,6 +112,8 @@ import java.util.function.Consumer; import java.util.function.Supplier; +import static org.hibernate.metamodel.mapping.internal.MappingModelCreationHelper.getTableIdentifierExpression; + /** * @author Steve Ebersole */ @@ -253,23 +254,24 @@ public ToOneAttributeMapping( declaringType, propertyAccess ); - this.sqlAliasStem = SqlAliasStemHelper.INSTANCE.generateStemFromAttributeName( name ); - this.isNullable = bootValue.isNullable(); - this.isLazy = navigableRole.getParent().getParent() == null + this.entityMappingType = entityMappingType; + this.navigableRole = navigableRole; + sqlAliasStem = SqlAliasStemHelper.INSTANCE.generateStemFromAttributeName( name ); + isNullable = bootValue.isNullable(); + isLazy = navigableRole.getParent().getParent() == null && declaringEntityPersister.getBytecodeEnhancementMetadata() .getLazyAttributesMetadata() .isLazyAttribute( name ); - this.referencedPropertyName = bootValue.getReferencedPropertyName(); - this.unwrapProxy = bootValue.isUnwrapProxy(); - this.entityMappingType = entityMappingType; + referencedPropertyName = bootValue.getReferencedPropertyName(); + unwrapProxy = bootValue.isUnwrapProxy(); - this.navigableRole = navigableRole; - this.declaringTableGroupProducer = resolveDeclaringTableGroupProducer( + declaringTableGroupProducer = resolveDeclaringTableGroupProducer( declaringEntityPersister, navigableRole ); + final var factory = declaringEntityPersister.getFactory(); if ( bootValue instanceof ManyToOne manyToOne ) { - this.notFoundAction = manyToOne.getNotFoundAction(); + notFoundAction = manyToOne.getNotFoundAction(); cardinality = manyToOne.isLogicalOneToOne() ? Cardinality.LOGICAL_ONE_TO_ONE : Cardinality.MANY_TO_ONE; @@ -280,34 +282,17 @@ public ToOneAttributeMapping( final String propertyPath = bootValue.getPropertyName() == null ? name : bootValue.getPropertyName(); + hasJoinTable = manyToOne.hasJoinTable(); if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE ) { - boolean hasJoinTable = false; - // Handle join table cases - for ( Join join : entityBinding.getJoinClosure() ) { - if ( join.getPersistentClass().getEntityName().equals( entityBinding.getEntityName() ) - && join.getPropertySpan() == 1 - && join.getTable() == manyToOne.getTable() - && equal( join.getKey(), manyToOne ) ) { - bidirectionalAttributeName = SelectablePath.parse( - join.getProperties().get(0).getName() - ); - hasJoinTable = true; - break; - } - } - // Simple one-to-one mapped by cases - if ( bidirectionalAttributeName == null ) { - bidirectionalAttributeName = findBidirectionalOneToOneAttributeName( - propertyPath, - declaringType, - null, - entityBinding.getPropertyClosure() - ); - } - this.hasJoinTable = hasJoinTable; + bidirectionalAttributeName = findBidirectionalOneToOneAttributeName( + declaringType, + manyToOne, + entityBinding, + bidirectionalAttributeName, + propertyPath + ); } else { - this.hasJoinTable = manyToOne.hasJoinTable(); bidirectionalAttributeName = findBidirectionalOneToManyAttributeName( propertyPath, declaringType, @@ -315,16 +300,17 @@ && equal( join.getKey(), manyToOne ) ) { entityBinding.getPropertyClosure() ); } - this.bidirectionalAttributePath = bidirectionalAttributeName; + bidirectionalAttributePath = bidirectionalAttributeName; } else { // Only set the bidirectional attribute name if the referenced property can actually be circular i.e. an entity type final Property property = entityBinding.getProperty( referencedPropertyName ); - this.hasJoinTable = cardinality == Cardinality.LOGICAL_ONE_TO_ONE - && property != null - && property.getValue() instanceof ManyToOne manyToOneValue - && manyToOneValue.isLogicalOneToOne(); - this.bidirectionalAttributePath = + hasJoinTable = + cardinality == Cardinality.LOGICAL_ONE_TO_ONE + && property != null + && property.getValue() instanceof ManyToOne manyToOneValue + && manyToOneValue.isLogicalOneToOne(); + bidirectionalAttributePath = property != null && property.getValue().getType() instanceof EntityType ? SelectablePath.parse( referencedPropertyName ) : null; @@ -333,24 +319,21 @@ && equal( join.getKey(), manyToOne ) ) { isKeyTableNullable = true; } else { - final String targetTableName = MappingModelCreationHelper.getTableIdentifierExpression( - manyToOne.getTable(), - declaringEntityPersister.getFactory() - ); + final String targetTableName = + getTableIdentifierExpression( manyToOne.getTable(), factory ); if ( CollectionPart.Nature.fromNameExact( navigableRole.getParent().getLocalName() ) != null ) { // * the to-one's parent is directly a collection element or index // * therefore, its parent-parent should be the collection itself - final PluralAttributeMapping pluralAttribute = (PluralAttributeMapping) declaringEntityPersister.findByPath( - navigableRole.getParent() - .getParent() - .getFullPath() - .substring( declaringEntityPersister.getNavigableRole() - .getFullPath() - .length() + 1 ) ); + final String rootPath = declaringEntityPersister.getNavigableRole().getFullPath(); + final String unqualifiedPath = + navigableRole.getParent().getParent().getFullPath() + .substring( rootPath.length() + 1 ); + final PluralAttributeMapping pluralAttribute = + (PluralAttributeMapping) declaringEntityPersister.findByPath( unqualifiedPath ); assert pluralAttribute != null; - - final AbstractCollectionPersister persister = (AbstractCollectionPersister) pluralAttribute.getCollectionDescriptor(); - isKeyTableNullable = !persister.getTableName().equals( targetTableName ); + isKeyTableNullable = + !pluralAttribute.getCollectionDescriptor().getTableName() + .equals( targetTableName ); } else { final int tableIndex = ArrayHelper.indexOf( @@ -363,8 +346,7 @@ && equal( join.getKey(), manyToOne ) ) { isOptional = manyToOne.isIgnoreNotFound(); isInternalLoadNullable = isNullable && bootValue.isForeignKeyEnabled() || hasNotFoundAction(); } - else { - assert bootValue instanceof OneToOne; + else if ( bootValue instanceof OneToOne oneToOne ) { cardinality = Cardinality.ONE_TO_ONE; hasJoinTable = false; @@ -411,8 +393,7 @@ class PrimaryKey implements Serializable { the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does not contain the "primaryKey" part, so in order to recognize the bidirectionality the "primaryKey." is removed from the otherSidePropertyName value. */ - final OneToOne oneToOne = (OneToOne) bootValue; - this.bidirectionalAttributePath = + bidirectionalAttributePath = oneToOne.getMappedByProperty() == null ? SelectablePath.parse( referencedPropertyName ) : SelectablePath.parse( oneToOne.getMappedByProperty() ); @@ -421,6 +402,9 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does isOptional = !bootValue.isConstrained(); isInternalLoadNullable = isNullable(); } + else { + throw new AssertionFailure( "Unrecognized kind of ToOne" ); + } if ( entityMappingType.getSoftDeleteMapping() != null ) { // cannot be lazy @@ -439,13 +423,10 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does targetKeyPropertyNames.add( EntityIdentifierMapping.ID_ROLE_NAME ); final PersistentClass entityBinding = bootValue.getBuildingContext().getMetadataCollector() .getEntityBinding( entityMappingType.getEntityName() ); - final Type propertyType; - if ( entityBinding.getIdentifierMapper() == null ) { - propertyType = entityBinding.getIdentifier().getType(); - } - else { - propertyType = entityBinding.getIdentifierMapper().getType(); - } + final Type propertyType = + entityBinding.getIdentifierMapper() == null + ? entityBinding.getIdentifier().getType() + : entityBinding.getIdentifierMapper().getType(); if ( entityBinding.getIdentifierProperty() == null ) { if ( propertyType instanceof ComponentType compositeType && compositeType.isEmbedded() && compositeType.getPropertyNames().length == 1 ) { @@ -454,32 +435,32 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does targetKeyPropertyNames, targetKeyPropertyName, compositeType.getSubtypes()[0], - declaringEntityPersister.getFactory() + factory ); addPrefixedPropertyNames( targetKeyPropertyNames, EntityIdentifierMapping.ID_ROLE_NAME, propertyType, - declaringEntityPersister.getFactory() + factory ); } else { - this.targetKeyPropertyName = EntityIdentifierMapping.ID_ROLE_NAME; + targetKeyPropertyName = EntityIdentifierMapping.ID_ROLE_NAME; addPrefixedPropertyPaths( targetKeyPropertyNames, null, propertyType, - declaringEntityPersister.getFactory() + factory ); } } else { - this.targetKeyPropertyName = entityBinding.getIdentifierProperty().getName(); + targetKeyPropertyName = entityBinding.getIdentifierProperty().getName(); addPrefixedPropertyPaths( targetKeyPropertyNames, targetKeyPropertyName, propertyType, - declaringEntityPersister.getFactory() + factory ); } this.targetKeyPropertyNames = targetKeyPropertyNames; @@ -489,19 +470,19 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does .getEntityBinding( entityMappingType.getEntityName() ); final Type propertyType = entityBinding.getRecursiveProperty( referencedPropertyName ).getType(); if ( bootValue.isReferenceToPrimaryKey() ) { - this.targetKeyPropertyName = referencedPropertyName; + targetKeyPropertyName = referencedPropertyName; final Set targetKeyPropertyNames = new HashSet<>( 3 ); addPrefixedPropertyNames( targetKeyPropertyNames, targetKeyPropertyName, propertyType, - declaringEntityPersister.getFactory() + factory ); addPrefixedPropertyNames( targetKeyPropertyNames, null, bootValue.getType(), - declaringEntityPersister.getFactory() + factory ); this.targetKeyPropertyNames = targetKeyPropertyNames; } @@ -509,49 +490,46 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does if ( propertyType instanceof ComponentType compositeType && compositeType.isEmbedded() && compositeType.getPropertyNames().length == 1 ) { final Set targetKeyPropertyNames = new HashSet<>( 2 ); - this.targetKeyPropertyName = compositeType.getPropertyNames()[0]; + targetKeyPropertyName = compositeType.getPropertyNames()[0]; addPrefixedPropertyPaths( targetKeyPropertyNames, targetKeyPropertyName, compositeType.getSubtypes()[0], - declaringEntityPersister.getFactory() + factory ); addPrefixedPropertyNames( targetKeyPropertyNames, EntityIdentifierMapping.ID_ROLE_NAME, propertyType, - declaringEntityPersister.getFactory() + factory ); this.targetKeyPropertyNames = targetKeyPropertyNames; } else { final Set targetKeyPropertyNames = new HashSet<>( 2 ); - this.targetKeyPropertyName = referencedPropertyName; - final String mapsIdAttributeName; + targetKeyPropertyName = referencedPropertyName; + final String mapsIdAttributeName = findMapsIdPropertyName( entityMappingType, referencedPropertyName ); // If there is a "virtual property" for a non-PK join mapping, we try to see if the columns match the // primary key columns and if so, we add the primary key property name as target key property - if ( ( mapsIdAttributeName = findMapsIdPropertyName( - entityMappingType, - referencedPropertyName - ) ) != null ) { + if ( mapsIdAttributeName != null ) { addPrefixedPropertyPaths( targetKeyPropertyNames, mapsIdAttributeName, entityMappingType.getEntityPersister().getIdentifierType(), - declaringEntityPersister.getFactory() + factory ); } addPrefixedPropertyNames( targetKeyPropertyNames, targetKeyPropertyName, propertyType, - declaringEntityPersister.getFactory() + factory ); addPrefixedPropertyNames( targetKeyPropertyNames, ForeignKeyDescriptor.PART_NAME, propertyType, - declaringEntityPersister.getFactory() + factory ); this.targetKeyPropertyNames = targetKeyPropertyNames; } @@ -559,6 +537,37 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does } } + private SelectablePath findBidirectionalOneToOneAttributeName( + ManagedMappingType declaringType, + ManyToOne manyToOne, + PersistentClass entityBinding, + SelectablePath bidirectionalAttributeName, + String propertyPath) { + //boolean foundJoinTable = false; + // Handle join table cases + for ( Join join : entityBinding.getJoinClosure() ) { + if ( join.getPersistentClass().getEntityName().equals( entityBinding.getEntityName() ) + && join.getPropertySpan() == 1 + && join.getTable() == manyToOne.getTable() + && equal( join.getKey(), manyToOne ) ) { + bidirectionalAttributeName = SelectablePath.parse( join.getProperties().get(0).getName() ); +// foundJoinTable = true; + break; + } + } + // Simple one-to-one mapped by cases + if ( bidirectionalAttributeName == null ) { + bidirectionalAttributeName = findBidirectionalOneToOneAttributeName( + propertyPath, + declaringType, + null, + entityBinding.getPropertyClosure() + ); + } +// assert hasJoinTable == foundJoinTable; + return bidirectionalAttributeName; + } + private static SelectablePath findBidirectionalOneToManyAttributeName( String propertyPath, ManagedMappingType declaringType, @@ -633,19 +642,21 @@ private static FetchTiming adjustFetchTiming(FetchTiming mappedFetchTiming, ToOn : mappedFetchTiming; } - private static TableGroupProducer resolveDeclaringTableGroupProducer(EntityPersister declaringEntityPersister, NavigableRole navigableRole) { + private static TableGroupProducer resolveDeclaringTableGroupProducer( + EntityPersister declaringEntityPersister, NavigableRole navigableRole) { // Also handle cases where a collection contains an embeddable, that contains an association NavigableRole parentRole = navigableRole.getParent(); String collectionRole = null; do { final CollectionPart.Nature nature = CollectionPart.Nature.fromNameExact( parentRole.getLocalName() ); - if (nature != null) { + if ( nature != null ) { collectionRole = parentRole.getParent().getFullPath(); break; } parentRole = parentRole.getParent(); - } while (parentRole != null); + } + while ( parentRole != null ); if ( collectionRole != null ) { // This is a collection part i.e. to-many association @@ -916,19 +927,16 @@ public ModelPart findSubPart(String name, EntityMappingType targetType) { // Prefer resolving the key part of the foreign key rather than the target part if possible // This way, we don't have to register table groups the target entity type if ( canUseParentTableGroup && targetKeyPropertyNames.contains( name ) ) { - final ModelPart fkPart; - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - fkPart = foreignKeyDescriptor.getKeyPart(); - } - else { - fkPart = foreignKeyDescriptor.getTargetPart(); - } - if ( fkPart instanceof EmbeddableValuedModelPart && fkPart instanceof VirtualModelPart + final ModelPart fkPart = + sideNature == ForeignKeyDescriptor.Nature.KEY + ? foreignKeyDescriptor.getKeyPart() + : foreignKeyDescriptor.getTargetPart(); + if ( fkPart instanceof EmbeddableValuedModelPart modelPart && fkPart instanceof VirtualModelPart && !EntityIdentifierMapping.ID_ROLE_NAME.equals( name ) && !ForeignKeyDescriptor.PART_NAME.equals( name ) && !ForeignKeyDescriptor.TARGET_PART_NAME.equals( name ) && !fkPart.getPartName().equals( name ) ) { - return ( (ModelPartContainer) fkPart ).findSubPart( name, targetType ); + return modelPart.findSubPart( name, targetType ); } return fkPart; } @@ -1020,10 +1028,9 @@ class Mother { We have a circularity but it is not bidirectional */ - final TableGroup parentTableGroup = creationState - .getSqlAstCreationState() - .getFromClauseAccess() - .getTableGroup( fetchParent.getNavigablePath() ); + final TableGroup parentTableGroup = + creationState.getSqlAstCreationState().getFromClauseAccess() + .getTableGroup( fetchParent.getNavigablePath() ); final DomainResult foreignKeyDomainResult; assert !creationState.isResolvingCircularFetch(); try { @@ -1303,8 +1310,9 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( else { // We get here is this is a lazy collection initialization for which we know the owner is in the PC // So we create a delayed fetch, as we are sure to find the entity in the PC - final FromClauseAccess fromClauseAccess = creationState.getSqlAstCreationState().getFromClauseAccess(); - final TableGroup parentTableGroup = fromClauseAccess.getTableGroup( parentNavigablePath ); + final TableGroup parentTableGroup = + creationState.getSqlAstCreationState().getFromClauseAccess() + .getTableGroup( parentNavigablePath ); final DomainResult domainResult; if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { domainResult = foreignKeyDescriptor.createKeyDomainResult( From 83cd97e0efe38a46c89780dbed3f96c4166619f9 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 18:44:40 +0200 Subject: [PATCH 6/7] HHH-19565 test demonstrating consistent behavior with lazy case --- .../ManyToOneLazyRestrictionTest.java | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneLazyRestrictionTest.java diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneLazyRestrictionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneLazyRestrictionTest.java new file mode 100644 index 000000000000..21efe6133128 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/where/annotations/ManyToOneLazyRestrictionTest.java @@ -0,0 +1,77 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.where.annotations; + +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Table; +import org.hibernate.Hibernate; +import org.hibernate.annotations.SQLRestriction; +import org.hibernate.testing.orm.junit.EntityManagerFactoryScope; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.Jpa; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +@Jpa(annotatedClasses = {ManyToOneLazyRestrictionTest.X.class, ManyToOneLazyRestrictionTest.Y.class}) +class ManyToOneLazyRestrictionTest { + @JiraKey("HHH-19565") + @Test void test(EntityManagerFactoryScope scope) { + scope.inTransaction(em -> { + Y y = new Y(); + X x = new X(); + x.id = -1; + y.x = x; + em.persist(x); + em.persist(y); + }); + // @SQLRestrictions should not be applied to + // foreign key associations, or the FK will + // be set to null when the entity is updated, + // leading to data loss + scope.inTransaction(em -> { + Y y = em.find(Y.class, 0L); + assertNotNull(y.x); + assertFalse(Hibernate.isInitialized(y.x)); + assertEquals(-1, y.x.getId()); + y.name = "hello"; + }); + scope.inTransaction(em -> { + Y y = em.find(Y.class, 0L); + assertNotNull(y.x); + assertEquals(-1, y.x.getId()); + assertEquals("hello", y.name); + assertFalse(Hibernate.isInitialized(y.x)); + }); + } + + @Entity + @Table(name = "XX") + @SQLRestriction("id>0") + static class X { + @Id + long id; + + public long getId() { + return id; + } + } + @Entity + @Table(name = "YY") + static class Y { + @Id + long id; + String name; + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "xx") + X x; + } +} From 79281fc611785ec4a5d4153c1d2e10c6ad60500a Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sat, 21 Jun 2025 21:28:25 +0200 Subject: [PATCH 7/7] HHH-19565 continue cleanup of ToOneAttributeMapping --- .../internal/EntityConcreteTypeLoader.java | 15 +- .../internal/ToOneAttributeMapping.java | 1138 +++++++++-------- 2 files changed, 612 insertions(+), 541 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityConcreteTypeLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityConcreteTypeLoader.java index c4bb0e742cdd..e7af4e4922f3 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityConcreteTypeLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityConcreteTypeLoader.java @@ -10,6 +10,7 @@ import org.hibernate.ObjectNotFoundException; import org.hibernate.WrongClassException; import org.hibernate.annotations.ConcreteProxy; +import org.hibernate.engine.jdbc.spi.JdbcServices; import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; @@ -20,7 +21,6 @@ import org.hibernate.sql.ast.tree.select.SelectStatement; import org.hibernate.sql.exec.internal.BaseExecutionContext; import org.hibernate.sql.exec.internal.JdbcParameterBindingsImpl; -import org.hibernate.sql.exec.spi.JdbcOperationQuerySelect; import org.hibernate.sql.exec.spi.JdbcParameterBindings; import org.hibernate.sql.exec.spi.JdbcParametersList; import org.hibernate.sql.results.internal.RowTransformerStandardImpl; @@ -60,6 +60,7 @@ public EntityConcreteTypeLoader(EntityMappingType entityDescriptor, SessionFacto public EntityMappingType getConcreteType(Object id, SharedSessionContractImplementor session) { final SessionFactoryImplementor factory = session.getSessionFactory(); + final JdbcServices jdbcServices = factory.getJdbcServices(); final JdbcParameterBindings bindings = new JdbcParameterBindingsImpl( jdbcParameters.size() ); final int offset = bindings.registerParametersForEachJdbcValue( @@ -70,14 +71,12 @@ public EntityMappingType getConcreteType(Object id, SharedSessionContractImpleme ); assert offset == jdbcParameters.size(); - final JdbcOperationQuerySelect jdbcSelect = - factory.getJdbcServices().getJdbcEnvironment().getSqlAstTranslatorFactory() - .buildSelectTranslator( factory, sqlSelect ) - .translate( bindings, QueryOptions.NONE ); - final List results = - session.getFactory().getJdbcServices().getJdbcSelectExecutor() + final List results = + jdbcServices.getJdbcSelectExecutor() .list( - jdbcSelect, + jdbcServices.getJdbcEnvironment().getSqlAstTranslatorFactory() + .buildSelectTranslator( factory, sqlSelect ) + .translate( bindings, QueryOptions.NONE ), bindings, new BaseExecutionContext( session ), RowTransformerStandardImpl.instance(), 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 5d6016c55c13..7925d014dc18 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 @@ -14,7 +14,6 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.internal.util.IndexedConsumer; -import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.collections.ArrayHelper; import org.hibernate.mapping.Collection; import org.hibernate.mapping.Component; @@ -85,7 +84,6 @@ import org.hibernate.sql.results.graph.FetchOptions; import org.hibernate.sql.results.graph.FetchParent; import org.hibernate.sql.results.graph.Fetchable; -import org.hibernate.sql.results.graph.FetchableContainer; import org.hibernate.sql.results.graph.InitializerParent; import org.hibernate.sql.results.graph.embeddable.EmbeddableValuedFetchable; import org.hibernate.sql.results.graph.entity.EntityFetch; @@ -112,6 +110,7 @@ import java.util.function.Consumer; import java.util.function.Supplier; +import static org.hibernate.internal.util.StringHelper.split; import static org.hibernate.metamodel.mapping.internal.MappingModelCreationHelper.getTableIdentifierExpression; /** @@ -120,7 +119,7 @@ public class ToOneAttributeMapping extends AbstractSingularAttributeMapping implements EntityValuedFetchable, EntityAssociationMapping, TableGroupJoinProducer, - LazyTableGroup.ParentTableGroupUseChecker { + LazyTableGroup.ParentTableGroupUseChecker { public enum Cardinality { ONE_TO_ONE, @@ -259,16 +258,12 @@ public ToOneAttributeMapping( sqlAliasStem = SqlAliasStemHelper.INSTANCE.generateStemFromAttributeName( name ); isNullable = bootValue.isNullable(); isLazy = navigableRole.getParent().getParent() == null - && declaringEntityPersister.getBytecodeEnhancementMetadata() - .getLazyAttributesMetadata() - .isLazyAttribute( name ); + && declaringEntityPersister.getBytecodeEnhancementMetadata().getLazyAttributesMetadata() + .isLazyAttribute( name ); referencedPropertyName = bootValue.getReferencedPropertyName(); unwrapProxy = bootValue.isUnwrapProxy(); - declaringTableGroupProducer = resolveDeclaringTableGroupProducer( - declaringEntityPersister, - navigableRole - ); + declaringTableGroupProducer = resolveDeclaringTableGroupProducer( declaringEntityPersister, navigableRole ); final var factory = declaringEntityPersister.getFactory(); if ( bootValue instanceof ManyToOne manyToOne ) { notFoundAction = manyToOne.getNotFoundAction(); @@ -278,29 +273,9 @@ public ToOneAttributeMapping( final PersistentClass entityBinding = manyToOne.getMetadata().getEntityBinding( manyToOne.getReferencedEntityName() ); if ( referencedPropertyName == null ) { - SelectablePath bidirectionalAttributeName = null; - final String propertyPath = bootValue.getPropertyName() == null - ? name - : bootValue.getPropertyName(); hasJoinTable = manyToOne.hasJoinTable(); - if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE ) { - bidirectionalAttributeName = findBidirectionalOneToOneAttributeName( - declaringType, - manyToOne, - entityBinding, - bidirectionalAttributeName, - propertyPath - ); - } - else { - bidirectionalAttributeName = findBidirectionalOneToManyAttributeName( - propertyPath, - declaringType, - null, - entityBinding.getPropertyClosure() - ); - } - bidirectionalAttributePath = bidirectionalAttributeName; + bidirectionalAttributePath = + bidirectionalAttributePath( declaringType, manyToOne, name, entityBinding ); } else { // Only set the bidirectional attribute name if the referenced property can actually be circular i.e. an entity type @@ -537,12 +512,23 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does } } - private SelectablePath findBidirectionalOneToOneAttributeName( + private SelectablePath bidirectionalAttributePath( ManagedMappingType declaringType, ManyToOne manyToOne, - PersistentClass entityBinding, - SelectablePath bidirectionalAttributeName, - String propertyPath) { + String name, + PersistentClass entityBinding) { + final String propertyName = manyToOne.getPropertyName(); + final String propertyPath = propertyName == null ? name : propertyName; + return cardinality == Cardinality.LOGICAL_ONE_TO_ONE + ? findBidirectionalOneToOneAttributeName( propertyPath, declaringType, manyToOne, entityBinding ) + : findBidirectionalOneToManyAttributeName( declaringType, propertyPath, entityBinding ); + } + + private SelectablePath findBidirectionalOneToOneAttributeName( + String propertyPath, ManagedMappingType declaringType, + ManyToOne manyToOne, + PersistentClass entityBinding) { + SelectablePath bidirectionalAttributeName = null; //boolean foundJoinTable = false; // Handle join table cases for ( Join join : entityBinding.getJoinClosure() ) { @@ -568,22 +554,35 @@ && equal( join.getKey(), manyToOne ) ) { return bidirectionalAttributeName; } + private static SelectablePath findBidirectionalOneToManyAttributeName( + ManagedMappingType declaringType, + String propertyPath, + PersistentClass entityBinding) { + return findBidirectionalOneToManyAttributeName( + propertyPath, + declaringType, + null, + entityBinding.getPropertyClosure() + ); + } + private static SelectablePath findBidirectionalOneToManyAttributeName( String propertyPath, ManagedMappingType declaringType, SelectablePath parentSelectablePath, - java.util.Collection properties) { + List properties) { for ( Property property : properties ) { final Value value = property.getValue(); if ( value instanceof Component component ) { - final SelectablePath bidirectionalAttributeName = findBidirectionalOneToManyAttributeName( - propertyPath, - declaringType, - parentSelectablePath == null - ? SelectablePath.parse( property.getName() ) - : parentSelectablePath.append( property.getName() ), - component.getProperties() - ); + final SelectablePath bidirectionalAttributeName = + findBidirectionalOneToManyAttributeName( + propertyPath, + declaringType, + parentSelectablePath == null + ? SelectablePath.parse( property.getName() ) + : parentSelectablePath.append( property.getName() ), + component.getProperties() + ); if ( bidirectionalAttributeName != null ) { return bidirectionalAttributeName; } @@ -591,7 +590,7 @@ private static SelectablePath findBidirectionalOneToManyAttributeName( if ( value instanceof Collection collection ) { if ( propertyPath.equals( collection.getMappedByProperty() ) && collection.getElement().getType().getName() - .equals( declaringType.getJavaType().getTypeName() ) ) { + .equals( declaringType.getJavaType().getTypeName() ) ) { return parentSelectablePath == null ? SelectablePath.parse( property.getName() ) : parentSelectablePath.append( property.getName() ); @@ -605,30 +604,32 @@ private SelectablePath findBidirectionalOneToOneAttributeName( String propertyPath, ManagedMappingType declaringType, SelectablePath parentSelectablePath, - java.util.Collection properties) { + List properties) { for ( Property property : properties ) { final Value value = property.getValue(); + final String name = property.getName(); if ( value instanceof Component component ) { - final SelectablePath bidirectionalAttributeName = findBidirectionalOneToOneAttributeName( - propertyPath, - declaringType, - parentSelectablePath == null - ? SelectablePath.parse( property.getName() ) - : parentSelectablePath.append( property.getName() ), - component.getProperties() - ); + final SelectablePath bidirectionalAttributeName = + findBidirectionalOneToOneAttributeName( + propertyPath, + declaringType, + parentSelectablePath == null + ? SelectablePath.parse( name ) + : parentSelectablePath.append( name ), + component.getProperties() + ); if ( bidirectionalAttributeName != null ) { return bidirectionalAttributeName; } } else if ( value instanceof OneToOne oneToOne ) { - if ( declaringTableGroupProducer.getNavigableRole().getLocalName() - .equals( oneToOne.getReferencedEntityName() ) + final String referencedEntityName = oneToOne.getReferencedEntityName(); + if ( declaringTableGroupProducer.getNavigableRole().getLocalName().equals( referencedEntityName ) && propertyPath.equals( oneToOne.getMappedByProperty() ) - && oneToOne.getReferencedEntityName().equals( declaringType.getJavaType().getTypeName() ) ) { + && referencedEntityName.equals( declaringType.getJavaType().getTypeName() ) ) { return parentSelectablePath == null - ? SelectablePath.parse( property.getName() ) - : parentSelectablePath.append( property.getName() ); + ? SelectablePath.parse( name ) + : parentSelectablePath.append( name ); } } } @@ -644,20 +645,8 @@ private static FetchTiming adjustFetchTiming(FetchTiming mappedFetchTiming, ToOn private static TableGroupProducer resolveDeclaringTableGroupProducer( EntityPersister declaringEntityPersister, NavigableRole navigableRole) { - // Also handle cases where a collection contains an embeddable, that contains an association - NavigableRole parentRole = navigableRole.getParent(); - String collectionRole = null; - do { - final CollectionPart.Nature nature = - CollectionPart.Nature.fromNameExact( parentRole.getLocalName() ); - if ( nature != null ) { - collectionRole = parentRole.getParent().getFullPath(); - break; - } - parentRole = parentRole.getParent(); - } - while ( parentRole != null ); - + // Also handle cases where a collection contains an embeddable that contains an association + final String collectionRole = collectionRole( navigableRole ); if ( collectionRole != null ) { // This is a collection part i.e. to-many association return declaringEntityPersister.getFactory().getMappingMetamodel() @@ -668,6 +657,18 @@ private static TableGroupProducer resolveDeclaringTableGroupProducer( return declaringEntityPersister; } + private static String collectionRole(NavigableRole navigableRole) { + NavigableRole parentRole = navigableRole.getParent(); + do { + if ( CollectionPart.Nature.fromNameExact( parentRole.getLocalName() ) != null ) { + return parentRole.getParent().getFullPath(); + } + parentRole = parentRole.getParent(); + } + while ( parentRole != null ); + return null; + } + private ToOneAttributeMapping( ToOneAttributeMapping original, ManagedMappingType declaringType, @@ -720,10 +721,10 @@ private static boolean equal(Value lhsValue, Value rhsValue) { static String findMapsIdPropertyName(EntityMappingType entityMappingType, String referencedPropertyName) { final EntityPersister persister = entityMappingType.getEntityPersister(); - if ( Arrays.equals( persister.getIdentifierColumnNames(), persister.getPropertyColumnNames( referencedPropertyName ) ) ) { - return persister.getIdentifierPropertyName(); - } - return null; + return Arrays.equals( persister.getIdentifierColumnNames(), + persister.getPropertyColumnNames( referencedPropertyName ) ) + ? persister.getIdentifierPropertyName() + : null; } public static void addPrefixedPropertyPaths( @@ -770,16 +771,7 @@ public static void addPrefixedPropertyNames( else if ( type instanceof EntityType entityType ) { final Type identifierOrUniqueKeyType = entityType.getIdentifierOrUniqueKeyType( factory.getRuntimeMetamodels() ); - final String propertyName; - if ( entityType.isReferenceToPrimaryKey() ) { - propertyName = entityType.getAssociatedEntityPersister( factory ).getIdentifierPropertyName(); - } - else if ( identifierOrUniqueKeyType instanceof EmbeddedComponentType ) { - propertyName = null; - } - else { - propertyName = entityType.getRHSUniqueKeyPropertyName(); - } + final String propertyName = propertyName( factory, entityType, identifierOrUniqueKeyType ); final String newPrefix; final String newPkPrefix; final String newFkPrefix; @@ -818,6 +810,18 @@ else if ( propertyName == null ) { } } + private static String propertyName(SessionFactoryImplementor factory, EntityType entityType, Type identifierOrUniqueKeyType) { + if ( entityType.isReferenceToPrimaryKey() ) { + return entityType.getAssociatedEntityPersister( factory ).getIdentifierPropertyName(); + } + else if ( identifierOrUniqueKeyType instanceof EmbeddedComponentType ) { + return null; + } + else { + return entityType.getRHSUniqueKeyPropertyName(); + } + } + public ToOneAttributeMapping copy(ManagedMappingType declaringType, TableGroupProducer declaringTableGroupProducer) { return new ToOneAttributeMapping( this, declaringType, declaringTableGroupProducer ); } @@ -827,11 +831,12 @@ public void setForeignKeyDescriptor(ForeignKeyDescriptor foreignKeyDescriptor) { assert identifyingColumnsTableExpression != null; this.foreignKeyDescriptor = foreignKeyDescriptor; if ( cardinality == Cardinality.ONE_TO_ONE && bidirectionalAttributePath != null ) { - this.sideNature = ForeignKeyDescriptor.Nature.TARGET; + sideNature = ForeignKeyDescriptor.Nature.TARGET; } else { - this.sideNature = foreignKeyDescriptor.getAssociationKey().table() - .equals( identifyingColumnsTableExpression ) + sideNature = + foreignKeyDescriptor.getAssociationKey().table() + .equals( identifyingColumnsTableExpression ) ? ForeignKeyDescriptor.Nature.KEY : ForeignKeyDescriptor.Nature.TARGET; } @@ -842,8 +847,8 @@ public void setForeignKeyDescriptor(ForeignKeyDescriptor foreignKeyDescriptor) { // Otherwise we need to join to the associated entity table(s) final boolean forceJoin = hasNotFoundAction() || entityMappingType.getSoftDeleteMapping() != null - || ( cardinality == Cardinality.ONE_TO_ONE && isNullable() ); - this.canUseParentTableGroup = ! forceJoin + || cardinality == Cardinality.ONE_TO_ONE && isNullable(); + canUseParentTableGroup = !forceJoin && sideNature == ForeignKeyDescriptor.Nature.KEY && declaringTableGroupProducer.containsTableReference( identifyingColumnsTableExpression ); } @@ -943,6 +948,15 @@ public ModelPart findSubPart(String name, EntityMappingType targetType) { return EntityValuedFetchable.super.findSubPart( name, targetType ); } + private boolean requiresJoinForDelayedFetch() { + return entityMappingType.isConcreteProxy() && sideNature == ForeignKeyDescriptor.Nature.TARGET; +// || entityMappingType.hasWhereRestrictions() && canAddRestriction() + } + + private boolean canAddRestriction() { + return hasJoinTable || sideNature == ForeignKeyDescriptor.Nature.TARGET; + } + @Override public Fetch resolveCircularFetch( NavigablePath fetchablePath, @@ -1074,7 +1088,6 @@ protected boolean isBidirectionalAttributeName( ModelPart parentModelPart, NavigablePath fetchablePath, DomainResultCreationState creationState) { - if ( bidirectionalAttributePath == null ) { /* check if mappedBy is on the other side of the association @@ -1170,9 +1183,10 @@ class Mother { final NavigablePath parentPath = grandparentNavigablePath.getParent(); // This can be null for a collection loader if ( parentPath == null ) { - return grandparentNavigablePath.getFullPath().equals( - entityMappingType.findByPath( bidirectionalAttributePath ).getNavigableRole().getFullPath() - ); + final String fullPath = + entityMappingType.findByPath( bidirectionalAttributePath ) + .getNavigableRole().getFullPath(); + return grandparentNavigablePath.getFullPath().equals( fullPath ); } else { // If the parent is null, this is a simple collection fetch of a root, in which case the types must match @@ -1246,30 +1260,20 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( referencedNavigablePath = getReferencedNavigablePath( creationState, parentNavigablePath ); hasBidirectionalFetchParent = fetchParent instanceof Fetch; } + + final SqlAstCreationState sqlAstCreationState = creationState.getSqlAstCreationState(); + final FromClauseAccess fromClauseAccess = sqlAstCreationState.getFromClauseAccess(); // The referencedNavigablePath can be null if this is a collection initialization if ( referencedNavigablePath != null ) { // If this is the key side, we must ensure that the key is not null, so we create a domain result for it // In the CircularBiDirectionalFetchImpl we return null if the key is null instead of the bidirectional value - final DomainResult keyDomainResult; - // For now, we don't do this if the key table is nullable to avoid an additional join - if ( sideNature == ForeignKeyDescriptor.Nature.KEY && !isKeyTableNullable ) { - keyDomainResult = foreignKeyDescriptor.createKeyDomainResult( - fetchablePath, - createTableGroupForDelayedFetch( - fetchablePath, - creationState.getSqlAstCreationState() - .getFromClauseAccess() - .findTableGroup( realFetchParent.getNavigablePath() ), - null, - creationState - ), - fetchParent, - creationState - ); - } - else { - keyDomainResult = null; - } + final DomainResult keyDomainResult = keyDomainResult( + fetchablePath, + fetchParent, + creationState, + sqlAstCreationState, + realFetchParent + ); if ( hasBidirectionalFetchParent ) { return new CircularBiDirectionalFetchImpl( @@ -1287,11 +1291,10 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( // The problem with a bidirectional fetch though is that we can't find an initializer // because there is none, as we don't fetch the data of the parent node. // To avoid creating another join, we create a special join fetch that uses the existing joined data - final FromClauseAccess fromClauseAccess = creationState.getSqlAstCreationState().getFromClauseAccess(); final TableGroup tableGroup = fromClauseAccess.getTableGroup( referencedNavigablePath ); fromClauseAccess.registerTableGroup( fetchablePath, tableGroup ); // Register a PROJECTION usage as we're effectively selecting the bidirectional association - creationState.getSqlAstCreationState().registerEntityNameUsage( + sqlAstCreationState.registerEntityNameUsage( tableGroup, EntityNameUse.PROJECTION, entityMappingType.getEntityName() @@ -1310,26 +1313,9 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( else { // We get here is this is a lazy collection initialization for which we know the owner is in the PC // So we create a delayed fetch, as we are sure to find the entity in the PC - final TableGroup parentTableGroup = - creationState.getSqlAstCreationState().getFromClauseAccess() - .getTableGroup( parentNavigablePath ); - final DomainResult domainResult; - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - domainResult = foreignKeyDescriptor.createKeyDomainResult( - fetchablePath, - createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ), - fetchParent, - creationState - ); - } - else { - domainResult = foreignKeyDescriptor.createTargetDomainResult( - fetchablePath, - parentTableGroup, - fetchParent, - creationState - ); - } + final TableGroup parentTableGroup = fromClauseAccess.getTableGroup( parentNavigablePath ); + final DomainResult domainResult = + domainResult( fetchablePath, fetchParent, creationState, parentTableGroup ); if ( fetchTiming == FetchTiming.IMMEDIATE ) { return buildEntityFetchSelect( fetchParent, @@ -1342,7 +1328,7 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( ); } - if ( entityMappingType.isConcreteProxy() && sideNature == ForeignKeyDescriptor.Nature.TARGET ) { + if ( requiresJoinForDelayedFetch() ) { createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ); } @@ -1357,6 +1343,55 @@ else if ( CollectionPart.Nature.fromNameExact( parentNavigablePath.getLocalName( } } + private DomainResult keyDomainResult( + NavigablePath fetchablePath, + FetchParent fetchParent, + DomainResultCreationState creationState, + SqlAstCreationState sqlAstCreationState, + FetchParent realFetchParent) { + // For now, we don't do this if the key table is nullable to avoid an additional join + if ( sideNature == ForeignKeyDescriptor.Nature.KEY && !isKeyTableNullable ) { + return foreignKeyDescriptor.createKeyDomainResult( + fetchablePath, + createTableGroupForDelayedFetch( + fetchablePath, + sqlAstCreationState.getFromClauseAccess() + .findTableGroup( realFetchParent.getNavigablePath() ), + null, + creationState + ), + fetchParent, + creationState + ); + } + else { + return null; + } + } + + private DomainResult domainResult( + NavigablePath fetchablePath, + FetchParent fetchParent, + DomainResultCreationState creationState, + TableGroup parentTableGroup) { + if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { + return foreignKeyDescriptor.createKeyDomainResult( + fetchablePath, + createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ), + fetchParent, + creationState + ); + } + else { + return foreignKeyDescriptor.createTargetDomainResult( + fetchablePath, + parentTableGroup, + fetchParent, + creationState + ); + } + } + /** * For Hibernate Reactive */ @@ -1424,9 +1459,6 @@ protected EntityFetch buildEntityFetchJoined( private NavigablePath getReferencedNavigablePath( DomainResultCreationState creationState, NavigablePath parentNavigablePath) { - NavigablePath referencedNavigablePath = parentNavigablePath.getParent(); - MappingType partMappingType = creationState.resolveModelPart( referencedNavigablePath ).getPartMappingType(); - /* class LineItem { @ManyToOne @@ -1479,6 +1511,8 @@ class Level3 { to be sure that the referencedNavigablePath corresponds to leve2Child */ + NavigablePath referencedNavigablePath = parentNavigablePath.getParent(); + MappingType partMappingType = creationState.resolveModelPart( referencedNavigablePath ).getPartMappingType(); while ( !( partMappingType instanceof EntityMappingType entityMapping ) || ( partMappingType != entityMappingType && !entityMappingType.getEntityPersister().isSubclassEntityName( partMappingType.getMappedJavaType().getTypeName() ) @@ -1501,53 +1535,48 @@ public EntityFetch generateFetch( String resultVariable, DomainResultCreationState creationState) { - final SqlAstCreationState sqlAstCreationState = creationState.getSqlAstCreationState(); - final FromClauseAccess fromClauseAccess = sqlAstCreationState.getFromClauseAccess(); - - final TableGroup parentTableGroup = fromClauseAccess.getTableGroup( fetchParent.getNavigablePath() ); - - final NavigablePath parentNavigablePath = fetchablePath.getParent(); - assert parentNavigablePath.equals( fetchParent.getNavigablePath() ) - || fetchParent.getNavigablePath() instanceof TreatedNavigablePath - && parentNavigablePath.equals( fetchParent.getNavigablePath().getRealParent() ); + assert fetchablePath.getParent().equals( fetchParent.getNavigablePath() ) + || fetchParent.getNavigablePath() instanceof TreatedNavigablePath + && fetchablePath.getParent().equals( fetchParent.getNavigablePath().getRealParent() ); /* - In case of selected we are going to add a fetch for the `fetchablePath` only if there is not already a `TableGroupJoin`. + If selected is true, we're going to add a fetch for the fetchablePath only if + there is not yet a TableGroupJoin. For example, given: - e.g. given : - public static class EntityA { - ... + public static class EntityA { + ... - @ManyToOne(fetch = FetchType.EAGER) - private EntityB entityB; - } + @ManyToOne(fetch = FetchType.EAGER) + private EntityB entityB; + } - @Entity(name = "EntityB") - public static class EntityB { - ... + @Entity(name = "EntityB") + public static class EntityB { + ... - private String name; - } + private String name; + } - and the HQL query : + Then, with the HQL query: - `Select a From EntityA a Left Join a.entityB b Where ( b.name IS NOT NULL )` + Select a From EntityA a Left Join a.entityB b Where (b.name IS NOT NULL) - having the left join we don't want to add an extra implicit join that will be translated into an SQL inner join (see HHH-15342) + having the 'left join', we don't want to add an extra implicit join that will be + translated into an SQL inner join (see HHH-15342). */ - final ForeignKeyDescriptor.Nature resolvingKeySideOfForeignKey = creationState.getCurrentlyResolvingForeignKeyPart(); - final ForeignKeyDescriptor.Nature side; - if ( resolvingKeySideOfForeignKey == ForeignKeyDescriptor.Nature.KEY && this.sideNature == ForeignKeyDescriptor.Nature.TARGET ) { - // If we are currently resolving the key part of a foreign key we do not want to add joins. - // So if the lhs of this association is the target of the FK, we have to use the KEY part to avoid a join - side = ForeignKeyDescriptor.Nature.KEY; - } - else { - side = this.sideNature; - } + final FromClauseAccess fromClauseAccess = creationState.getSqlAstCreationState().getFromClauseAccess(); + final TableGroup parentTableGroup = fromClauseAccess.getTableGroup( fetchParent.getNavigablePath() ); + + final ForeignKeyDescriptor.Nature side = + creationState.getCurrentlyResolvingForeignKeyPart() == ForeignKeyDescriptor.Nature.KEY + && sideNature == ForeignKeyDescriptor.Nature.TARGET + // If we are currently resolving the key part of a foreign key we do not want to add joins. + // So if the lhs of this association is the target of the FK, we have to use the KEY part to avoid a join + ? ForeignKeyDescriptor.Nature.KEY + : sideNature; - if ( ( fetchTiming == FetchTiming.IMMEDIATE && selected ) || needsJoinFetch( side ) ) { + if ( fetchTiming == FetchTiming.IMMEDIATE && selected || needsJoinFetch( side ) ) { final TableGroup tableGroup = determineTableGroupForFetch( fetchablePath, fetchParent, @@ -1561,37 +1590,20 @@ public static class EntityB { () -> { // When a filter exists that affects a singular association, we have to enable NotFound handling // to force an exception if the filter would result in the entity not being found. - // If we silently just read null, this could lead to data loss on flush + // If we silently just read null, this could lead to data loss on flush. final boolean affectedByEnabledFilters = isAffectedByEnabledFilters( creationState ); - DomainResult keyResult = null; - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - // If the key side is non-nullable we also need to add the keyResult - // to be able to manually check invalid foreign key references - if ( hasNotFoundAction() || !isInternalLoadNullable || affectedByEnabledFilters ) { - keyResult = foreignKeyDescriptor.createKeyDomainResult( - fetchablePath, - tableGroup, - fetchParent, - creationState - ); - } - } - else if ( hasNotFoundAction() - || getAssociatedEntityMappingType().getSoftDeleteMapping() != null ) { - // For the target side only add keyResult when a not-found action is present - keyResult = foreignKeyDescriptor.createTargetDomainResult( - fetchablePath, - parentTableGroup, - fetchParent, - creationState - ); - } - return buildEntityFetchJoined( fetchParent, this, tableGroup, - keyResult, + keyResult( + fetchParent, + fetchablePath, + creationState, + affectedByEnabledFilters, + tableGroup, + parentTableGroup + ), affectedByEnabledFilters, fetchablePath, creationState @@ -1600,80 +1612,119 @@ else if ( hasNotFoundAction() creationState ); } + else { + /* + 1. No JoinTable + Model: + EntityA{ + @ManyToOne + EntityB b + } - /* - 1. No JoinTable - Model: - EntityA{ - @ManyToOne - EntityB b - } + EntityB{ + @ManyToOne + EntityA a + } - EntityB{ - @ManyToOne - EntityA a - } + Relational: + ENTITY_A( id ) + ENTITY_B( id, entity_a_id) - Relational: - ENTITY_A( id ) - ENTITY_B( id, entity_a_id) + 1.1 EntityA -> EntityB : as keyResult we need ENTITY_B.id + 1.2 EntityB -> EntityA : as keyResult we need ENTITY_B.entity_a_id (FK referring column) - 1.1 EntityA -> EntityB : as keyResult we need ENTITY_B.id - 1.2 EntityB -> EntityA : as keyResult we need ENTITY_B.entity_a_id (FK referring column) + 2. JoinTable - 2. JoinTable + */ - */ + final DomainResult keyResult = keyResult( fetchParent, fetchablePath, creationState, side, parentTableGroup ); + final boolean selectByUniqueKey = isSelectByUniqueKey( side ); - final DomainResult keyResult; - if ( side == ForeignKeyDescriptor.Nature.KEY ) { - final TableGroup tableGroup = sideNature == ForeignKeyDescriptor.Nature.KEY - ? createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ) - : parentTableGroup; - keyResult = foreignKeyDescriptor.createKeyDomainResult( + if ( needsImmediateFetch( fetchTiming ) ) { + return buildEntityFetchSelect( + fetchParent, + this, + fetchablePath, + keyResult, + selectByUniqueKey, + isAffectedByEnabledFilters( creationState ), + creationState + ); + } + else { + if ( requiresJoinForDelayedFetch() ) { + createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ); + } + + return buildEntityDelayedFetch( + fetchParent, + this, + fetchablePath, + keyResult, + selectByUniqueKey, + creationState + ); + } + } + } + + private DomainResult keyResult( + FetchParent fetchParent, + NavigablePath fetchablePath, + DomainResultCreationState creationState, + boolean affectedByEnabledFilters, + TableGroup tableGroup, + TableGroup parentTableGroup) { + if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { + // If the key side is non-nullable we also need to add the keyResult + // to be able to manually check invalid foreign key references + if ( hasNotFoundAction() || !isInternalLoadNullable || affectedByEnabledFilters ) { + return foreignKeyDescriptor.createKeyDomainResult( + fetchablePath, + tableGroup, + fetchParent, + creationState + ); + } + } + else if ( hasNotFoundAction() || getAssociatedEntityMappingType().getSoftDeleteMapping() != null ) { + // For the target side only add keyResult when a not-found action is present + return foreignKeyDescriptor.createTargetDomainResult( fetchablePath, - tableGroup, + parentTableGroup, fetchParent, creationState ); } - else { - final TableGroup tableGroup = sideNature == ForeignKeyDescriptor.Nature.TARGET - ? parentTableGroup - : createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ); - keyResult = foreignKeyDescriptor.createTargetDomainResult( + return null; + } + + private DomainResult keyResult( + FetchParent fetchParent, + NavigablePath fetchablePath, + DomainResultCreationState creationState, + ForeignKeyDescriptor.Nature side, + TableGroup parentTableGroup) { + if ( side == ForeignKeyDescriptor.Nature.KEY ) { + return foreignKeyDescriptor.createKeyDomainResult( fetchablePath, - tableGroup, + sideNature == ForeignKeyDescriptor.Nature.KEY + ? createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ) + : parentTableGroup, fetchParent, creationState ); } - final boolean selectByUniqueKey = isSelectByUniqueKey( side ); - - if ( needsImmediateFetch( fetchTiming ) ) { - return buildEntityFetchSelect( - fetchParent, - this, + else { + return foreignKeyDescriptor.createTargetDomainResult( fetchablePath, - keyResult, - selectByUniqueKey, - isAffectedByEnabledFilters( creationState ), + sideNature == ForeignKeyDescriptor.Nature.TARGET + ? parentTableGroup + : createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ), + fetchParent, creationState ); } - - if ( entityMappingType.isConcreteProxy() && sideNature == ForeignKeyDescriptor.Nature.TARGET ) { - createTableGroupForDelayedFetch( fetchablePath, parentTableGroup, null, creationState ); - } - - return buildEntityDelayedFetch( - fetchParent, - this, - fetchablePath, - keyResult, - selectByUniqueKey, - creationState - ); } private boolean needsJoinFetch(ForeignKeyDescriptor.Nature side) { @@ -1684,8 +1735,8 @@ private boolean needsJoinFetch(ForeignKeyDescriptor.Nature side) { final ValuedModelPart targetPart = foreignKeyDescriptor.getTargetPart(); if ( identifier != targetPart ) { // If the identifier and the target part of the same class, we can preserve laziness as deferred loading will still work - return identifier.getExpressibleJavaType().getJavaTypeClass() != targetPart.getExpressibleJavaType() - .getJavaTypeClass(); + return identifier.getExpressibleJavaType().getJavaTypeClass() + != targetPart.getExpressibleJavaType().getJavaTypeClass(); } } @@ -1708,9 +1759,9 @@ else if ( !entityMappingType.isConcreteProxy() ) { // When resolving the concrete entity type we can preserve laziness // and handle not found actions based on the discriminator value return hasNotFoundAction() - || entityMappingType.getSoftDeleteMapping() != null - || ( !entityMappingType.getEntityPersister().isInstrumented() - && cardinality == Cardinality.ONE_TO_ONE && isOptional ); + || entityMappingType.getSoftDeleteMapping() != null + || isOptional && cardinality == Cardinality.ONE_TO_ONE + && !entityMappingType.getEntityPersister().isInstrumented(); } else { return false; @@ -1724,12 +1775,7 @@ private TableGroup determineTableGroupForFetch( String resultVariable, FromClauseAccess fromClauseAccess, DomainResultCreationState creationState) { - final FetchableContainer parentEntityType = fetchParent.getReferencedMappingType(); - final SqlAstJoinType joinType = - parentEntityType instanceof JoinedSubclassEntityPersister joinedSubclassEntityPersister - && joinedSubclassEntityPersister.findDeclaredAttributeMapping( getPartName() ) == null - ? getJoinTypeForFetch( fetchablePath, parentTableGroup ) - : null; + final SqlAstJoinType joinType = joinType( fetchablePath, fetchParent, parentTableGroup ); final TableGroup existingTableGroup = fromClauseAccess.findTableGroupForGetOrCreate( fetchablePath ); if ( existingTableGroup != null && existingTableGroup.getModelPart() == this ) { return existingTableGroup; @@ -1783,56 +1829,61 @@ && isSimpleJoinPredicate( tableGroupJoin.getPredicate() ) ) { } } + private SqlAstJoinType joinType(NavigablePath fetchablePath, FetchParent fetchParent, TableGroup parentTableGroup) { + return fetchParent.getReferencedMappingType() instanceof JoinedSubclassEntityPersister joinedSubclassEntityPersister + && joinedSubclassEntityPersister.findDeclaredAttributeMapping( getPartName() ) == null + ? getJoinTypeForFetch( fetchablePath, parentTableGroup ) + : null; + } + private TableGroup createTableGroupForDelayedFetch( NavigablePath fetchablePath, TableGroup parentTableGroup, String resultVariable, DomainResultCreationState creationState) { // Check if we can reuse a table group join of the parent - final TableGroup compatibleTableGroup = parentTableGroup.findCompatibleJoinedGroup( - this, - SqlAstJoinType.LEFT - ); + final TableGroup compatibleTableGroup = + parentTableGroup.findCompatibleJoinedGroup( this, SqlAstJoinType.LEFT ); if ( compatibleTableGroup != null ) { return compatibleTableGroup; } - // We have to create the table group that points to the target so that table reference resolving works - final TableGroupJoin tableGroupJoin = createTableGroupJoin( - fetchablePath, - parentTableGroup, - resultVariable, - null, - SqlAstJoinType.LEFT, - false, - false, - creationState.getSqlAstCreationState() - ); - parentTableGroup.addTableGroupJoin( tableGroupJoin ); - creationState.getSqlAstCreationState().getFromClauseAccess().registerTableGroup( - fetchablePath, - tableGroupJoin.getJoinedGroup() - ); - return tableGroupJoin.getJoinedGroup(); + else { + // We have to create the table group that points to the target so that table reference resolving works + final var sqlAstCreationState = creationState.getSqlAstCreationState(); + final TableGroupJoin tableGroupJoin = createTableGroupJoin( + fetchablePath, + parentTableGroup, + resultVariable, + null, + SqlAstJoinType.LEFT, + false, + false, + sqlAstCreationState + ); + parentTableGroup.addTableGroupJoin( tableGroupJoin ); + final TableGroup joinedGroup = tableGroupJoin.getJoinedGroup(); + sqlAstCreationState.getFromClauseAccess() + .registerTableGroup( fetchablePath, joinedGroup ); + return joinedGroup; + } } private boolean isSelectByUniqueKey(ForeignKeyDescriptor.Nature side) { if ( referencedPropertyName == null ) { return false; } + final EntityIdentifierMapping identifierMapping = entityMappingType.getIdentifierMapping(); if ( side == ForeignKeyDescriptor.Nature.KEY ) { // case 1.2 return !foreignKeyDescriptor.getNavigableRole() - .equals( entityMappingType.getIdentifierMapping().getNavigableRole() ); + .equals( identifierMapping.getNavigableRole() ); } else { // case 1.1 // Make sure the entity identifier is not a target key property i.e. this really is a unique key mapping - return bidirectionalAttributePath != null && ( - !( entityMappingType.getIdentifierMapping() instanceof SingleAttributeIdentifierMapping ) - || !targetKeyPropertyNames.contains( - entityMappingType.getIdentifierMapping().getAttributeName() - ) - ); + return bidirectionalAttributePath != null + && !( identifierMapping instanceof SingleAttributeIdentifierMapping + && targetKeyPropertyNames.contains( identifierMapping.getAttributeName() ) ); } } @@ -1856,12 +1907,12 @@ public DomainResult createSnapshotDomainResult( } } - public static class NullDomainResult implements DomainResult { - private final DomainResultAssembler resultAssembler; + public static class NullDomainResult implements DomainResult { + private final DomainResultAssembler resultAssembler; private final JavaType resultJavaType; - public NullDomainResult(JavaType javaType) { - resultAssembler = new NullValueAssembler( javaType ); + public NullDomainResult(JavaType javaType) { + resultAssembler = new NullValueAssembler<>( javaType ); this.resultJavaType = javaType; } @@ -1871,8 +1922,8 @@ public String getResultVariable() { } @Override - public DomainResultAssembler createResultAssembler( - InitializerParent parent, + public DomainResultAssembler createResultAssembler( + InitializerParent parent, AssemblerCreationState creationState) { return resultAssembler; } @@ -1892,19 +1943,7 @@ private EntityFetch withRegisteredAssociationKeys( Supplier fetchCreator, DomainResultCreationState creationState) { final boolean added = creationState.registerVisitedAssociationKey( foreignKeyDescriptor.getAssociationKey() ); - AssociationKey additionalAssociationKey = null; - if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE && bidirectionalAttributePath != null ) { - final ModelPart bidirectionalModelPart = entityMappingType.findByPath( bidirectionalAttributePath ); - // Add the inverse association key side as well to be able to resolve to a CircularFetch - if ( bidirectionalModelPart instanceof ToOneAttributeMapping bidirectionalAttribute ) { - assert bidirectionalModelPart.getPartMappingType() == declaringTableGroupProducer; - final AssociationKey secondKey = bidirectionalAttribute.getForeignKeyDescriptor().getAssociationKey(); - if ( creationState.registerVisitedAssociationKey( secondKey ) ) { - additionalAssociationKey = secondKey; - } - } - } - + final AssociationKey additionalAssociationKey = additionalAssociationKey( creationState ); try { return fetchCreator.get(); } @@ -1918,26 +1957,36 @@ private EntityFetch withRegisteredAssociationKeys( } } + private AssociationKey additionalAssociationKey(DomainResultCreationState creationState) { + if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE && bidirectionalAttributePath != null ) { + // Add the inverse association key side as well to be able to resolve to a CircularFetch + if ( entityMappingType.findByPath( bidirectionalAttributePath ) + instanceof ToOneAttributeMapping bidirectionalAttribute ) { + assert bidirectionalAttribute.getPartMappingType() == declaringTableGroupProducer; + final AssociationKey secondKey = bidirectionalAttribute.getForeignKeyDescriptor().getAssociationKey(); + if ( creationState.registerVisitedAssociationKey( secondKey ) ) { + return secondKey; + } + } + } + return null; + } + @Override public SqlAstJoinType getDefaultSqlAstJoinType(TableGroup parentTableGroup) { - if ( isKeyTableNullable || isNullable ) { + if ( isKeyTableNullable || isNullable + || parentTableGroup.getModelPart() instanceof CollectionPart + || !parentTableGroup.canUseInnerJoins() ) { return SqlAstJoinType.LEFT; } - else if ( parentTableGroup.getModelPart() instanceof CollectionPart ) { - return SqlAstJoinType.LEFT; - } - else { - if ( parentTableGroup.canUseInnerJoins() ) { - final Class attributeDeclaringType = declaringTableGroupProducer.getJavaType().getJavaTypeClass(); - final Class parentTableGroupType = parentTableGroup.getModelPart().getJavaType().getJavaTypeClass(); - - // This attribute mapping must be declared on the parent table group type or one of its super types - // If not, this is a fetch for a subtype of the parent table group, which might be left joined - if ( attributeDeclaringType.isAssignableFrom( parentTableGroupType ) ) { - return SqlAstJoinType.INNER; - } - } - return SqlAstJoinType.LEFT; + else { + final Class attributeDeclaringType = declaringTableGroupProducer.getJavaType().getJavaTypeClass(); + final Class parentTableGroupType = parentTableGroup.getModelPart().getJavaType().getJavaTypeClass(); + // This attribute mapping must be declared on the parent table group type or one of its super types + // If not, this is a fetch for a subtype of the parent table group, which might be left joined + return attributeDeclaringType.isAssignableFrom( parentTableGroupType ) + ? SqlAstJoinType.INNER + : SqlAstJoinType.LEFT; } } @@ -1983,68 +2032,17 @@ public TableGroupJoin createTableGroupJoin( // If a parent is a collection part, there is no custom predicate and the join is INNER or LEFT // we check if this attribute is the map key property to reuse the existing index table group if ( !addsPredicate && ( joinType == SqlAstJoinType.INNER || joinType == SqlAstJoinType.LEFT ) ) { - TableGroup parentTableGroup = lhs; - ModelPartContainer parentContainer = lhs.getModelPart(); - StringBuilder embeddablePathSb = null; - // Traverse up embeddable table groups until we find a table group for a collection part - while ( !( parentContainer instanceof CollectionPart ) ) { - if ( parentContainer instanceof EmbeddableValuedModelPart ) { - if ( embeddablePathSb == null ) { - embeddablePathSb = new StringBuilder(); - } - embeddablePathSb.insert( 0, parentContainer.getPartName() + "." ); - final NavigablePath parentNavigablePath = parentTableGroup.getNavigablePath(); - final TableGroup tableGroup = fromClauseAccess.findTableGroup( parentNavigablePath.getParent() ); - if ( tableGroup == null ) { - assert parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.PART_NAME ) - || parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.TARGET_PART_NAME ); - // Might happen that we don't register a table group for the collection role if this is a - // foreign key part and the collection is delayed. We can just break out in this case though, - // since these checks here are only for reusing a map key property, which we won't have - break; - } - parentTableGroup = tableGroup; - parentContainer = tableGroup.getModelPart(); - } - else { - break; - } - } - - if ( CollectionPart.Nature.ELEMENT.getName().equals( parentTableGroup.getNavigablePath().getLocalName() ) ) { - final NavigablePath parentParentPath = parentTableGroup.getNavigablePath().getParent(); - final PluralTableGroup pluralTableGroup = (PluralTableGroup) fromClauseAccess.findTableGroup( parentParentPath ); - if ( pluralTableGroup != null ) { - final String indexPropertyName = pluralTableGroup.getModelPart() - .getIndexMetadata() - .getIndexPropertyName(); - final String pathName; - if ( embeddablePathSb != null ) { - pathName = embeddablePathSb.append( getAttributeName() ).toString(); - } - else { - pathName = getAttributeName(); - } - - if ( pathName.equals( indexPropertyName ) ) { - final TableGroup indexTableGroup = pluralTableGroup.getIndexTableGroup(); - // If this is the map key property, we can reuse the index table group - initializeIfNeeded( lhs, requestedJoinType, indexTableGroup ); - return new TableGroupJoin( - navigablePath, - joinType, - new MappedByTableGroup( - navigablePath, - this, - indexTableGroup, - fetched, - pluralTableGroup, - this - ), - null - ); - } - } + final TableGroupJoin tableGroupJoin = + createTableGroupJoin( + navigablePath, + lhs, + requestedJoinType, + fetched, + fromClauseAccess, + joinType + ); + if ( tableGroupJoin != null ) { + return tableGroupJoin; } } @@ -2114,7 +2112,7 @@ public TableGroupJoin createTableGroupJoin( // owning entity is updated, that is, to data loss. // But we let it apply on the TARGET side of a @OneToOne, and we apply // it whenever there is a dedicated join table. - if ( hasJoinTable || sideNature == ForeignKeyDescriptor.Nature.TARGET ) { + if ( canAddRestriction() ) { associatedEntityMappingType.applyWhereRestrictions( join::applyPredicate, tableGroup, @@ -2144,17 +2142,109 @@ public TableGroupJoin createTableGroupJoin( return join; } + private TableGroupJoin createTableGroupJoin( + NavigablePath navigablePath, + TableGroup lhs, + SqlAstJoinType requestedJoinType, + boolean fetched, + FromClauseAccess fromClauseAccess, + SqlAstJoinType joinType) { + + StringBuilder embeddablePath = null; + TableGroup parentTableGroup = lhs; + ModelPartContainer parentContainer = lhs.getModelPart(); + // Traverse up embeddable table groups until we find a table group for a collection part + while ( !( parentContainer instanceof CollectionPart ) ) { + if ( parentContainer instanceof EmbeddableValuedModelPart ) { + if ( embeddablePath == null ) { + embeddablePath = new StringBuilder(); + } + embeddablePath.insert( 0, parentContainer.getPartName() + "." ); + final NavigablePath parentNavigablePath = parentTableGroup.getNavigablePath(); + final TableGroup tableGroup = fromClauseAccess.findTableGroup( parentNavigablePath.getParent() ); + if ( tableGroup == null ) { + assert parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.PART_NAME ) + || parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.TARGET_PART_NAME ); + // Might happen that we don't register a table group for the collection role if this is a + // foreign key part and the collection is delayed. We can just break out in this case though, + // since these checks here are only for reusing a map key property, which we won't have + break; + } + parentTableGroup = tableGroup; + parentContainer = tableGroup.getModelPart(); + } + else { + break; + } + } + + return createTableGroupJoin( + navigablePath, + lhs, + requestedJoinType, + fetched, + fromClauseAccess, + joinType, + parentTableGroup, + embeddablePath + ); + } + + private TableGroupJoin createTableGroupJoin( + NavigablePath navigablePath, + TableGroup lhs, + SqlAstJoinType requestedJoinType, + boolean fetched, + FromClauseAccess fromClauseAccess, + SqlAstJoinType joinType, + TableGroup parentTableGroup, + StringBuilder embeddablePath) { + final NavigablePath parentGroupNavigablePath = parentTableGroup.getNavigablePath(); + if ( CollectionPart.Nature.ELEMENT.getName().equals( parentGroupNavigablePath.getLocalName() ) ) { + final NavigablePath parentParentPath = parentGroupNavigablePath.getParent(); + final PluralTableGroup pluralTableGroup = + (PluralTableGroup) fromClauseAccess.findTableGroup( parentParentPath ); + if ( pluralTableGroup != null ) { + final String indexPropertyName = + pluralTableGroup.getModelPart().getIndexMetadata().getIndexPropertyName(); + final String pathName = + embeddablePath == null + ? getAttributeName() + : embeddablePath.append( getAttributeName() ).toString(); + if ( pathName.equals( indexPropertyName ) ) { + final TableGroup indexTableGroup = pluralTableGroup.getIndexTableGroup(); + // If this is the map key property, we can reuse the index table group + initializeIfNeeded( lhs, requestedJoinType, indexTableGroup ); + return new TableGroupJoin( + navigablePath, + joinType, + new MappedByTableGroup( + navigablePath, + this, + indexTableGroup, + fetched, + pluralTableGroup, + this + ), + null + ); + } + } + } + return null; + } + @Override public SqlAstJoinType determineSqlJoinType(TableGroup lhs, @Nullable SqlAstJoinType requestedJoinType, boolean fetched) { if ( requestedJoinType != null ) { return requestedJoinType; } - - if ( fetched ) { + else if ( fetched ) { return getDefaultSqlAstJoinType( lhs ); } - - return SqlAstJoinType.INNER; + else { + return SqlAstJoinType.INNER; + } } @Override @@ -2174,48 +2264,19 @@ public LazyTableGroup createRootTableGroupJoin( creationState.getSqlAliasBaseGenerator() ); - final SoftDeleteMapping softDeleteMapping = getAssociatedEntityMappingType().getSoftDeleteMapping(); - final boolean canUseInnerJoin; - final SqlAstJoinType currentlyProcessingJoinType = - creationState instanceof SqmToSqlAstConverter sqmToSqlAstConverter - ? sqmToSqlAstConverter.getCurrentlyProcessingJoinType() - : null; - if ( currentlyProcessingJoinType != null && currentlyProcessingJoinType != SqlAstJoinType.INNER ) { - // Don't change the join type though, as that has implications for eager initialization of a LazyTableGroup - canUseInnerJoin = false; - } - else { - canUseInnerJoin = determineSqlJoinType( lhs, requestedJoinType, fetched ) == SqlAstJoinType.INNER; - } + final EntityMappingType associatedEntityMappingType = getAssociatedEntityMappingType(); + final SoftDeleteMapping softDeleteMapping = associatedEntityMappingType.getSoftDeleteMapping(); + final boolean canUseInnerJoin = canUseInnerJoin( lhs, requestedJoinType, fetched, creationState ); - TableGroup realParentTableGroup = lhs; - final FromClauseAccess fromClauseAccess = creationState.getFromClauseAccess(); - while ( realParentTableGroup.getModelPart() instanceof EmbeddableValuedModelPart ) { - final NavigablePath parentNavigablePath = realParentTableGroup.getNavigablePath(); - final TableGroup tableGroup = fromClauseAccess.findTableGroup( parentNavigablePath.getParent() ); - if ( tableGroup == null ) { - assert parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.PART_NAME ) - || parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.TARGET_PART_NAME ); - // Might happen that we don't register a table group for the collection role if this is a - // foreign key part and the collection is delayed. We can just break out in this case though, - // since the realParentTableGroup is only relevant if this association is actually joined, - // which it is not, because this is part of the target FK - realParentTableGroup = null; - break; - } - realParentTableGroup = tableGroup; - } + final TableGroup realParentTableGroup = realParentTableGroup( lhs, creationState ); - final TableGroupProducer tableGroupProducer; - if ( requestedJoinType != null && realParentTableGroup instanceof CorrelatedTableGroup ) { - // If the parent is a correlated table group, and we're explicitly joining, we can't refer to columns of the - // table in the outer query, because the context in which a column is used could be an aggregate function. - // Using a parent column in such a case would lead to an error if the parent query lacks a proper group by - tableGroupProducer = entityMappingType; - } - else { - tableGroupProducer = this; - } + // If the parent is a correlated table group, and we're explicitly joining, we can't refer to columns of the + // table in the outer query, because the context in which a column is used could be an aggregate function. + // Using a parent column in such a case would lead to an error if the parent query lacks a proper group by + final TableGroupProducer tableGroupProducer = + requestedJoinType != null && realParentTableGroup instanceof CorrelatedTableGroup + ? entityMappingType + : this; final LazyTableGroup lazyTableGroup = new LazyTableGroup( canUseInnerJoin, @@ -2233,21 +2294,19 @@ public LazyTableGroup createRootTableGroupJoin( tableGroupProducer, explicitSourceAlias, sqlAliasBase, - creationState.getCreationContext().getSessionFactory(), + associatedEntityMappingType.getEntityPersister().getFactory(), lhs ); if ( predicateConsumer != null ) { - final TableReference lhsTableReference = lhs.resolveTableReference( - navigablePath, - identifyingColumnsTableExpression - ); - + final TableReference lhsTableReference = + lhs.resolveTableReference( navigablePath, identifyingColumnsTableExpression ); + final boolean targetSide = sideNature == ForeignKeyDescriptor.Nature.TARGET; lazyTableGroup.setTableGroupInitializerCallback( tableGroup -> predicateConsumer.accept( foreignKeyDescriptor.generateJoinPredicate( - sideNature == ForeignKeyDescriptor.Nature.TARGET ? lhsTableReference : tableGroup.getPrimaryTableReference(), - sideNature == ForeignKeyDescriptor.Nature.TARGET ? tableGroup.getPrimaryTableReference() : lhsTableReference, + targetSide ? lhsTableReference : tableGroup.getPrimaryTableReference(), + targetSide ? tableGroup.getPrimaryTableReference() : lhsTableReference, creationState ) ) @@ -2257,7 +2316,7 @@ public LazyTableGroup createRootTableGroupJoin( // add the restriction final TableReference tableReference = lazyTableGroup.resolveTableReference( navigablePath, - getAssociatedEntityMappingType().getSoftDeleteTableDetails().getTableName() + associatedEntityMappingType.getSoftDeleteTableDetails().getTableName() ); predicateConsumer.accept( softDeleteMapping.createNonDeletedRestriction( tableReference, @@ -2277,24 +2336,62 @@ public LazyTableGroup createRootTableGroupJoin( return lazyTableGroup; } + private static TableGroup realParentTableGroup(TableGroup lhs, SqlAstCreationState creationState) { + TableGroup realParentTableGroup = lhs; + final FromClauseAccess fromClauseAccess = creationState.getFromClauseAccess(); + while ( realParentTableGroup.getModelPart() instanceof EmbeddableValuedModelPart ) { + final NavigablePath parentNavigablePath = realParentTableGroup.getNavigablePath(); + final TableGroup tableGroup = fromClauseAccess.findTableGroup( parentNavigablePath.getParent() ); + if ( tableGroup == null ) { + assert parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.PART_NAME ) + || parentNavigablePath.getLocalName().equals( ForeignKeyDescriptor.TARGET_PART_NAME ); + // Might happen that we don't register a table group for the collection role if this is a + // foreign key part and the collection is delayed. We can just break out in this case though, + // since the realParentTableGroup is only relevant if this association is actually joined, + // which it is not, because this is part of the target FK + realParentTableGroup = null; + break; + } + realParentTableGroup = tableGroup; + } + return realParentTableGroup; + } + + private boolean canUseInnerJoin( + TableGroup lhs, + SqlAstJoinType requestedJoinType, + boolean fetched, + SqlAstCreationState creationState) { + final SqlAstJoinType currentlyProcessingJoinType = + creationState instanceof SqmToSqlAstConverter sqmToSqlAstConverter + ? sqmToSqlAstConverter.getCurrentlyProcessingJoinType() + : null; + if ( currentlyProcessingJoinType == null || currentlyProcessingJoinType == SqlAstJoinType.INNER ) { + return determineSqlJoinType( lhs, requestedJoinType, fetched ) == SqlAstJoinType.INNER; + } + else { + // Don't change the join type though, as that has implications for eager initialization of a LazyTableGroup + return false; + } + } + @Override public boolean canUseParentTableGroup( TableGroupProducer producer, NavigablePath navigablePath, ValuedModelPart valuedModelPart) { return producer == this - && sideNature == ForeignKeyDescriptor.Nature.KEY - && foreignKeyDescriptor.isKeyPart( valuedModelPart ); + && sideNature == ForeignKeyDescriptor.Nature.KEY + && foreignKeyDescriptor.isKeyPart( valuedModelPart ); } private void initializeIfNeeded(TableGroup lhs, SqlAstJoinType sqlAstJoinType, TableGroup tableGroup) { if ( sqlAstJoinType == SqlAstJoinType.INNER && ( isNullable || !lhs.canUseInnerJoins() ) ) { if ( hasJoinTable ) { // Set the join type of the table reference join to INNER to retain cardinality expectation - final TableReference lhsTableReference = lhs.resolveTableReference( - tableGroup.getNavigablePath(), - identifyingColumnsTableExpression - ); + final TableReference lhsTableReference = + lhs.resolveTableReference( tableGroup.getNavigablePath(), + identifyingColumnsTableExpression ); final List tableReferenceJoins = lhs.getTableReferenceJoins(); for ( int i = 0; i < tableReferenceJoins.size(); i++ ) { final TableReferenceJoin tableReferenceJoin = tableReferenceJoins.get( i ); @@ -2336,11 +2433,9 @@ public TableGroup createTableGroupInternal( String sourceAlias, final SqlAliasBase sqlAliasBase, SqlAstCreationState creationState) { - final TableReference primaryTableReference = getEntityMappingType().createPrimaryTableReference( - sqlAliasBase, - creationState - ); - + final EntityMappingType entityMappingType = getEntityMappingType(); + final TableReference primaryTableReference = + entityMappingType.createPrimaryTableReference( sqlAliasBase, creationState ); return new StandardTableGroup( canUseInnerJoins, navigablePath, @@ -2350,14 +2445,14 @@ public TableGroup createTableGroupInternal( primaryTableReference, true, sqlAliasBase, - getEntityMappingType().getRootEntityDescriptor()::containsTableReference, - (tableExpression, tg) -> getEntityMappingType().createTableReferenceJoin( + entityMappingType.getRootEntityDescriptor()::containsTableReference, + (tableExpression, tg) -> entityMappingType.createTableReferenceJoin( tableExpression, sqlAliasBase, primaryTableReference, creationState ), - creationState.getCreationContext().getSessionFactory() + entityMappingType.getEntityPersister().getFactory() ); } @@ -2435,17 +2530,17 @@ protected Object extractValue(Object domainValue, SharedSessionContractImplement if ( domainValue == null ) { return null; } - - if ( referencedPropertyName != null ) { - domainValue = lazyInitialize( domainValue ); - assert getAssociatedEntityMappingType() - .getRepresentationStrategy() - .getInstantiator() - .isInstance( domainValue ); - return extractAttributePathValue( domainValue, getAssociatedEntityMappingType(), referencedPropertyName ); + else { + if ( referencedPropertyName != null ) { + final Object initializedValue = lazyInitialize( domainValue ); + final EntityMappingType mappingType = getAssociatedEntityMappingType(); + assert mappingType.getRepresentationStrategy().getInstantiator().isInstance( initializedValue ); + return extractAttributePathValue( initializedValue, mappingType, referencedPropertyName ); + } + else { + return foreignKeyDescriptor.getAssociationKeyFromSide( domainValue, sideNature.inverse(), session ); + } } - - return foreignKeyDescriptor.getAssociationKeyFromSide( domainValue, sideNature.inverse(), session ); } /** @@ -2454,43 +2549,34 @@ assert getAssociatedEntityMappingType() */ protected Object lazyInitialize(Object domainValue) { final LazyInitializer lazyInitializer = HibernateProxy.extractLazyInitializer( domainValue ); - if ( lazyInitializer != null ) { - return lazyInitializer.getImplementation(); - } - return domainValue; + return lazyInitializer == null ? domainValue : lazyInitializer.getImplementation(); } protected static Object extractAttributePathValue(Object domainValue, EntityMappingType entityType, String attributePath) { - if ( ! attributePath.contains( "." ) ) { - return entityType.findAttributeMapping( attributePath ).getValue( domainValue ); + if ( attributePath.contains( "." ) ) { + Object value = domainValue; + ManagedMappingType managedType = entityType; + for ( String part : split( ".", attributePath ) ) { + assert managedType != null; + final AttributeMapping attributeMapping = managedType.findAttributeMapping( part ); + value = attributeMapping.getValue( value ); + managedType = + attributeMapping.getMappedType() instanceof ManagedMappingType managedMappingType + ? managedMappingType + : null; + } + return value; } - - Object value = domainValue; - ManagedMappingType managedType = entityType; - final String[] pathParts = StringHelper.split( ".", attributePath ); - for ( int i = 0; i < pathParts.length; i++ ) { - assert managedType != null; - - final String pathPart = pathParts[ i ]; - final AttributeMapping attributeMapping = managedType.findAttributeMapping( pathPart ); - value = attributeMapping.getValue( value ); - managedType = - attributeMapping.getMappedType() instanceof ManagedMappingType managedMappingType - ? managedMappingType - : null; + else { + return entityType.findAttributeMapping( attributePath ).getValue( domainValue ); } - - return value; } @Override public int forEachSelectable(int offset, SelectableConsumer consumer) { - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - return foreignKeyDescriptor.visitKeySelectables( offset, consumer ); - } - else { - return 0; - } + return sideNature == ForeignKeyDescriptor.Nature.KEY + ? foreignKeyDescriptor.visitKeySelectables( offset, consumer ) + : 0; } @Override @@ -2499,7 +2585,8 @@ public void applySqlSelections( TableGroup tableGroup, DomainResultCreationState creationState) { if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - foreignKeyDescriptor.getKeyPart().applySqlSelections( navigablePath, tableGroup, creationState ); + foreignKeyDescriptor.getKeyPart() + .applySqlSelections( navigablePath, tableGroup, creationState ); } } @@ -2510,31 +2597,23 @@ public void applySqlSelections( DomainResultCreationState creationState, BiConsumer selectionConsumer) { if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - foreignKeyDescriptor.getKeyPart().applySqlSelections( - navigablePath, - tableGroup, - creationState, - selectionConsumer - ); + foreignKeyDescriptor.getKeyPart() + .applySqlSelections( navigablePath, tableGroup, creationState, selectionConsumer ); } } @Override public String getContainingTableExpression() { - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - return foreignKeyDescriptor.getKeyTable(); - } - else { - return foreignKeyDescriptor.getTargetTable(); - } + return sideNature == ForeignKeyDescriptor.Nature.KEY + ? foreignKeyDescriptor.getKeyTable() + : foreignKeyDescriptor.getTargetTable(); } @Override public int getJdbcTypeCount() { - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - return foreignKeyDescriptor.getJdbcTypeCount(); - } - return 0; + return sideNature == ForeignKeyDescriptor.Nature.KEY + ? foreignKeyDescriptor.getJdbcTypeCount() + : 0; } @Override @@ -2544,10 +2623,9 @@ public JdbcMapping getJdbcMapping(final int index) { @Override public SelectableMapping getSelectable(int columnIndex) { - if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { - return foreignKeyDescriptor.getSelectable( columnIndex ); - } - return null; + return sideNature == ForeignKeyDescriptor.Nature.KEY + ? foreignKeyDescriptor.getSelectable( columnIndex ) + : null; } @Override @@ -2565,19 +2643,13 @@ public Object disassemble(Object value, SharedSessionContractImplementor session @Override public void addToCacheKey(MutableCacheKeyBuilder cacheKey, Object value, SharedSessionContractImplementor session) { - final Object cacheValue; - // the value may come from a database snapshot, in this case it corresponds to the value of the key and can be - // added to the cache key - if ( value != null && foreignKeyDescriptor.getJavaType().getJavaTypeClass() == value.getClass() ) { - cacheValue = value; - } - else { - cacheValue = foreignKeyDescriptor.getAssociationKeyFromSide( - value, - sideNature.inverse(), - session - ); - } + // the value may come from a database snapshot, + // in this case it corresponds to the value of + // the key and can be added to the cache key + final Object cacheValue = + value != null && foreignKeyDescriptor.getJavaType().getJavaTypeClass() == value.getClass() + ? value + : foreignKeyDescriptor.getAssociationKeyFromSide( value, sideNature.inverse(), session ); foreignKeyDescriptor.addToCacheKey( cacheKey, cacheValue, session ); }