Fix loading OnetoOne self referencing entity

hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSqlAstCreationState.java

@@ -148,8 +148,13 @@ public class LoaderSqlAstCreationState
 	}
 
 	@Override
-	public void registerVisitedAssociationKey(AssociationKey associationKey) {
-		visitedAssociationKeys.add( associationKey );
+	public boolean registerVisitedAssociationKey(AssociationKey associationKey) {
+		return visitedAssociationKeys.add( associationKey );
+	}
+
+	@Override
+	public void removeVisitedAssociationKey(AssociationKey associationKey) {
+		visitedAssociationKeys.remove( associationKey );
 	}
 
 	@Override

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

@@ -425,10 +425,21 @@ public class EntityCollectionPart
 			boolean selected,
 			String resultVariable,
 			DomainResultCreationState creationState) {
-		creationState.registerVisitedAssociationKey( getForeignKeyDescriptor().getAssociationKey() );
+		final boolean added = creationState.registerVisitedAssociationKey( getForeignKeyDescriptor().getAssociationKey() );
 
 		final TableGroup partTableGroup = resolveTableGroup( fetchablePath, creationState );
-		return new EntityFetchJoinedImpl( fetchParent, this, partTableGroup, selected, fetchablePath, creationState );
+		final EntityFetchJoinedImpl fetch = new EntityFetchJoinedImpl(
+				fetchParent,
+				this,
+				partTableGroup,
+				selected,
+				fetchablePath,
+				creationState
+		);
+		if ( added ) {
+			creationState.removeVisitedAssociationKey( getForeignKeyDescriptor().getAssociationKey() );
+		}
+		return fetch;
 	}
 
 	private TableGroup resolveTableGroup(NavigablePath fetchablePath, DomainResultCreationState creationState) {

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

@@ -378,26 +378,37 @@ public class PluralAttributeMappingImpl
 			DomainResultCreationState creationState) {
 		final SqlAstCreationState sqlAstCreationState = creationState.getSqlAstCreationState();
 
-		creationState.registerVisitedAssociationKey( fkDescriptor.getAssociationKey() );
+		final boolean added = creationState.registerVisitedAssociationKey( fkDescriptor.getAssociationKey() );
 
-		if ( fetchTiming == FetchTiming.IMMEDIATE ) {
-			if ( selected ) {
-				final TableGroup collectionTableGroup = resolveCollectionTableGroup(
-						fetchParent,
-						fetchablePath,
-						creationState,
-						sqlAstCreationState
-				);
+		try {
+			if ( fetchTiming == FetchTiming.IMMEDIATE ) {
+				if ( selected ) {
+					final TableGroup collectionTableGroup = resolveCollectionTableGroup(
+							fetchParent,
+							fetchablePath,
+							creationState,
+							sqlAstCreationState
+					);
 
-				return new EagerCollectionFetch(
-						fetchablePath,
-						this,
-						collectionTableGroup,
-						fetchParent,
-						creationState
-				);
+					return new EagerCollectionFetch(
+							fetchablePath,
+							this,
+							collectionTableGroup,
+							fetchParent,
+							creationState
+					);
+				}
+				else {
+					return createSelectEagerCollectionFetch(
+							fetchParent,
+							fetchablePath,
+							creationState,
+							sqlAstCreationState
+					);
+				}
 			}
-			else {
+
+			if ( getCollectionDescriptor().getCollectionType().hasHolder() ) {
 				return createSelectEagerCollectionFetch(
 						fetchParent,
 						fetchablePath,
@@ -405,13 +416,16 @@ public class PluralAttributeMappingImpl
 						sqlAstCreationState
 				);
 			}
-		}
 
-		if ( getCollectionDescriptor().getCollectionType().hasHolder() ) {
-			return createSelectEagerCollectionFetch( fetchParent, fetchablePath, creationState, sqlAstCreationState );
+			return createDelayedCollectionFetch( fetchParent, fetchablePath, creationState, sqlAstCreationState );
+		}
+		finally {
+			// This is only necessary because the association key is too general i.e. also matching FKs that other associations would match
+			// and on top of this, we are not handling circular fetches for plural attributes yet
+			if ( added ) {
+				creationState.removeVisitedAssociationKey( fkDescriptor.getAssociationKey() );
+			}
 		}
-
-		return createDelayedCollectionFetch( fetchParent, fetchablePath, creationState, sqlAstCreationState );
 	}
 
 	private Fetch createSelectEagerCollectionFetch(

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

@@ -572,9 +572,15 @@ public class ToOneAttributeMapping
 	public void setForeignKeyDescriptor(ForeignKeyDescriptor foreignKeyDescriptor) {
 		assert identifyingColumnsTableExpression != null;
 		this.foreignKeyDescriptor = foreignKeyDescriptor;
-		this.sideNature = foreignKeyDescriptor.getAssociationKey().getTable().equals( identifyingColumnsTableExpression )
-				? ForeignKeyDescriptor.Nature.KEY
-				: ForeignKeyDescriptor.Nature.TARGET;
+		if ( cardinality == Cardinality.ONE_TO_ONE && bidirectionalAttributeName != null ) {
+			this.sideNature = ForeignKeyDescriptor.Nature.TARGET;
+		}
+		else {
+			this.sideNature = foreignKeyDescriptor.getAssociationKey().getTable().equals(
+					identifyingColumnsTableExpression )
+					? ForeignKeyDescriptor.Nature.KEY
+					: ForeignKeyDescriptor.Nature.TARGET;
+		}
 
 		// We can only use the parent table group if the FK is located there and ignoreNotFound is false
 		// If this is not the case, the FK is not constrained or on a join/secondary table, so we need a join
@@ -738,36 +744,35 @@ public class ToOneAttributeMapping
 
 				We have a circularity but it is not bidirectional
 			 */
-			if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) {
-				final TableGroup parentTableGroup = creationState
-						.getSqlAstCreationState()
-						.getFromClauseAccess()
-						.getTableGroup( fetchParent.getNavigablePath() );
-				final DomainResult<?> foreignKeyDomainResult;
-				assert !creationState.isResolvingCircularFetch();
-				try {
-					creationState.setResolvingCircularFetch( true );
-					foreignKeyDomainResult = foreignKeyDescriptor.createKeyDomainResult(
-							fetchablePath,
-							parentTableGroup,
-							creationState
-					);
-				}
-				finally {
-					creationState.setResolvingCircularFetch( false );
-				}
-				return new CircularFetchImpl(
-						this,
-						getEntityMappingType(),
-						fetchTiming,
+			final TableGroup parentTableGroup = creationState
+					.getSqlAstCreationState()
+					.getFromClauseAccess()
+					.getTableGroup( fetchParent.getNavigablePath() );
+			final DomainResult<?> foreignKeyDomainResult;
+			assert !creationState.isResolvingCircularFetch();
+			try {
+				creationState.setResolvingCircularFetch( true );
+				foreignKeyDomainResult = foreignKeyDescriptor.createDomainResult(
 						fetchablePath,
-						fetchParent,
-						this,
-						isSelectByUniqueKey( sideNature ),
-						fetchablePath,
-						foreignKeyDomainResult
+						parentTableGroup,
+						sideNature,
+						creationState
 				);
 			}
+			finally {
+				creationState.setResolvingCircularFetch( false );
+			}
+			return new CircularFetchImpl(
+					this,
+					getEntityMappingType(),
+					fetchTiming,
+					fetchablePath,
+					fetchParent,
+					this,
+					isSelectByUniqueKey( sideNature ),
+					fetchablePath,
+					foreignKeyDomainResult
+			);
 		}
 		return null;
 	}
@@ -998,6 +1003,7 @@ public class ToOneAttributeMapping
 				|| fetchParent.getNavigablePath() instanceof TreatedNavigablePath
 				&& parentNavigablePath.equals( fetchParent.getNavigablePath().getRealParent() );
 
+
 		if ( fetchTiming == FetchTiming.IMMEDIATE && selected ) {
 			final TableGroup tableGroup;
 			if ( fetchParent instanceof EntityResultJoinedSubclassImpl &&
@@ -1034,15 +1040,17 @@ public class ToOneAttributeMapping
 				);
 			}
 
-			creationState.registerVisitedAssociationKey( foreignKeyDescriptor.getAssociationKey() );
+			final boolean added = creationState.registerVisitedAssociationKey( foreignKeyDescriptor.getAssociationKey() );
+			AssociationKey additionalAssociationKey = null;
 			if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE && bidirectionalAttributeName != null ) {
 				final ModelPart bidirectionalModelPart = entityMappingType.findSubPart( bidirectionalAttributeName );
 				// Add the inverse association key side as well to be able to resolve to a CircularFetch
 				if ( bidirectionalModelPart instanceof ToOneAttributeMapping ) {
 					final ToOneAttributeMapping bidirectionalAttribute = (ToOneAttributeMapping) bidirectionalModelPart;
-					creationState.registerVisitedAssociationKey(
-							bidirectionalAttribute.getForeignKeyDescriptor().getAssociationKey()
-					);
+					final AssociationKey secondKey = bidirectionalAttribute.getForeignKeyDescriptor().getAssociationKey();
+					if ( creationState.registerVisitedAssociationKey( secondKey ) ) {
+						additionalAssociationKey = secondKey;
+					}
 				}
 			}
 			final EntityFetchJoinedImpl entityFetchJoined = new EntityFetchJoinedImpl(
@@ -1053,6 +1061,12 @@ public class ToOneAttributeMapping
 					fetchablePath,
 					creationState
 			);
+			if ( added ) {
+				creationState.removeVisitedAssociationKey( foreignKeyDescriptor.getAssociationKey() );
+			}
+			if ( additionalAssociationKey != null ) {
+				creationState.removeVisitedAssociationKey( additionalAssociationKey );
+			}
 			return entityFetchJoined;
 		}
 

hibernate-core/src/main/java/org/hibernate/query/results/DomainResultCreationStateImpl.java

@@ -175,7 +175,8 @@ public class DomainResultCreationStateImpl
 	}
 
 	@Override
-	public void registerVisitedAssociationKey(AssociationKey associationKey) {
+	public boolean registerVisitedAssociationKey(AssociationKey associationKey) {
+		return false;
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/sql/results/graph/DomainResultCreationState.java

@@ -31,7 +31,11 @@ public interface DomainResultCreationState {
 		return (SqlAliasBaseManager) getSqlAstCreationState().getSqlAliasBaseGenerator();
 	}
 
-	default void registerVisitedAssociationKey(AssociationKey associationKey){
+	default boolean registerVisitedAssociationKey(AssociationKey associationKey) {
+		return false;
+	}
+
+	default void removeVisitedAssociationKey(AssociationKey associationKey){
 	}
 
 	default boolean isAssociationKeyVisited(AssociationKey associationKey){

hibernate-core/src/test/java/org/hibernate/orm/test/constraint/ForeignKeyConstraintTest.java

@@ -24,6 +24,7 @@ import jakarta.persistence.Embeddable;
 import jakarta.persistence.Embedded;
 import jakarta.persistence.EmbeddedId;
 import jakarta.persistence.Entity;
+import jakarta.persistence.FetchType;
 import jakarta.persistence.ForeignKey;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.GenerationType;
@@ -252,7 +253,7 @@ public class ForeignKeyConstraintTest {
 		@jakarta.persistence.Column( name = "MATRICULATION_NUMBER" )
 		public String matriculationNumber;
 
-		@ManyToOne
+		@ManyToOne(fetch = FetchType.LAZY)
 		@JoinColumns(
 				value = {
 						@JoinColumn( name = "CAR_NR", referencedColumnName = "CAR_NR" ),
@@ -262,7 +263,7 @@ public class ForeignKeyConstraintTest {
 		)
 		public Car car;
 
-		@ManyToOne
+		@ManyToOne(fetch = FetchType.LAZY)
 		@JoinColumns(
 				value = {
 						@JoinColumn( name = "CAR_NR2", referencedColumnName = "CAR_NR" ),
@@ -272,7 +273,7 @@ public class ForeignKeyConstraintTest {
 		)
 		public Car car2;
 
-		@OneToOne
+		@OneToOne(fetch = FetchType.LAZY)
 		@JoinColumns(
 				value = {
 						@JoinColumn( name = "CAR_NR3", referencedColumnName = "CAR_NR" ),
@@ -282,7 +283,7 @@ public class ForeignKeyConstraintTest {
 		)
 		public Car car3;
 
-		@OneToOne
+		@OneToOne(fetch = FetchType.LAZY)
 		@JoinColumns(
 				value = {
 						@JoinColumn( name = "CAR_NR4", referencedColumnName = "CAR_NR" ),
@@ -372,19 +373,19 @@ public class ForeignKeyConstraintTest {
 
 		public String color;
 
-		@ManyToOne
+		@ManyToOne(fetch = FetchType.LAZY)
 		@JoinColumn( name = "OWNER_PERSON_ID", foreignKey = @ForeignKey( name = "FK_CAR_OWNER") )
 		public Person owner;
 
-		@ManyToOne
+		@ManyToOne(fetch = FetchType.LAZY)
 		@JoinColumn( name = "OWNER_PERSON_ID2", foreignKey = @ForeignKey( name = "FK_CAR_OWNER2", value = ConstraintMode.NO_CONSTRAINT ) )
 		public Person owner2;
 
-		@OneToOne
+		@OneToOne(fetch = FetchType.LAZY)
 		@JoinColumn( name = "OWNER_PERSON_ID3", foreignKey = @ForeignKey( name = "FK_CAR_OWNER3") )
 		public Person owner3;
 
-		@OneToOne
+		@OneToOne(fetch = FetchType.LAZY)
 		@JoinColumn( name = "OWNER_PERSON_ID4", foreignKey = @ForeignKey( name = "FK_CAR_OWNER4", value = ConstraintMode.NO_CONSTRAINT ) )
 		public Person owner4;
 

hibernate-core/src/test/java/org/hibernate/orm/test/loading/LoadParentChildEntityTest.java

@@ -0,0 +1,98 @@
+package org.hibernate.orm.test.loading;
+
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.Id;
+import jakarta.persistence.OneToOne;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+@DomainModel(
+		annotatedClasses = {
+				LoadParentChildEntityTest.ContainingEntity.class,
+		}
+)
+@SessionFactory
+public class LoadParentChildEntityTest {
+
+	@Test
+	public void testLoad(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					ContainingEntity parent = new ContainingEntity();
+					parent.setId( 1 );
+
+					ContainingEntity child = new ContainingEntity();
+					child.setId( 2 );
+
+					parent.setChild( child );
+
+					child.setParent( parent );
+
+					session.persist( parent );
+					session.persist( child );
+
+					assertThat( parent.getChild() ).isNotNull();
+				}
+		);
+
+		scope.inTransaction(
+				session -> {
+					ContainingEntity load = session.load( ContainingEntity.class, 1 );
+					assertThat( load.getChild() ).isNotNull();
+					assertThat( load.getParent() ).isNull();
+				}
+		);
+
+		scope.inTransaction(
+				session -> {
+					ContainingEntity load = session.load( ContainingEntity.class, 2 );
+					assertThat( load.getParent() ).isNotNull();
+					assertThat( load.getChild() ).isNull();
+				}
+		);
+	}
+
+
+	@Entity(name = "containing")
+	public static class ContainingEntity {
+
+		@Id
+		private Integer id;
+
+		@OneToOne
+		private ContainingEntity parent;
+
+		@OneToOne(mappedBy = "parent")
+		private ContainingEntity child;
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public ContainingEntity getParent() {
+			return parent;
+		}
+
+		public void setParent(ContainingEntity parent) {
+			this.parent = parent;
+		}
+
+		public ContainingEntity getChild() {
+			return child;
+		}
+
+		public void setChild(ContainingEntity child) {
+			this.child = child;
+		}
+
+	}
+}

hibernate-core/src/test/java/org/hibernate/orm/test/sql/exec/onetoone/bidirectional/EntityWithBidirectionalOneToOneTest.java

@@ -76,7 +76,6 @@ public class EntityWithBidirectionalOneToOneTest {
 	}
 
 	@Test
-	@FailureExpected( reason = "Change to attribute ordering triggered a problem with circularity detection", jiraKey = "HHH-14403" )
 	public void testGetMother(SessionFactoryScope scope) {
 		SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector();
 		statementInspector.clear();
@@ -167,8 +166,9 @@ public class EntityWithBidirectionalOneToOneTest {
 			assertSame( adoptedChild.getStepMother(), mother );
 			assertSame( adoptedChild.getBiologicalMother(), mother );
 
-			statementInspector.assertExecutedCount( 1 );
+			statementInspector.assertExecutedCount( 2 );
 			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 4 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 5 );
 		} );
 	}
 
@@ -230,7 +230,7 @@ public class EntityWithBidirectionalOneToOneTest {
 
 			statementInspector.assertExecutedCount( 2 );
 			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 4);
-			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 4);
+			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 5);
 		} );
 	}
 
@@ -280,12 +280,60 @@ public class EntityWithBidirectionalOneToOneTest {
 				fetchablePath Child.mother.adopted.biologicalMother.adopted.stepMother --- Circular ---
 				fetchablePath Child.mother.adopted.stepMother --- Circular --
 			 */
-			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 4 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
+		} );
+	}
+
+	@Test
+	public void testGetChild3(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					Mother mother = new Mother( 10, "Strange mom" );
+					session.save( mother );
+					session.get( AdoptedChild.class, 3 ).setBiologicalMother( mother );
+				}
+		);
+		SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector();
+		statementInspector.clear();
+		scope.inTransaction( session -> {
+			final Child child = session.get( Child.class, 2 );
+
+			Mother mother = child.getMother();
+			assertTrue(
+					Hibernate.isInitialized( mother ),
+					"The mother eager OneToOne association is not initialized"
+			);
+			assertThat( mother, notNullValue() );
+			assertThat( mother.getName(), is( "Giulia" ) );
+
+			Child biologicalChild = mother.getBiologicalChild();
+			assertSame( biologicalChild, child );
+			assertTrue(
+					Hibernate.isInitialized( biologicalChild ),
+					"The child eager OneToOne association is not initialized"
+			);
+
+			AdoptedChild adoptedChild = mother.getAdopted();
+			assertThat( adoptedChild, notNullValue() );
+			assertTrue(
+					Hibernate.isInitialized( adoptedChild ),
+					"The adoptedChild eager OneToOne association is not initialized"
+			);
+			assertSame( adoptedChild.getStepMother(), mother );
+			assertThat( adoptedChild.getBiologicalMother(), notNullValue() );
+			assertTrue(
+					Hibernate.isInitialized( adoptedChild.getBiologicalMother() ),
+					"The biologicalMother eager OneToOne association is not initialized"
+			);
+			assertThat( adoptedChild.getBiologicalMother().getName(), is( "Strange mom" ) );
+
+			statementInspector.assertExecutedCount( 2 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 5 );
 		} );
 	}
 
 	@Test
-	@FailureExpected( reason = "Change to attribute ordering triggered a problem with circularity detection", jiraKey = "HHH-14403" )
 	public void testGetAdoptedChild(SessionFactoryScope scope) {
 		SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector();
 		statementInspector.clear();
@@ -315,7 +363,7 @@ public class EntityWithBidirectionalOneToOneTest {
 			);
 			assertThat( adoptedChild.getBiologicalMother(), nullValue() );
 
-			statementInspector.assertExecutedCount( 2 );
+			statementInspector.assertExecutedCount( 1 );
 			/*
 				fetchablePath: AdoptedChild.biologicalMother --- NO circular ---
 				fetchablePath: AdoptedChild.biologicalMother.biologicalChild --- NO circular ---
@@ -334,8 +382,7 @@ public class EntityWithBidirectionalOneToOneTest {
 				fetchablePath: AdoptedChild.stepMother.biologicalChild.mother --- Circular ---
 				fetchablePath: AdoptedChild.stepMother.adopted --- Circular ---
 			 */
-			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
-			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 4 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 5 );
 		} );
 	}
 
@@ -401,14 +448,14 @@ public class EntityWithBidirectionalOneToOneTest {
 
 			assertThat( biologicalMother.getAdopted(), nullValue() );
 
-			statementInspector.assertExecutedCount( 2 );
-			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 4 );
-			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 4 );
+			statementInspector.assertExecutedCount( 3 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 5 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 2, "join", 3 );
 		} );
 	}
 
 	@Test
-	@FailureExpected( reason = "Change to attribute ordering triggered a problem with circularity detection", jiraKey = "HHH-14403" )
 	public void testHqlSelectMother(SessionFactoryScope scope) {
 		SQLStatementInspector statementInspector = (SQLStatementInspector) scope.getStatementInspector();
 		statementInspector.clear();
@@ -424,12 +471,10 @@ public class EntityWithBidirectionalOneToOneTest {
 					Child child = mother.getBiologicalChild();
 					assertThat( child, notNullValue() );
 					assertThat( child.getName(), is( "Luis" ) );
-					statementInspector.assertExecutedCount( 3 );
+					statementInspector.assertExecutedCount( 2 );
 					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 );
 					// Mother.biologicalChild
-					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 4 );
-					// Mother.adopted
-					statementInspector.assertNumberOfOccurrenceInQuery( 2, "join", 3 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 5 );
 				}
 		);
 	}