From 63eedee7a2800b0f9258779591a0c74cf65ac09c Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Wed, 26 Jul 2023 16:37:26 +0200 Subject: [PATCH] HHH-16759 When ComponentType is immutable, use instantiator instead of setting property values --- .../InFlightMetadataCollectorImpl.java | 13 ++ .../boot/model/internal/AnyBinder.java | 1 + .../boot/model/internal/CollectionBinder.java | 1 + .../boot/model/internal/EmbeddableBinder.java | 2 +- .../model/internal/OneToOneSecondPass.java | 2 + .../OptionalDeterminationSecondPass.java | 13 ++ .../boot/model/internal/PropertyBinder.java | 34 +++- .../java/org/hibernate/mapping/ManyToOne.java | 5 + .../internal/ToOneAttributeMapping.java | 177 ++++++++++++------ .../sql/ast/tree/from/TableReferenceJoin.java | 2 +- .../org/hibernate/type/ComponentType.java | 40 ++-- .../org/hibernate/type/CompositeType.java | 2 +- .../java/org/hibernate/type/TypeHelper.java | 19 +- .../org/hibernate/type/UserComponentType.java | 5 +- ...ssociationsOneOfWhichIsAJoinTableTest.java | 2 +- .../records/MergeRecordPropertyTestCase.java | 91 ++++++++- 16 files changed, 301 insertions(+), 108 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/boot/model/internal/OptionalDeterminationSecondPass.java diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java index 6bd7033691..d03a4a9bbd 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/InFlightMetadataCollectorImpl.java @@ -48,6 +48,7 @@ import org.hibernate.boot.model.internal.FkSecondPass; import org.hibernate.boot.model.internal.IdGeneratorResolverSecondPass; import org.hibernate.boot.model.internal.JPAIndexHolder; +import org.hibernate.boot.model.internal.OptionalDeterminationSecondPass; import org.hibernate.boot.model.internal.QuerySecondPass; import org.hibernate.boot.model.internal.SecondaryTableFromAnnotationSecondPass; import org.hibernate.boot.model.internal.SecondaryTableSecondPass; @@ -1668,6 +1669,7 @@ public Join locateJoin(Identifier tableName) { private ArrayList implicitColumnNamingSecondPassList; private ArrayList generalSecondPassList; + private ArrayList optionalDeterminationSecondPassList; @Override public void addSecondPass(SecondPass secondPass) { @@ -1706,6 +1708,9 @@ else if ( secondPass instanceof QuerySecondPass ) { else if ( secondPass instanceof ImplicitColumnNamingSecondPass ) { addImplicitColumnNamingSecondPass( (ImplicitColumnNamingSecondPass) secondPass ); } + else if ( secondPass instanceof OptionalDeterminationSecondPass ) { + addOptionalDeterminationSecondPass( (OptionalDeterminationSecondPass) secondPass ); + } else { // add to the general SecondPass list if ( generalSecondPassList == null ) { @@ -1787,6 +1792,13 @@ private void addImplicitColumnNamingSecondPass(ImplicitColumnNamingSecondPass se implicitColumnNamingSecondPassList.add( secondPass ); } + private void addOptionalDeterminationSecondPass(OptionalDeterminationSecondPass secondPass) { + if ( optionalDeterminationSecondPassList == null ) { + optionalDeterminationSecondPassList = new ArrayList<>(); + } + optionalDeterminationSecondPassList.add( secondPass ); + } + private boolean inSecondPass = false; @@ -1813,6 +1825,7 @@ public void processSecondPasses(MetadataBuildingContext buildingContext) { processSecondPasses( querySecondPassList ); processSecondPasses( generalSecondPassList ); + processSecondPasses( optionalDeterminationSecondPassList ); processPropertyReferences(); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java index 6ea2964e27..44243b9f62 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnyBinder.java @@ -119,6 +119,7 @@ private static void bindAny( } binder.setAccessType( inferredData.getDefaultAccess() ); binder.setCascade( cascadeStrategy ); + binder.setBuildingContext( context ); Property prop = binder.makeProperty(); //composite FK columns are in the same table, so it's OK propertyHolder.addProperty( prop, columns, inferredData.getDeclaringClass() ); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java index 850a69c3ab..44c9085276 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/CollectionBinder.java @@ -1287,6 +1287,7 @@ private void bindProperty() { binder.setProperty( property ); binder.setInsertable( insertable ); binder.setUpdatable( updatable ); + binder.setBuildingContext( buildingContext ); Property prop = binder.makeProperty(); //we don't care about the join stuffs because the column is on the association table. if ( !declaringClassSet ) { diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java index 4e70685df5..6dc5c14c40 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/EmbeddableBinder.java @@ -193,7 +193,7 @@ private static Component bindEmbeddable( entityBinder, isComponentEmbedded, isIdentifierMapper, - false, + context.getMetadataCollector().isInSecondPass(), customInstantiatorImpl, compositeUserTypeClass, annotatedColumns, diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java index e90383c396..56c2abd530 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OneToOneSecondPass.java @@ -112,6 +112,7 @@ public void doSecondPass(Map persistentClasses) throws binder.setValue( value ); binder.setCascade( cascadeStrategy ); binder.setAccessType( inferredData.getDefaultAccess() ); + binder.setBuildingContext( buildingContext ); final LazyGroup lazyGroupAnnotation = property.getAnnotation( LazyGroup.class ); if ( lazyGroupAnnotation != null ) { @@ -186,6 +187,7 @@ private void bindTargetManyToOne( manyToOne.setFetchMode( oneToOne.getFetchMode() ); manyToOne.setLazy( oneToOne.isLazy() ); manyToOne.setReferencedEntityName( oneToOne.getReferencedEntityName() ); + manyToOne.setReferencedPropertyName( mappedBy ); manyToOne.setUnwrapProxy( oneToOne.isUnwrapProxy() ); manyToOne.markAsLogicalOneToOne(); property.setValue( manyToOne ); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OptionalDeterminationSecondPass.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OptionalDeterminationSecondPass.java new file mode 100644 index 0000000000..393f8544bf --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/OptionalDeterminationSecondPass.java @@ -0,0 +1,13 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html. + */ +package org.hibernate.boot.model.internal; + + +import org.hibernate.boot.spi.SecondPass; + +public interface OptionalDeterminationSecondPass extends SecondPass { +} diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java index ce48d1701a..d3c4521d92 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/PropertyBinder.java @@ -48,13 +48,16 @@ import org.hibernate.boot.spi.AccessType; import org.hibernate.boot.spi.MetadataBuildingContext; import org.hibernate.boot.spi.PropertyData; +import org.hibernate.boot.spi.SecondPass; import org.hibernate.engine.OptimisticLockStyle; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.mapping.Collection; import org.hibernate.mapping.Component; import org.hibernate.mapping.GeneratorCreator; +import org.hibernate.mapping.Join; import org.hibernate.mapping.KeyValue; import org.hibernate.mapping.MappedSuperclass; +import org.hibernate.mapping.PersistentClass; import org.hibernate.mapping.Property; import org.hibernate.mapping.RootClass; import org.hibernate.mapping.SimpleValue; @@ -448,13 +451,32 @@ private void handleMutability(Property property) { private void handleOptional(Property property) { if ( this.property != null ) { - property.setOptional( !isId && isOptional( this.property ) && isNullable( property ) ); - } - } + property.setOptional( !isId && isOptional( this.property ) ); + if ( property.isOptional() ) { + final OptionalDeterminationSecondPass secondPass = persistentClasses -> { + // Defer determining whether a property and its columns are nullable, + // as handleOptional might be called when the value is not yet fully initialized + if ( property.getPersistentClass() != null ) { + for ( Join join : property.getPersistentClass().getJoins() ) { + if ( join.getProperties().contains( property ) ) { + // If this property is part of a join it is inherently optional + return; + } + } + } - private static boolean isNullable(Property property) { - final Value value = property.getValue(); - return value instanceof org.hibernate.mapping.OneToMany || value.isNullable(); + if ( !property.getValue().isNullable() ) { + property.setOptional( false ); + } + }; + // Always register this as second pass and never execute it directly, + // even if we are in a second pass already. + // If we are in a second pass, then we are currently processing the generalSecondPassList + // to which the following call will add the second pass to, + // so it will be executed within that second pass, just a bit later + buildingContext.getMetadataCollector().addSecondPass( secondPass ); + } + } } private void handleNaturalId(Property property) { 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 d46e432841..6e527d4b4d 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java +++ b/hibernate-core/src/main/java/org/hibernate/mapping/ManyToOne.java @@ -143,4 +143,9 @@ public void markAsLogicalOneToOne() { public boolean isLogicalOneToOne() { return isLogicalOneToOne; } + + @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 87b7a5b6aa..118ef6a9b9 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 @@ -9,11 +9,13 @@ import java.util.Arrays; import java.util.HashSet; import java.util.Iterator; +import java.util.List; import java.util.Set; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; +import org.hibernate.AssertionFailure; import org.hibernate.LockMode; import org.hibernate.annotations.NotFoundAction; import org.hibernate.cache.MutableCacheKeyBuilder; @@ -77,6 +79,7 @@ import org.hibernate.sql.ast.tree.from.TableGroupJoinProducer; import org.hibernate.sql.ast.tree.from.TableGroupProducer; import org.hibernate.sql.ast.tree.from.TableReference; +import org.hibernate.sql.ast.tree.from.TableReferenceJoin; import org.hibernate.sql.ast.tree.predicate.Predicate; import org.hibernate.sql.results.graph.DomainResult; import org.hibernate.sql.results.graph.DomainResultCreationState; @@ -145,6 +148,7 @@ public class Entity1 { private final Set targetKeyPropertyNames; private final Cardinality cardinality; + private final boolean hasJoinTable; /* Capture the other side's name of a possibly bidirectional association to allow resolving circular fetches. It may be null if the referenced property is a non-entity. @@ -171,6 +175,7 @@ protected ToOneAttributeMapping(ToOneAttributeMapping original) { referencedPropertyName = original.referencedPropertyName; targetKeyPropertyName = original.targetKeyPropertyName; cardinality = original.cardinality; + hasJoinTable = original.hasJoinTable; bidirectionalAttributePath = original.bidirectionalAttributePath; declaringTableGroupProducer = original.declaringTableGroupProducer; isKeyTableNullable = original.isKeyTableNullable; @@ -242,7 +247,10 @@ public ToOneAttributeMapping( this.entityMappingType = entityMappingType; this.navigableRole = navigableRole; - this.declaringTableGroupProducer = resolveDeclaringTableGroupProducer( declaringEntityPersister, navigableRole ); + this.declaringTableGroupProducer = resolveDeclaringTableGroupProducer( + declaringEntityPersister, + navigableRole + ); if ( bootValue instanceof ManyToOne ) { final ManyToOne manyToOne = (ManyToOne) bootValue; this.notFoundAction = ( (ManyToOne) bootValue ).getNotFoundAction(); @@ -260,6 +268,7 @@ public ToOneAttributeMapping( ? name : bootValue.getPropertyName(); 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() ) @@ -267,7 +276,10 @@ public ToOneAttributeMapping( && join.getTable() == manyToOne.getTable() && equal( join.getKey(), manyToOne ) ) { //noinspection deprecation - bidirectionalAttributeName = SelectablePath.parse( join.getPropertyIterator().next().getName() ); + bidirectionalAttributeName = SelectablePath.parse( + join.getPropertyIterator().next().getName() + ); + hasJoinTable = true; break; } } @@ -280,8 +292,10 @@ && equal( join.getKey(), manyToOne ) ) { entityBinding.getPropertyClosure() ); } + this.hasJoinTable = hasJoinTable; } else { + this.hasJoinTable = false; bidirectionalAttributeName = findBidirectionalOneToManyAttributeName( propertyPath, declaringType, @@ -294,7 +308,12 @@ && equal( join.getKey(), manyToOne ) ) { 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.bidirectionalAttributePath = property != null && property.getValue() instanceof EntityType + this.hasJoinTable = cardinality == Cardinality.LOGICAL_ONE_TO_ONE + && property != null + && ( property.getValue() instanceof ManyToOne ) + && ( (ManyToOne) property.getValue() ).isLogicalOneToOne(); + this.bidirectionalAttributePath = property != null && property.getValue() + .getType() instanceof EntityType ? SelectablePath.parse( referencedPropertyName ) : null; } @@ -302,12 +321,20 @@ && equal( join.getKey(), manyToOne ) ) { isKeyTableNullable = true; } else { - final String targetTableName = MappingModelCreationHelper.getTableIdentifierExpression( manyToOne.getTable(), declaringEntityPersister.getFactory() ); + final String targetTableName = MappingModelCreationHelper.getTableIdentifierExpression( + manyToOne.getTable(), + declaringEntityPersister.getFactory() + ); 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 ) ); + navigableRole.getParent() + .getParent() + .getFullPath() + .substring( declaringEntityPersister.getNavigableRole() + .getFullPath() + .length() + 1 ) ); assert pluralAttribute != null; final AbstractCollectionPersister persister = (AbstractCollectionPersister) pluralAttribute.getCollectionDescriptor(); @@ -328,6 +355,7 @@ && equal( join.getKey(), manyToOne ) ) { else { assert bootValue instanceof OneToOne; cardinality = Cardinality.ONE_TO_ONE; + hasJoinTable = false; /* The otherSidePropertyName value is used to determine bidirectionality based on the navigablePath string @@ -381,7 +409,7 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does } notFoundAction = null; isKeyTableNullable = isNullable(); - isOptional = ! bootValue.isConstrained(); + isOptional = !bootValue.isConstrained(); isInternalLoadNullable = isNullable(); } @@ -436,67 +464,78 @@ the navigable path is NavigablePath(Card.fields.{element}.{id}.card) and it does } this.targetKeyPropertyNames = targetKeyPropertyNames; } - else if ( bootValue.isReferenceToPrimaryKey() ) { - this.targetKeyPropertyName = referencedPropertyName; - final Set targetKeyPropertyNames = new HashSet<>( 2 ); - addPrefixedPropertyNames( - targetKeyPropertyNames, - targetKeyPropertyName, - bootValue.getType(), - declaringEntityPersister.getFactory() - ); - this.targetKeyPropertyNames = targetKeyPropertyNames; - } else { final PersistentClass entityBinding = bootValue.getBuildingContext().getMetadataCollector() .getEntityBinding( entityMappingType.getEntityName() ); final Type propertyType = entityBinding.getRecursiveProperty( referencedPropertyName ).getType(); - final CompositeType compositeType; - if ( propertyType.isComponentType() && ( compositeType = (CompositeType) propertyType ).isEmbedded() - && compositeType.getPropertyNames().length == 1 ) { - final Set targetKeyPropertyNames = new HashSet<>( 2 ); - this.targetKeyPropertyName = compositeType.getPropertyNames()[0]; - addPrefixedPropertyPaths( + if ( bootValue.isReferenceToPrimaryKey() ) { + this.targetKeyPropertyName = referencedPropertyName; + final Set targetKeyPropertyNames = new HashSet<>( 3 ); + addPrefixedPropertyNames( targetKeyPropertyNames, targetKeyPropertyName, - compositeType.getSubtypes()[0], + propertyType, declaringEntityPersister.getFactory() ); addPrefixedPropertyNames( targetKeyPropertyNames, - EntityIdentifierMapping.ID_ROLE_NAME, - propertyType, + null, + bootValue.getType(), declaringEntityPersister.getFactory() ); this.targetKeyPropertyNames = targetKeyPropertyNames; } else { - final Set targetKeyPropertyNames = new HashSet<>( 2 ); - this.targetKeyPropertyName = referencedPropertyName; - final String mapsIdAttributeName; - // 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 ) { + final CompositeType compositeType; + if ( propertyType.isComponentType() && ( compositeType = (CompositeType) propertyType ).isEmbedded() + && compositeType.getPropertyNames().length == 1 ) { + final Set targetKeyPropertyNames = new HashSet<>( 2 ); + this.targetKeyPropertyName = compositeType.getPropertyNames()[0]; addPrefixedPropertyPaths( targetKeyPropertyNames, - mapsIdAttributeName, - entityMappingType.getEntityPersister().getIdentifierType(), + targetKeyPropertyName, + compositeType.getSubtypes()[0], declaringEntityPersister.getFactory() ); + addPrefixedPropertyNames( + targetKeyPropertyNames, + EntityIdentifierMapping.ID_ROLE_NAME, + propertyType, + declaringEntityPersister.getFactory() + ); + this.targetKeyPropertyNames = targetKeyPropertyNames; + } + else { + final Set targetKeyPropertyNames = new HashSet<>( 2 ); + this.targetKeyPropertyName = referencedPropertyName; + final String mapsIdAttributeName; + // 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 ) { + addPrefixedPropertyPaths( + targetKeyPropertyNames, + mapsIdAttributeName, + entityMappingType.getEntityPersister().getIdentifierType(), + declaringEntityPersister.getFactory() + ); + } + addPrefixedPropertyNames( + targetKeyPropertyNames, + targetKeyPropertyName, + propertyType, + declaringEntityPersister.getFactory() + ); + addPrefixedPropertyNames( + targetKeyPropertyNames, + ForeignKeyDescriptor.PART_NAME, + propertyType, + declaringEntityPersister.getFactory() + ); + this.targetKeyPropertyNames = targetKeyPropertyNames; } - addPrefixedPropertyNames( - targetKeyPropertyNames, - targetKeyPropertyName, - propertyType, - declaringEntityPersister.getFactory() - ); - addPrefixedPropertyNames( - targetKeyPropertyNames, - ForeignKeyDescriptor.PART_NAME, - propertyType, - declaringEntityPersister.getFactory() - ); - this.targetKeyPropertyNames = targetKeyPropertyNames; } } } @@ -629,6 +668,7 @@ private ToOneAttributeMapping( this.targetKeyPropertyName = original.targetKeyPropertyName; this.targetKeyPropertyNames = original.targetKeyPropertyNames; this.cardinality = original.cardinality; + this.hasJoinTable = original.hasJoinTable; this.bidirectionalAttributePath = original.bidirectionalAttributePath; this.declaringTableGroupProducer = declaringTableGroupProducer; this.isInternalLoadNullable = original.isInternalLoadNullable; @@ -729,7 +769,7 @@ else if ( identifierOrUniqueKeyType instanceof EmbeddedComponentType ) { final String newFkPrefix; if ( prefix == null ) { newPrefix = propertyName; - newPkPrefix = propertyName + "." + EntityIdentifierMapping.ID_ROLE_NAME; + newPkPrefix = EntityIdentifierMapping.ID_ROLE_NAME; newFkPrefix = ForeignKeyDescriptor.PART_NAME; } else if ( propertyName == null ) { @@ -891,9 +931,8 @@ public Fetch resolveCircularFetch( FetchTiming fetchTiming, DomainResultCreationState creationState) { final AssociationKey associationKey = foreignKeyDescriptor.getAssociationKey(); - - if ( creationState.isAssociationKeyVisited( associationKey ) - || bidirectionalAttributePath != null && !creationState.isRegisteringVisitedAssociationKeys() ) { + final boolean associationKeyVisited = creationState.isAssociationKeyVisited( associationKey ); + if ( associationKeyVisited || bidirectionalAttributePath != null ) { NavigablePath parentNavigablePath = fetchablePath.getParent(); assert parentNavigablePath.equals( fetchParent.getNavigablePath() ); // The parent navigable path is {fk} if we are creating the domain result for the foreign key for a circular fetch @@ -952,6 +991,12 @@ public class PrimaryKey { ); } + if ( !associationKeyVisited && creationState.isRegisteringVisitedAssociationKeys() ) { + // If the current association key hasn't been visited yet and we are registering keys, + // then there can't be a circular fetch + return null; + } + /* class Child { @OneToOne @@ -2069,8 +2114,34 @@ public boolean canUseParentTableGroup( private void initializeIfNeeded(TableGroup lhs, SqlAstJoinType sqlAstJoinType, TableGroup tableGroup) { if ( sqlAstJoinType == SqlAstJoinType.INNER && ( isNullable || !lhs.canUseInnerJoins() ) ) { - // Force initialization of the underlying table group join to retain cardinality - tableGroup.getPrimaryTableReference(); + 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 List tableReferenceJoins = lhs.getTableReferenceJoins(); + for ( int i = 0; i < tableReferenceJoins.size(); i++ ) { + final TableReferenceJoin tableReferenceJoin = tableReferenceJoins.get( i ); + if ( tableReferenceJoin.getJoinType() != SqlAstJoinType.INNER + && tableReferenceJoin.getJoinedTableReference() == lhsTableReference ) { + tableReferenceJoins.set( + i, + new TableReferenceJoin( + true, + tableReferenceJoin.getJoinedTableReference(), + tableReferenceJoin.getPredicate() + ) + ); + return; + } + } + throw new AssertionFailure( "Couldn't find table reference join for join table: " + lhsTableReference ); + } + else { + // Force initialization of the underlying table group join to retain cardinality + tableGroup.getPrimaryTableReference(); + } } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java index eb53c19750..a0c4e6a3ab 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableReferenceJoin.java @@ -55,7 +55,7 @@ public void accept(SqlAstWalker sqlTreeWalker) { @Override public String toString() { - return getJoinType().getText() + " join " + getJoinedTableReference().toString(); + return getJoinType().getText() + "join " + getJoinedTableReference().toString(); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java b/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java index e0da518f9f..300e8c3f51 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/ComponentType.java @@ -496,17 +496,12 @@ public Object replace( Object owner, Map copyCache) { - if ( !isMutable() ) { - return original; - } - if ( original == null ) { + if ( original == null && target == null ) { return null; } - //if ( original == target ) return target; final Object[] originalValues = getPropertyValues( original ); - final Object[] resultValues = target == null ? new Object[originalValues.length] : getPropertyValues( target ); - + final Object[] resultValues = getPropertyValues( target ); final Object[] replacedValues = TypeHelper.replace( originalValues, resultValues, @@ -516,7 +511,7 @@ public Object replace( copyCache ); - if ( target == null ) { + if ( target == null || !isMutable() ) { return instantiator().instantiate( () -> replacedValues, session.getSessionFactory() ); } else { @@ -534,24 +529,12 @@ public Object replace( Map copyCache, ForeignKeyDirection foreignKeyDirection) { - if ( !isMutable() ) { - return original; - } - if ( original == null ) { + if ( original == null && target == null ) { return null; } - //if ( original == target ) return target; final Object[] originalValues = getPropertyValues( original ); - final Object[] resultValues; - - if ( target == null ) { - resultValues = new Object[originalValues.length]; - } - else { - resultValues = getPropertyValues( target ); - } - + final Object[] resultValues = getPropertyValues( target ); final Object[] replacedValues = TypeHelper.replace( originalValues, resultValues, @@ -562,7 +545,7 @@ public Object replace( foreignKeyDirection ); - if ( target == null ) { + if ( target == null || !isMutable() ) { return instantiator().instantiate( () -> replacedValues, session.getSessionFactory() ); } else { @@ -770,7 +753,7 @@ private ValueExtractor jdbcValueExtractor() { return embeddableTypeDescriptor().getAggregateMapping().getJdbcMapping().getJdbcValueExtractor(); } - private EmbeddableInstantiator instantiator() { + protected final EmbeddableInstantiator instantiator() { return embeddableTypeDescriptor().getRepresentationStrategy().getInstantiator(); } @@ -805,4 +788,13 @@ public EmbeddableValuedModelPart mappingModelPart() { } return mappingModelPart; } + + @Override + public Object replacePropertyValues(Object component, Object[] values, SharedSessionContractImplementor session) + throws HibernateException { + if ( !isMutable() ) { + return instantiator().instantiate( () -> values, session.getSessionFactory() ); + } + return CompositeTypeImplementor.super.replacePropertyValues( component, values, session ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/type/CompositeType.java b/hibernate-core/src/main/java/org/hibernate/type/CompositeType.java index 3a7b606a2d..f3aa9893e3 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/CompositeType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/CompositeType.java @@ -103,7 +103,7 @@ Object getPropertyValue(Object component, int index, SharedSessionContractImplem * * @param component The component instance * @param values The values to inject - * @return A new instance is necessary + * @return A new instance as necessary * * @throws HibernateException Indicates an issue performing the injection * diff --git a/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java b/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java index 8cbb386f21..88652f4b1d 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/type/TypeHelper.java @@ -193,17 +193,16 @@ public static Object[] replaceAssociations( final Type type = types[i]; if ( type.isComponentType() ) { final CompositeType compositeType = (CompositeType) type; - // need to extract the component values and check for subtype replacements... - final Object[] objects = - replaceCompositeAssociations( - session, - copyCache, - foreignKeyDirection, - target[i], - currentOriginal, - compositeType - ); if ( target[i] != null ) { + // need to extract the component values and check for subtype replacements... + final Object[] objects = replaceCompositeAssociations( + session, + copyCache, + foreignKeyDirection, + target[i], + currentOriginal, + compositeType + ); target[i] = compositeType.replacePropertyValues( target[i], objects, session ); } copied[i] = target[i]; diff --git a/hibernate-core/src/main/java/org/hibernate/type/UserComponentType.java b/hibernate-core/src/main/java/org/hibernate/type/UserComponentType.java index 531c140cdf..73363ed7ed 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/UserComponentType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/UserComponentType.java @@ -91,9 +91,6 @@ public Object assemble(Serializable object, SharedSessionContractImplementor ses @Override public Object replacePropertyValues(Object component, Object[] values, SharedSessionContractImplementor session) throws HibernateException { - return getMappingModelPart() - .getEmbeddableTypeDescriptor() - .getRepresentationStrategy() - .getInstantiator().instantiate( () -> values, session.getSessionFactory() ); + return instantiator().instantiate( () -> values, session.getSessionFactory() ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java index b477446ef5..56f23af3cc 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/onetoone/bidirectional/EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest.java @@ -177,7 +177,7 @@ public void testHqlSelectParent(SessionFactoryScope scope) { .setParameter( "id", 1 ) .getSingleResult(); statementInspector.assertExecutedCount( 2 ); - statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 2 ); + statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 ); statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 3 ); Male son = parent.getSon(); assertThat( son, CoreMatchers.notNullValue() ); diff --git a/hibernate-core/src/test/java17/org/hibernate/orm/test/records/MergeRecordPropertyTestCase.java b/hibernate-core/src/test/java17/org/hibernate/orm/test/records/MergeRecordPropertyTestCase.java index 53fc8f39a1..e90b59c4a8 100644 --- a/hibernate-core/src/test/java17/org/hibernate/orm/test/records/MergeRecordPropertyTestCase.java +++ b/hibernate-core/src/test/java17/org/hibernate/orm/test/records/MergeRecordPropertyTestCase.java @@ -4,14 +4,17 @@ import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import jakarta.persistence.Embeddable; import jakarta.persistence.Embedded; import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*; @DomainModel(annotatedClasses = { MergeRecordPropertyTestCase.MyEntity.class @@ -20,6 +23,16 @@ @JiraKey("HHH-16759") public class MergeRecordPropertyTestCase { + @AfterEach + protected void cleanupTest(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + session.createQuery( "update MyEntity e set e.record.assoc = null" ).executeUpdate(); + session.createQuery( "delete from MyEntity" ).executeUpdate(); + } + ); + } + @Test public void mergeDetached(SessionFactoryScope scope) { scope.inTransaction( @@ -38,24 +51,53 @@ public void mergeDetached(SessionFactoryScope scope) { } @Test - public void mergeTransient(SessionFactoryScope scope) { + public void mergeDetachedNullComponent(SessionFactoryScope scope) { scope.inTransaction( - session -> session.merge( new MyEntity( 2L, new MyRecord( "test", "xyz" ) ) ) + session -> session.persist( new MyEntity( 1L, new MyRecord( "test", "abc" ) ) ) + ); + scope.inTransaction( + session -> session.merge( new MyEntity( 1L ) ) ); scope.inSession( session -> { - final MyEntity entity = session.find( MyEntity.class, 2L ); + final MyEntity entity = session.find( MyEntity.class, 1L ); + assertNull( entity.record ); + } + ); + } + + @Test + public void mergeTransient(SessionFactoryScope scope) { + scope.inTransaction( + session -> session.merge( new MyEntity( 1L, new MyRecord( "test", "xyz" ) ) ) + ); + scope.inSession( + session -> { + final MyEntity entity = session.find( MyEntity.class, 1L ); assertEquals( "test", entity.record.name ); assertEquals( "xyz", entity.record.description ); } ); } + @Test + public void mergeTransientNullComponent(SessionFactoryScope scope) { + scope.inTransaction( + session -> session.merge( new MyEntity( 1L ) ) + ); + scope.inSession( + session -> { + final MyEntity entity = session.find( MyEntity.class, 1L ); + assertNull( entity.record ); + } + ); + } + @Test public void mergePersistent(SessionFactoryScope scope) { scope.inTransaction( session -> { - final MyEntity entity = new MyEntity( 3L, new MyRecord( "test", "efg" ) ); + final MyEntity entity = new MyEntity( 1L, new MyRecord( "test", "efg" ) ); session.persist( entity ); entity.setRecord( new MyRecord( "test", "h" ) ); session.merge( entity ); @@ -63,13 +105,41 @@ public void mergePersistent(SessionFactoryScope scope) { ); scope.inSession( session -> { - final MyEntity entity = session.find( MyEntity.class, 3L ); + final MyEntity entity = session.find( MyEntity.class, 1L ); assertEquals( "test", entity.record.name ); assertEquals( "h", entity.record.description ); } ); } + @Test + public void mergePersistentDetached(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final MyEntity entity = new MyEntity( 1L, new MyRecord( "test", "abc" ) ); + session.persist( entity ); + session.flush(); + session.clear(); + final MyEntity entity2 = new MyEntity( 2L, new MyRecord( "test", "efg", new MyEntity( 1L ) ) ); + final MyEntity mergedEntity = session.merge( entity2 ); + assertTrue( session.contains( mergedEntity.record.assoc() ) ); + } + ); + } + + @Test + public void mergePersistentManaged(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final MyEntity entity = new MyEntity( 1L, new MyRecord( "test", "abc" ) ); + session.persist( entity ); + final MyEntity entity2 = new MyEntity( 1L, new MyRecord( "test", "efg", new MyEntity( 1L ) ) ); + final MyEntity mergedEntity = session.merge( entity2 ); + assertTrue( session.contains( mergedEntity.record.assoc() ) ); + } + ); + } + @Entity(name = "MyEntity") public static class MyEntity { @Id @@ -80,6 +150,10 @@ public static class MyEntity { public MyEntity() { } + public MyEntity(Long id) { + this.id = id; + } + public MyEntity(Long id, MyRecord record) { this.id = id; this.record = record; @@ -103,6 +177,9 @@ public void setRecord(MyRecord record) { } @Embeddable - public static record MyRecord(String name, String description) { + public static record MyRecord(String name, String description, @ManyToOne(fetch = FetchType.LAZY) MyEntity assoc) { + public MyRecord(String name, String description) { + this( name, description, null ); + } } }