Fix bidirectional fetching issues

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

@@ -729,7 +729,7 @@ public class LoaderSelectBuilder {
 
 			FetchTiming fetchTiming = fetchable.getMappedFetchOptions().getTiming();
 			boolean joined = fetchable.getMappedFetchOptions().getStyle() == FetchStyle.JOIN;
-
+			boolean explicitFetch = false;
 			EntityGraphTraversalState.TraversalResult traversalResult = null;
 
 			if ( !( fetchable instanceof CollectionPart ) ) {
@@ -738,6 +738,7 @@ public class LoaderSelectBuilder {
 					traversalResult = entityGraphTraversalState.traverse( fetchParent, fetchable, isKeyFetchable );
 					fetchTiming = traversalResult.getFetchTiming();
 					joined = traversalResult.isJoined();
+					explicitFetch = true;
 				}
 				else if ( loadQueryInfluencers.hasEnabledFetchProfiles() ) {
 					// There is no point in checking the fetch profile if it can't affect this fetchable
@@ -753,6 +754,7 @@ public class LoaderSelectBuilder {
 							if ( profileFetch != null ) {
 								fetchTiming = FetchTiming.IMMEDIATE;
 								joined = joined || profileFetch.getStyle() == org.hibernate.engine.profile.Fetch.Style.JOIN;
+								explicitFetch = true;
 							}
 						}
 					}
@@ -796,7 +798,8 @@ public class LoaderSelectBuilder {
 					fetchDepth++;
 				}
 
-				if ( !creationState.isResolvingCircularFetch() ) {
+				// There is no need to check for circular fetches if this is an explicit fetch
+				if ( !explicitFetch && !creationState.isResolvingCircularFetch() ) {
 					final Fetch biDirectionalFetch = fetchable.resolveCircularFetch(
 							fetchablePath,
 							fetchParent,

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityDiscriminatorMapping.java

@@ -13,7 +13,6 @@ import org.hibernate.sql.ast.spi.SqlAstCreationState;
 import org.hibernate.sql.ast.tree.expression.Expression;
 import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.results.graph.DomainResultCreationState;
-import org.hibernate.sql.results.graph.Fetch;
 import org.hibernate.sql.results.graph.FetchOptions;
 import org.hibernate.sql.results.graph.FetchParent;
 import org.hibernate.sql.results.graph.basic.BasicFetch;
@@ -77,13 +76,4 @@ public interface EntityDiscriminatorMapping extends VirtualModelPart, BasicValue
 		return FetchTiming.IMMEDIATE;
 	}
 
-	@Override
-	default Fetch resolveCircularFetch(
-			NavigablePath fetchablePath,
-			FetchParent fetchParent,
-			FetchTiming fetchTiming,
-			DomainResultCreationState creationState) {
-		// can never be circular
-		return null;
-	}
 }

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/PluralAttributeMapping.java

@@ -9,6 +9,7 @@ package org.hibernate.metamodel.mapping;
 import java.util.function.Consumer;
 
 import org.hibernate.loader.ast.spi.Loadable;
+import org.hibernate.metamodel.mapping.internal.ToOneAttributeMapping;
 import org.hibernate.metamodel.mapping.ordering.OrderByFragment;
 import org.hibernate.persister.collection.CollectionPersister;
 import org.hibernate.query.NavigablePath;
@@ -60,7 +61,7 @@ public interface PluralAttributeMapping
 
 	String getSeparateCollectionTable();
 
-	boolean isBidirectionalAttributeName(NavigablePath fetchablePath);
+	boolean isBidirectionalAttributeName(NavigablePath fetchablePath, ToOneAttributeMapping modelPart);
 
 	@Override
 	default boolean incrementFetchDepth(){

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

@@ -196,16 +196,6 @@ public class EmbeddedAttributeMapping
 		return navigableRole;
 	}
 
-	@Override
-	public Fetch resolveCircularFetch(
-			NavigablePath fetchablePath,
-			FetchParent fetchParent,
-			FetchTiming fetchTiming,
-			DomainResultCreationState creationState) {
-		// an embeddable can never be circular
-		return null;
-	}
-
 	@Override
 	public Fetch generateFetch(
 			FetchParent fetchParent,

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

@@ -134,15 +134,6 @@ public class EntityCollectionPart
 		return this;
 	}
 
-	@Override
-	public Fetch resolveCircularFetch(
-			NavigablePath fetchablePath,
-			FetchParent fetchParent,
-			FetchTiming fetchTiming,
-			DomainResultCreationState creationState) {
-		return null;
-	}
-
 	@Override
 	public EntityFetch generateFetch(
 			FetchParent fetchParent,

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

@@ -104,7 +104,6 @@ public class PluralAttributeMappingImpl
 	private final FetchStyle fetchStyle;
 
 	private final String bidirectionalAttributeName;
-	private final Boolean isInverse;
 
 	private final CollectionPersister collectionDescriptor;
 	private final String separateCollectionTable;
@@ -184,8 +183,6 @@ public class PluralAttributeMappingImpl
 
 		this.bidirectionalAttributeName = StringHelper.subStringNullIfEmpty( bootDescriptor.getMappedByProperty(), '.');
 
-		this.isInverse = bootDescriptor.isInverse();
-
 		this.sqlAliasStem = SqlAliasStemHelper.INSTANCE.generateStemFromAttributeName( attributeName );
 
 		if ( bootDescriptor.isOneToMany() ) {
@@ -232,14 +229,13 @@ public class PluralAttributeMappingImpl
 	}
 
 	@Override
-	public boolean isBidirectionalAttributeName(NavigablePath fetchablePath) {
+	public boolean isBidirectionalAttributeName(NavigablePath fetchablePath, ToOneAttributeMapping modelPart) {
-		if ( isInverse ) {
-			return true;
-		}
 		if ( bidirectionalAttributeName == null ) {
-			return false;
+			// If the FK-target of the to-one mapping is the same as the FK-target of this plural mapping,
+			// then we say this is bidirectional, given that this is only invoked for model parts of the collection elements
+			return fkDescriptor.getTargetPart() == modelPart.getForeignKeyDescriptor().getTargetPart();
 		}
-		return fetchablePath.getFullPath().endsWith( bidirectionalAttributeName );
+		return fetchablePath.getUnaliasedLocalName().endsWith( bidirectionalAttributeName );
 	}
 
 	@SuppressWarnings("unused")

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

@@ -24,7 +24,6 @@ import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
 import org.hibernate.mapping.ToOne;
 import org.hibernate.metamodel.mapping.AssociationKey;
-import org.hibernate.metamodel.mapping.AttributeMapping;
 import org.hibernate.metamodel.mapping.CollectionPart;
 import org.hibernate.metamodel.mapping.EntityAssociationMapping;
 import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
@@ -474,34 +473,21 @@ public class ToOneAttributeMapping
 				return null;
 			}
 
-			ModelPart modelPart = creationState.resolveModelPart( parentNavigablePath );
+			ModelPart parentModelPart = creationState.resolveModelPart( parentNavigablePath );
-			if ( modelPart instanceof EmbeddedIdentifierMappingImpl ) {
+			if ( parentModelPart instanceof EmbeddedIdentifierMappingImpl ) {
 				while ( parentNavigablePath instanceof EntityIdentifierNavigablePath ) {
 					parentNavigablePath = parentNavigablePath.getParent();
+					assert parentNavigablePath != null;
+					parentModelPart = creationState.resolveModelPart( parentNavigablePath );
 				}
 			}
-			while ( modelPart instanceof EmbeddableValuedFetchable ) {
+			while ( parentModelPart instanceof EmbeddableValuedFetchable ) {
 				parentNavigablePath = parentNavigablePath.getParent();
 				assert parentNavigablePath != null;
-				modelPart = creationState.resolveModelPart( parentNavigablePath );
+				parentModelPart = creationState.resolveModelPart( parentNavigablePath );
 			}
 
-			if ( isBidirectionalAttributeName( parentNavigablePath ) ) {
+			if ( isBidirectionalAttributeName( parentNavigablePath, parentModelPart, fetchablePath, creationState ) ) {
-				/*
-					class Child {
-						@OneToOne(mappedBy = "biologicalChild")
-						private Mother mother;
-					}
-
-					class Mother {
-						@OneToOne
-						private Child biologicalChild;
-					}
-
-					fetchablePath= Mother.biologicalChild.mother
-					this.mappedBy = "biologicalChild"
-					parent.getFullPath() = "Mother.biologicalChild"
-				 */
 				return createCircularBiDirectionalFetch(
 						fetchablePath,
 						fetchParent,
@@ -510,39 +496,6 @@ public class ToOneAttributeMapping
 				);
 			}
 
-			/*
-				check if mappedBy is on the other side of the association
-			 */
-			final boolean isBiDirectional = isBidirectional(
-					modelPart,
-					parentNavigablePath.getParent(),
-					fetchablePath,
-					creationState
-			);
-			if ( isBiDirectional ) {
-					/*
-						class Child {
-							@OneToOne(mappedBy = "biologicalChild")
-							private Mother mother;
-						}
-
-						class Mother {
-							@OneToOne
-							private Child biologicalChild;
-						}
-
-						fetchablePath = "Child.mother.biologicalChild"
-						otherSideAssociationModelPart = ToOneAttributeMapping("Child.mother")
-						otherSideMappedBy = "biologicalChild"
-
-					 */
-				return createCircularBiDirectionalFetch(
-						fetchablePath,
-						fetchParent,
-						parentNavigablePath,
-						LockMode.READ
-				);
-			}
 			/*
 						class Child {
 							@OneToOne
@@ -554,7 +507,7 @@ public class ToOneAttributeMapping
 							private Child stepMother;
 						}
 
-				We have a cirularity but it is not bidirectional
+				We have a circularity but it is not bidirectional
 			 */
 			if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) {
 				final TableGroup parentTableGroup = creationState
@@ -581,6 +534,7 @@ public class ToOneAttributeMapping
 						fetchablePath,
 						fetchParent,
 						this,
+						isSelectByUniqueKey( sideNature ),
 						fetchablePath,
 						foreignKeyDomainResult
 				);
@@ -589,35 +543,95 @@ public class ToOneAttributeMapping
 		return null;
 	}
 
-	private boolean isBidirectional(
+	protected boolean isBidirectionalAttributeName(
-			ModelPart modelPart,
+			NavigablePath parentNavigablePath,
-			NavigablePath parentOfParent,
+			ModelPart parentModelPart,
 			NavigablePath fetchablePath,
 			DomainResultCreationState creationState) {
-		if ( modelPart instanceof ToOneAttributeMapping ) {
+		if ( bidirectionalAttributeName == null ) {
-			return ( (ToOneAttributeMapping) modelPart ).isBidirectionalAttributeName( fetchablePath );
+			/*
+				check if mappedBy is on the other side of the association
+			 */
+
+			/*
+				class Child {
+					@OneToOne(mappedBy = "biologicalChild")
+					private Mother mother;
 				}
 
-		if ( modelPart instanceof PluralAttributeMapping ) {
+				class Mother {
-			return ( (PluralAttributeMapping) modelPart ).isBidirectionalAttributeName( fetchablePath );
+					@OneToOne
+					private Child biologicalChild;
 				}
 
-		if ( modelPart instanceof EntityCollectionPart ) {
+				fetchablePath = "Child.mother.biologicalChild"
+				otherSideAssociationModelPart = ToOneAttributeMapping("Child.mother")
+				otherSideMappedBy = "biologicalChild"
+
+			 */
+			if ( parentModelPart instanceof ToOneAttributeMapping ) {
+				final ToOneAttributeMapping toOneAttributeMapping = (ToOneAttributeMapping) parentModelPart;
+				if ( toOneAttributeMapping.bidirectionalAttributeName != null ) {
+					return toOneAttributeMapping.isBidirectionalAttributeName(
+							fetchablePath,
+							this,
+							parentNavigablePath,
+							creationState
+					);
+				}
+			}
+			else if ( parentModelPart instanceof PluralAttributeMapping ) {
+				return ( (PluralAttributeMapping) parentModelPart ).isBidirectionalAttributeName( fetchablePath, this );
+			}
+			else if ( parentModelPart instanceof EntityCollectionPart ) {
+				NavigablePath parentOfParent = parentNavigablePath.getParent();
 				if ( parentOfParent instanceof EntityIdentifierNavigablePath ) {
 					parentOfParent = parentOfParent.getParent();
 				}
-			return ( (PluralAttributeMapping) creationState.resolveModelPart( parentOfParent ) ).isBidirectionalAttributeName(
+				return ( (PluralAttributeMapping) creationState.resolveModelPart( parentOfParent ) )
-					fetchablePath );
+						.isBidirectionalAttributeName( fetchablePath, this );
 			}
-
 			return false;
 		}
+		if ( cardinality == Cardinality.MANY_TO_ONE ) {
+			/*
+				class Child {
+					@OneToOne(mappedBy = "biologicalChild")
+					private Mother mother;
+				}
 
-	protected boolean isBidirectionalAttributeName(NavigablePath fetchablePath) {
+				class Mother {
-		if ( bidirectionalAttributeName == null ) {
+					@OneToOne
+					private Child biologicalChild;
+				}
+
+				fetchablePath= Mother.biologicalChild.mother
+				this.mappedBy = "biologicalChild"
+				parent.getFullPath() = "Mother.biologicalChild"
+			 */
+			final String fullPath = parentNavigablePath.getFullPath();
+			if ( fullPath.endsWith( bidirectionalAttributeName + "." + CollectionPart.Nature.ELEMENT.getName() ) ) {
+				final NavigablePath parentPath = parentNavigablePath.getParent().getParent();
+				// This can be null for a collection loader
+				if ( parentPath != null ) {
+					// 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() ) == '('
+						);
+					}
+					// If we have a parent, we ensure that the parent is the same as the attribute name
+					else {
+						return parentPath.getUnaliasedLocalName().equals( navigableRole.getLocalName() );
+					}
+				}
+			}
 			return false;
 		}
-		return fetchablePath.getFullPath().endsWith( bidirectionalAttributeName );
+		return parentNavigablePath.getUnaliasedLocalName().equals( bidirectionalAttributeName );
 	}
 
 	public String getBidirectionalAttributeName(){
@@ -744,16 +758,7 @@ public class ToOneAttributeMapping
 				side,
 				creationState
 		);
-		boolean selectByUniqueKey;
+		final boolean selectByUniqueKey = isSelectByUniqueKey( side );
-		if ( side == ForeignKeyDescriptor.Nature.KEY ) {
-			// case 1.2
-			selectByUniqueKey = !getKeyTargetMatchPart().getNavigableRole()
-					.equals( entityMappingType.getIdentifierMapping().getNavigableRole() );
-		}
-		else {
-			// case 1.1
-			selectByUniqueKey = bidirectionalAttributeName != null;
-		}
 
 		if ( fetchTiming == FetchTiming.IMMEDIATE ) {
 			return new EntityFetchSelectImpl(
@@ -774,6 +779,18 @@ public class ToOneAttributeMapping
 		);
 	}
 
+	private boolean isSelectByUniqueKey(ForeignKeyDescriptor.Nature side) {
+		if ( side == ForeignKeyDescriptor.Nature.KEY ) {
+			// case 1.2
+			return !getKeyTargetMatchPart().getNavigableRole()
+					.equals( entityMappingType.getIdentifierMapping().getNavigableRole() );
+		}
+		else {
+			// case 1.1
+			return bidirectionalAttributeName != null;
+		}
+	}
+
 	@Override
 	public <T> DomainResult<T> createDelayedDomainResult(
 			NavigablePath navigablePath,

hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java

@@ -5134,7 +5134,8 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 
 			EntityGraphTraversalState.TraversalResult traversalResult = null;
 			final FromClauseIndex fromClauseIndex = getFromClauseIndex();
-			final SqmAttributeJoin fetchedJoin = fromClauseIndex.findFetchedJoinByPath( fetchablePath );
+			final SqmAttributeJoin<?, ?> fetchedJoin = fromClauseIndex.findFetchedJoinByPath( fetchablePath );
+			boolean explicitFetch = false;
 
 			if ( fetchedJoin != null ) {
 				// there was an explicit fetch in the SQM
@@ -5148,6 +5149,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				}
 				joined = true;
 				alias = fetchedJoin.getExplicitAlias();
+				explicitFetch = true;
 			}
 			else {
 				// there was not an explicit fetch in the SQM
@@ -5158,6 +5160,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 						traversalResult = entityGraphTraversalState.traverse( fetchParent, fetchable, isKeyFetchable );
 						fetchTiming = traversalResult.getFetchTiming();
 						joined = traversalResult.isJoined();
+						explicitFetch = true;
 					}
 					else if ( getLoadQueryInfluencers().hasEnabledFetchProfiles() ) {
 						// There is no point in checking the fetch profile if it can't affect this fetchable
@@ -5173,6 +5176,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 								if ( profileFetch != null ) {
 									fetchTiming = FetchTiming.IMMEDIATE;
 									joined = joined || profileFetch.getStyle() == org.hibernate.engine.profile.Fetch.Style.JOIN;
+									explicitFetch = true;
 								}
 							}
 						}
@@ -5224,8 +5228,8 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 				if ( incrementFetchDepth ) {
 					fetchDepth++;
 				}
-				// There is no need to check for circular fetches if this is a fetch join
+				// There is no need to check for circular fetches if this is an explicit fetch
-				if ( fetchedJoin == null && !isResolvingCircularFetch() ) {
+				if ( !explicitFetch && !isResolvingCircularFetch() ) {
 					final Fetch biDirectionalFetch = fetchable.resolveCircularFetch(
 							fetchablePath,
 							fetchParent,

hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularBiDirectionalFetchImpl.java

@@ -192,11 +192,17 @@ public class CircularBiDirectionalFetchImpl implements BiDirectionalFetch, Assoc
 		public Object assemble(RowProcessingState rowProcessingState, JdbcValuesSourceProcessingOptions options) {
 			EntityInitializer initializer = resolveCircularInitializer( rowProcessingState );
 			if ( initializer == null ) {
-				final Initializer parentInitializer = rowProcessingState.resolveInitializer( circularPath );
 				if ( circularPath.getParent() != null ) {
-					initializer = (EntityInitializer) rowProcessingState.resolveInitializer( circularPath.getParent() );
+					NavigablePath path = circularPath.getParent();
+					Initializer parentInitializer = rowProcessingState.resolveInitializer( path );
+					while ( !( parentInitializer instanceof EntityInitializer ) && path.getParent() != null ) {
+						path = path.getParent();
+						parentInitializer = rowProcessingState.resolveInitializer( path );
+					}
+					initializer = (EntityInitializer) parentInitializer;
 				}
 				else {
+					final Initializer parentInitializer = rowProcessingState.resolveInitializer( circularPath );
 					assert parentInitializer instanceof CollectionInitializer;
 					final CollectionInitializer circ = (CollectionInitializer) parentInitializer;
 					final EntityPersister entityPersister = (EntityPersister) ( (AttributeMapping) fetchable ).getMappedType();
@@ -280,7 +286,6 @@ public class CircularBiDirectionalFetchImpl implements BiDirectionalFetch, Assoc
 			while ( !( parentInitializer instanceof EntityInitializer ) && path.getParent() != null ) {
 				path = path.getParent();
 				parentInitializer = rowProcessingState.resolveInitializer( path );
-
 			}
 
 			if ( !( parentInitializer instanceof EntityInitializer ) ) {

hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularFetchImpl.java

@@ -28,6 +28,7 @@ import org.hibernate.sql.results.graph.FetchParentAccess;
 import org.hibernate.sql.results.graph.Fetchable;
 import org.hibernate.sql.results.graph.entity.EntityInitializer;
 import org.hibernate.sql.results.graph.entity.internal.EntityDelayedFetchInitializer;
+import org.hibernate.sql.results.graph.entity.internal.EntitySelectFetchByUniqueKeyInitializer;
 import org.hibernate.sql.results.graph.entity.internal.EntitySelectFetchInitializer;
 import org.hibernate.sql.results.jdbc.spi.JdbcValuesSourceProcessingOptions;
 import org.hibernate.sql.results.jdbc.spi.RowProcessingState;
@@ -37,12 +38,13 @@ import org.hibernate.type.descriptor.java.JavaTypeDescriptor;
  * @author Andrea Boriero
  */
 public class CircularFetchImpl implements BiDirectionalFetch, Association {
-	private DomainResult keyResult;
+	private final DomainResult<?> keyResult;
-	private EntityValuedModelPart referencedModelPart;
+	private final EntityValuedModelPart referencedModelPart;
 	private final EntityMappingType entityMappingType;
 	private final FetchTiming timing;
 	private final NavigablePath navigablePath;
 	private final ToOneAttributeMapping fetchable;
+	private final boolean selectByUniqueKey;
 
 	private final FetchParent fetchParent;
 	private final NavigablePath referencedNavigablePath;
@@ -54,13 +56,15 @@ public class CircularFetchImpl implements BiDirectionalFetch, Association {
 			NavigablePath navigablePath,
 			FetchParent fetchParent,
 			ToOneAttributeMapping fetchable,
+			boolean selectByUniqueKey,
 			NavigablePath referencedNavigablePath,
-			DomainResult keyResult) {
+			DomainResult<?> keyResult) {
 		this.referencedModelPart = referencedModelPart;
 		this.entityMappingType = entityMappingType;
 		this.timing = timing;
 		this.fetchParent = fetchParent;
 		this.navigablePath = navigablePath;
+		this.selectByUniqueKey = selectByUniqueKey;
 		this.referencedNavigablePath = referencedNavigablePath;
 		this.fetchable = fetchable;
 		this.keyResult = keyResult;
@@ -102,6 +106,15 @@ public class CircularFetchImpl implements BiDirectionalFetch, Association {
 				getNavigablePath(),
 				referencedModelPart,
 				() -> {
+					if ( selectByUniqueKey ) {
+						return new EntitySelectFetchByUniqueKeyInitializer(
+								parentAccess,
+								fetchable,
+								getNavigablePath(),
+								entityMappingType.getEntityPersister(),
+								resultAssembler
+						);
+					}
 					if ( timing == FetchTiming.IMMEDIATE ) {
 						return new EntitySelectFetchInitializer(
 								parentAccess,