From eb82b2f3905dcaf704f5e8d957a0f63faed0c3e2 Mon Sep 17 00:00:00 2001
From: Christian Beikov <christian.beikov@gmail.com>
Date: Tue, 25 Apr 2023 10:16:05 +0200
Subject: [PATCH] HHH-16382 Make sure joins are adapted to inner if non-FK
 parts of a path are de-referenced

---
 .../sqm/sql/BaseSqmToSqlAstConverter.java     | 27 ++++++++------
 .../annotations/onetoone/OneToOneTest.java    | 35 +++++++++++++++++++
 2 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java
index 4c17533f4e..2916f5be3b 100644
--- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java
+++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java
@@ -3406,23 +3406,30 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
 			registerTreatUsage( (SqmFrom<?, ?>) parentPath, tableGroup );
 		}
 
-		if ( getCurrentClauseStack().getCurrent() != Clause.SELECT
+		if ( parentPath instanceof SqmSimplePath<?>
+				&& CollectionPart.Nature.fromName( parentPath.getNavigablePath().getLocalName() ) == null
+				&& getCurrentClauseStack().getCurrent() != Clause.SELECT
 				&& parentPath.getParentPath() != null
 				&& tableGroup.getModelPart() instanceof ToOneAttributeMapping ) {
 			// we need to handle the case of an implicit path involving a to-one
-			// association with not-found mapping where that path has been previously
-			// joined using left.  typically, this indicates that the to-one is being
+			// association that path has been previously joined using left.
+			// typically, this indicates that the to-one is being
 			// fetched - the fetch would use a left-join.  however, since the path is
 			// used outside the select-clause also, we need to force the join to be inner
-			final ToOneAttributeMapping toOneMapping = (ToOneAttributeMapping) tableGroup.getModelPart();
-			if ( toOneMapping.hasNotFoundAction() ) {
+			final ToOneAttributeMapping toOneAttributeMapping = (ToOneAttributeMapping) tableGroup.getModelPart();
+			final String partName = sqmPath.getResolvedModel().getPathName();
+			final ModelPart pathPart;
+			if ( !toOneAttributeMapping.isFkOptimizationAllowed()
+					|| !( ( pathPart = toOneAttributeMapping.findSubPart( partName ) ) instanceof ValuedModelPart )
+					|| !toOneAttributeMapping.getForeignKeyDescriptor().isKeyPart( (ValuedModelPart) pathPart ) ) {
 				final NavigablePath parentParentPath = parentPath.getParentPath().getNavigablePath();
 				final TableGroup parentParentTableGroup = fromClauseIndex.findTableGroup( parentParentPath );
-				parentParentTableGroup.visitTableGroupJoins( (join) -> {
-					if ( join.getNavigablePath().equals( parentPath.getNavigablePath() ) && join.isImplicit() ) {
-						join.setJoinType( SqlAstJoinType.INNER );
-					}
-				} );
+				final TableGroupJoin tableGroupJoin = parentParentTableGroup.findTableGroupJoin( tableGroup );
+				// We might get null here if the parentParentTableGroup is correlated and tableGroup is from the outer query
+				// In this case, we don't want to override the join type, though it is debatable if it's ok to reuse a join in this case
+				if ( tableGroupJoin != null ) {
+					tableGroupJoin.setJoinType( SqlAstJoinType.INNER );
+				}
 			}
 		}
 
diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetoone/OneToOneTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetoone/OneToOneTest.java
index f79fa7e404..2ba1fcb191 100644
--- a/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetoone/OneToOneTest.java
+++ b/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/onetoone/OneToOneTest.java
@@ -7,6 +7,7 @@
 package org.hibernate.orm.test.annotations.onetoone;
 
 import java.util.Iterator;
+import java.util.List;
 
 import jakarta.persistence.criteria.CriteriaBuilder;
 import jakarta.persistence.criteria.CriteriaQuery;
@@ -27,6 +28,10 @@ import org.hibernate.orm.test.annotations.Customer;
 import org.hibernate.orm.test.annotations.Discount;
 import org.hibernate.orm.test.annotations.Passport;
 import org.hibernate.orm.test.annotations.Ticket;
+import org.hibernate.query.criteria.HibernateCriteriaBuilder;
+import org.hibernate.query.criteria.JpaCriteriaQuery;
+import org.hibernate.query.criteria.JpaRoot;
+
 import org.junit.Test;
 
 import static org.hamcrest.CoreMatchers.is;
@@ -416,6 +421,36 @@ public class OneToOneTest extends BaseNonConfigCoreFunctionalTestCase {
 			assertThat( p, is( notNullValue() ) );
 		} );
 	}
+
+	@Test
+	public void testDereferenceOneToOne() {
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			Client c1 = new Client();
+			c1.setName( "C1" );
+			Client c2 = new Client();
+			c2.setName( "C2" );
+			Client c3 = new Client();
+			c3.setName( "C3" );
+			Address a = new Address();
+			a.setCity( "Vienna" );
+			c1.setAddress( a );
+			c3.setAddress( new Address() );
+			session.persist( c1 );
+			session.persist( c2 );
+			session.persist( c3 );
+		} );
+
+		TransactionUtil.doInHibernate( this::sessionFactory, session -> {
+			HibernateCriteriaBuilder cb = session.getCriteriaBuilder();
+			JpaCriteriaQuery<Client> query = cb.createQuery( Client.class );
+			JpaRoot<Client> root = query.from( Client.class );
+			query.where( root.get( "address" ).get( "city" ).isNull() );
+			List<Client> resultList = session.createQuery( query ).getResultList();
+
+			assertEquals( 1, resultList.size() );
+			assertEquals( "C3", resultList.get( 0 ).getName() );
+		} );
+	}
 	
 	@Override
 	protected Class[] getAnnotatedClasses() {