Do not alter the join type anymore for non-real table groups that have table reference joins. Handle nullable key tables for to-ones properly and implement bidirectional one-to-one fetching optimization

hibernate-core/src/main/java/org/hibernate/cfg/OneToOneSecondPass.java

@@ -198,6 +198,7 @@ public class OneToOneSecondPass implements SecondPass {
 					manyToOne.setLazy( value.isLazy() );
 					manyToOne.setReferencedEntityName( value.getReferencedEntityName() );
 					manyToOne.setUnwrapProxy( value.isUnwrapProxy() );
+					manyToOne.markAsLogicalOneToOne();
 					prop.setValue( manyToOne );
 					Iterator otherSideJoinKeyColumns = otherSideJoin.getKey().getColumnIterator();
 					while ( otherSideJoinKeyColumns.hasNext() ) {

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

@@ -35,7 +35,19 @@ public interface Queryable extends ModelPart {
 	ModelPart findSubPart(String name, EntityMappingType treatTargetType);
 
 	default ModelPart resolveSubPart(DotIdentifierSequence path) {
-		return path.resolve( (ModelPart) this, (part, name) -> ( (Queryable) part ).findSubPart( name, null ) );
+		return path.resolve(
+				(ModelPart) this,
+				(part, name) -> {
+					final String fullPath = part.getNavigableRole().getFullPath();
+					if ( fullPath.equals( name ) ) {
+						return part;
+					}
+					else {
+						return ( (Queryable) part ).findSubPart( name.substring( fullPath.length() + 1 ), null );
+					}
+				},
+				(part, name) -> ( (Queryable) part ).findSubPart( name, null )
+		);
 	}
 
 	/**

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

@@ -16,14 +16,18 @@ import java.util.function.Consumer;
 import org.hibernate.LockMode;
 import org.hibernate.engine.FetchStyle;
 import org.hibernate.engine.FetchTiming;
+import org.hibernate.engine.jdbc.spi.JdbcServices;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.internal.util.StringHelper;
+import org.hibernate.internal.util.collections.ArrayHelper;
 import org.hibernate.mapping.Collection;
 import org.hibernate.mapping.IndexedConsumer;
+import org.hibernate.mapping.Join;
 import org.hibernate.mapping.ManyToOne;
 import org.hibernate.mapping.OneToOne;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
+import org.hibernate.mapping.Selectable;
 import org.hibernate.mapping.ToOne;
 import org.hibernate.metamodel.mapping.AssociationKey;
 import org.hibernate.metamodel.mapping.CollectionPart;
@@ -38,6 +42,8 @@ import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.metamodel.mapping.SelectableConsumer;
 import org.hibernate.metamodel.mapping.StateArrayContributorMetadataAccess;
 import org.hibernate.metamodel.model.domain.NavigableRole;
+import org.hibernate.persister.collection.QueryableCollection;
+import org.hibernate.persister.entity.AbstractEntityPersister;
 import org.hibernate.persister.entity.EntityPersister;
 import org.hibernate.property.access.spi.PropertyAccess;
 import org.hibernate.query.EntityIdentifierNavigablePath;
@@ -99,6 +105,7 @@ public class ToOneAttributeMapping
 
 	private final String sqlAliasStem;
 	private final boolean isNullable;
+	private final boolean isKeyTableNullable;
 	private final boolean isConstrained;
 	private final boolean isIgnoreNotFound;
 	private final boolean unwrapProxy;
@@ -185,20 +192,75 @@ public class ToOneAttributeMapping
 			}
 			if ( referencedPropertyName == null ) {
 				String bidirectionalAttributeName = null;
-				final PersistentClass entityBinding = manyToOne.getMetadata().getEntityBinding( manyToOne.getReferencedEntityName() );
+				final PersistentClass entityBinding = manyToOne.getMetadata()
+						.getEntityBinding( manyToOne.getReferencedEntityName() );
+				if ( cardinality == Cardinality.LOGICAL_ONE_TO_ONE ) {
+					// Handle join table cases
+					final Iterator<Join> joinClosureIterator = entityBinding.getJoinClosureIterator();
+					while ( joinClosureIterator.hasNext() ) {
+						final Join join = joinClosureIterator.next();
+						if ( join.getPersistentClass().getEntityName().equals( entityBinding.getEntityName() )
+								&& join.getPropertySpan() == 1
+								&& join.getTable() == manyToOne.getTable()
+								&& equal( join.getKey().getColumnIterator(), manyToOne.getColumnIterator() ) ) {
+							bidirectionalAttributeName = join.getPropertyIterator().next().getName();
+							break;
+						}
+					}
+					// Simple one-to-one mapped by cases
+					if ( bidirectionalAttributeName == null ) {
 						final Iterator<Property> propertyClosureIterator = entityBinding.getPropertyClosureIterator();
-				while (propertyClosureIterator.hasNext()) {
+						while ( propertyClosureIterator.hasNext() ) {
+							final Property property = propertyClosureIterator.next();
+							if ( property.getValue() instanceof OneToOne && name.equals( ( (OneToOne) property.getValue() ).getMappedByProperty() ) ) {
+								bidirectionalAttributeName = property.getName();
+								break;
+							}
+						}
+					}
+				}
+				else {
+					final Iterator<Property> propertyClosureIterator = entityBinding.getPropertyClosureIterator();
+					while ( propertyClosureIterator.hasNext() ) {
 						final Property property = propertyClosureIterator.next();
 						if ( property.getValue() instanceof Collection && name.equals( ( (Collection) property.getValue() ).getMappedByProperty() ) ) {
 							bidirectionalAttributeName = property.getName();
 							break;
 						}
 					}
+				}
 				this.bidirectionalAttributeName = bidirectionalAttributeName;
 			}
 			else {
 				this.bidirectionalAttributeName = referencedPropertyName;
 			}
+			if ( bootValue.isNullable() ) {
+				isKeyTableNullable = true;
+			}
+			else {
+				final JdbcServices jdbcServices = declaringEntityPersister.getFactory().getJdbcServices();
+				final String targetTableName = jdbcServices.getJdbcEnvironment()
+						.getQualifiedObjectNameFormatter()
+						.format(
+								manyToOne.getTable().getQualifiedTableName(),
+								jdbcServices.getDialect()
+						);
+				if ( CollectionPart.Nature.fromNameExact( navigableRole.getParent().getLocalName() ) != null ) {
+					final PluralAttributeMapping pluralAttribute = (PluralAttributeMapping) declaringEntityPersister.resolveSubPart(
+							navigableRole.getParent().getParent()
+					);
+					final QueryableCollection persister = (QueryableCollection) pluralAttribute.getCollectionDescriptor();
+					isKeyTableNullable = !persister.getTableName().equals( targetTableName );
+				}
+				else {
+					final AbstractEntityPersister persister = (AbstractEntityPersister) declaringEntityPersister;
+					final int tableIndex = ArrayHelper.indexOf(
+							persister.getTableNames(),
+							targetTableName
+					);
+					isKeyTableNullable = persister.isNullableTable( tableIndex );
+				}
+			}
 			isOptional = ( (ManyToOne) bootValue ).isIgnoreNotFound();
 		}
 		else {
@@ -265,6 +327,7 @@ public class ToOneAttributeMapping
 				this.bidirectionalAttributeName = bidirectionalAttributeName;
 			}
 			isIgnoreNotFound = isNullable();
+			isKeyTableNullable = isNullable();
 			isOptional = ! bootValue.isConstrained();
 		}
 		isConstrained = bootValue.isConstrained();
@@ -344,6 +407,7 @@ public class ToOneAttributeMapping
 		this.navigableRole = original.navigableRole;
 		this.sqlAliasStem = original.sqlAliasStem;
 		this.isNullable = original.isNullable;
+		this.isKeyTableNullable = original.isKeyTableNullable;
 		this.isOptional = original.isOptional;
 		this.isIgnoreNotFound = original.isIgnoreNotFound;
 		this.unwrapProxy = original.unwrapProxy;
@@ -357,6 +421,23 @@ public class ToOneAttributeMapping
 		this.isConstrained = original.isConstrained;
 	}
 
+	private boolean equal(Iterator<Selectable> lhsColumns, Iterator<Selectable> rhsColumns) {
+		boolean hasNext;
+		do {
+			final Selectable lhs = lhsColumns.next();
+			final Selectable rhs = rhsColumns.next();
+			if ( !lhs.getText().equals( rhs.getText() ) ) {
+				return false;
+			}
+
+			hasNext = lhsColumns.hasNext();
+			if ( hasNext != rhsColumns.hasNext() ) {
+				return false;
+			}
+		} while ( hasNext );
+		return true;
+	}
+
 	private static void addPrefixedPropertyNames(
 			Set<String> targetKeyPropertyNames,
 			String prefix,
@@ -747,6 +828,16 @@ public class ToOneAttributeMapping
 			}
 
 			creationState.registerVisitedAssociationKey( foreignKeyDescriptor.getAssociationKey() );
+			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()
+					);
+				}
+			}
 			return new EntityFetchJoinedImpl(
 					fetchParent,
 					this,
@@ -875,7 +966,7 @@ public class ToOneAttributeMapping
 
 	@Override
 	public SqlAstJoinType getDefaultSqlAstJoinType(TableGroup parentTableGroup) {
-		if ( isNullable ) {
+		if ( isKeyTableNullable || isNullable ) {
 			return SqlAstJoinType.LEFT;
 		}
 		else if ( parentTableGroup.getModelPart() instanceof CollectionPart ) {

hibernate-core/src/main/java/org/hibernate/query/DotIdentifierSequence.java

@@ -24,13 +24,13 @@ public interface DotIdentifierSequence {
 		return getParent() == null;
 	}
 
-	default <T> T resolve(T base, BiFunction<T, String, T> resolver) {
+	default <T> T resolve(T base, BiFunction<T, String, T> baseResolver, BiFunction<T, String, T> resolver) {
 		final T result;
 		if ( getParent() == null ) {
-			result = base;
+			result = baseResolver.apply( base, getLocalName() );
 		}
 		else {
-			result = resolver.apply( getParent().resolve( base, resolver ), getLocalName() );
+			result = resolver.apply( getParent().resolve( base, baseResolver, resolver ), getLocalName() );
 		}
 		return result;
 	}

hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java

@@ -3534,17 +3534,7 @@ public abstract class AbstractSqlAstTranslator<T extends JdbcOperation> implemen
 		}
 		else if ( !( joinedGroup instanceof LazyTableGroup ) || ( (LazyTableGroup) joinedGroup ).getUnderlyingTableGroup() != null ) {
 			appendSql( WHITESPACE );
-			SqlAstJoinType joinType = tableGroupJoin.getJoinType();
+			renderJoinType( tableGroupJoin.getJoinType() );
-			// todo (6.0): no exactly sure why this is necessary and IMO this should ideally go away.
-			//  I realized that inverse one-to-one associations with join tables are considered "non-nullable" in the boot model.
-			//  Due to that, we use an inner join for the table group join of that association which the following "works around"
-			//  IMO such an association should be considered nullable. It's also odd that the association
-			//  has the FK Side KEY, although it is clearly TARGET
-			//  See org.hibernate.orm.test.annotations.manytoone.ManyToOneJoinTest.testOneToOneJoinTable2
-			if ( !joinedGroup.isRealTableGroup() && joinType == SqlAstJoinType.INNER && !joinedGroup.getTableReferenceJoins().isEmpty() ) {
-				joinType = SqlAstJoinType.LEFT;
-			}
-			renderJoinType( joinType );
 
 			if ( tableGroupJoin.getPredicate() != null && !tableGroupJoin.getPredicate().isEmpty() ) {
 				renderTableGroup( joinedGroup, tableGroupJoin.getPredicate() );

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

@@ -84,7 +84,7 @@ public class EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest {
 				session -> {
 					final Parent parent = session.get( Parent.class, 1 );
 					statementInspector.assertExecutedCount( 1 );
-					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 6 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
 					Male son = parent.getSon();
 					assertThat( son, CoreMatchers.notNullValue() );
 					assertTrue(
@@ -113,7 +113,7 @@ public class EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest {
 		scope.inTransaction( session -> {
 			final Male son = session.get( Male.class, 2 );
 			statementInspector.assertExecutedCount( 1 );
-			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 6 );
+			statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 3 );
 			Parent parent = son.getParent();
 			assertThat( parent, CoreMatchers.notNullValue() );
 			assertTrue(
@@ -155,7 +155,7 @@ public class EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest {
 					// The join to the target table PARENT for Male#parent is avoided,
 					// because the FK in the collection table is not-null and data from the target table is not needed
 					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 1 );
-					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 6 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 3 );
 					assertThat( son.getParent(), CoreMatchers.notNullValue() );
 
 					String description = son.getParent().getDescription();
@@ -178,7 +178,7 @@ public class EntityWithBidirectionalAssociationsOneOfWhichIsAJoinTableTest {
 							.getSingleResult();
 					statementInspector.assertExecutedCount( 2 );
 					statementInspector.assertNumberOfOccurrenceInQuery( 0, "join", 2 );
-					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 6 );
+					statementInspector.assertNumberOfOccurrenceInQuery( 1, "join", 3 );
 					Male son = parent.getSon();
 					assertThat( son, CoreMatchers.notNullValue() );
 					assertTrue(