HHH-17143 - More not-found fix ups

https://hibernate.atlassian.net/browse/HHH-17143
hibernate · Dec 1, 2023 · 3e96ddc · 3e96ddc
1 parent f7709e7
commit 3e96ddc
Show file tree

Hide file tree

Showing 4 changed files with 41 additions and 17 deletions.
diff --git a/...te-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java b/...te-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java
@@ -63,6 +63,7 @@
 import org.hibernate.spi.EntityIdentifierNavigablePath;
 import org.hibernate.spi.NavigablePath;
 import org.hibernate.spi.TreatedNavigablePath;
+import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlAstJoinType;
 import org.hibernate.sql.ast.spi.FromClauseAccess;
 import org.hibernate.sql.ast.spi.SqlAliasBase;
@@ -1991,6 +1992,19 @@ public TableGroupJoin createTableGroupJoin(
 		return join;
 	}
 
+	@Override
+	public SqlAstJoinType determineSqlJoinType(TableGroup lhs, SqlAstJoinType requestedJoinType, boolean fetched) {
+		if ( requestedJoinType != null ) {
+			return requestedJoinType;
+		}
+
+		if ( fetched ) {
+			return getDefaultSqlAstJoinType( lhs );
+		}
+
+		return SqlAstJoinType.INNER;
+	}
+
 	@Override
 	public LazyTableGroup createRootTableGroupJoin(
 			NavigablePath navigablePath,

diff --git a/...re/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java b/...re/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java
@@ -206,10 +206,11 @@ private static <T> EntityValuedPathInterpretation<T> from(
 					resultTableGroup = tableGroup;
 				}
 			}
-			else if ( inferredMapping == null && hasNotFound( mapping ) ) {
-				// This is necessary to allow expression like `where root.notFoundAssociation is null`
-				// to render to `alias.not_found_fk is null`, but IMO this shouldn't be done
-				// todo: discuss removing this part and create a left joined table group instead?
+			else if ( inferredMapping == null
+					&& hasNotFound( mapping )
+					&& sqlAstCreationState.getCurrentClauseStack().getCurrent() == Clause.SET ) {
+				// for not-found mappings encountered in the SET clause of an UPDATE statement
+				// we will want to (1) not join and (2) render the fk
 				resultModelPart = keyTargetMatchPart;
 				resultTableGroup = sqlAstCreationState.getFromClauseAccess()
 						.findTableGroup( tableGroup.getNavigablePath().getParent() );
@@ -218,6 +219,10 @@ else if ( inferredMapping == null && hasNotFound( mapping ) ) {
 				// If the mapping is an inverse association, use the PK and disallow FK optimizations
 				resultModelPart = ( (EntityAssociationMapping) mapping ).getAssociatedEntityMappingType().getIdentifierMapping();
 				resultTableGroup = tableGroup;
+
+				// todo (not-found) : in the case of not-found=ignore, we want to do the join, however -
+				//  	* use a left join when the association is the path terminus (`root.association`)
+				//  	* use an inner join when it is further de-referenced (`root.association.stuff`)
 			}
 		}
 		else if ( mapping instanceof AnonymousTupleEntityValuedModelPart ) {

diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoinProducer.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoinProducer.java
@@ -78,18 +78,14 @@ TableGroup createRootTableGroupJoin(
 			SqlAstCreationState creationState);
 
 	default SqlAstJoinType determineSqlJoinType(TableGroup lhs, SqlAstJoinType requestedJoinType, boolean fetched) {
-		final SqlAstJoinType joinType;
-		if ( requestedJoinType == null ) {
-			if ( fetched ) {
-				joinType = getDefaultSqlAstJoinType( lhs );
-			}
-			else {
-				joinType = SqlAstJoinType.INNER;
-			}
+		if ( requestedJoinType != null ) {
+			return requestedJoinType;
 		}
-		else {
-			joinType = requestedJoinType;
+
+		if ( fetched ) {
+			return getDefaultSqlAstJoinType( lhs );
 		}
-		return joinType;
+
+		return SqlAstJoinType.INNER;
 	}
 }
diff --git a/...e/src/test/java/org/hibernate/orm/test/notfound/MutationQueriesAndNotFoundActionTest.java b/...e/src/test/java/org/hibernate/orm/test/notfound/MutationQueriesAndNotFoundActionTest.java
@@ -3,6 +3,7 @@
 import org.hibernate.annotations.NotFound;
 import org.hibernate.annotations.NotFoundAction;
 
+import org.hibernate.testing.jdbc.SQLStatementInspector;
 import org.hibernate.testing.orm.junit.DomainModel;
 import org.hibernate.testing.orm.junit.JiraKey;
 import org.hibernate.testing.orm.junit.SessionFactory;
@@ -17,15 +18,15 @@
 import jakarta.persistence.ManyToOne;
 import jakarta.persistence.Table;
 
-import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
 
 @DomainModel(
 		annotatedClasses = {
 				MutationQueriesAndNotFoundActionTest.User.class,
 				MutationQueriesAndNotFoundActionTest.Comment.class
 		}
 )
-@SessionFactory
+@SessionFactory( useCollectingStatementInspector = true )
 @JiraKey("HHH-16878")
 public class MutationQueriesAndNotFoundActionTest {
 
@@ -56,15 +57,23 @@ public void tearDown(SessionFactoryScope scope) {
 
 	@Test
 	public void testUpdate(SessionFactoryScope scope) {
+		final SQLStatementInspector statementInspector = scope.getCollectingStatementInspector();
+
 		scope.inTransaction(
 				session -> {
+					statementInspector.clear();
+
 					int affectedComments = session.createMutationQuery(
 									"update Comment c set c.text = :text where c.user = :user" )
 							.setParameter( "text", "updated" )
 							.setParameter( "user", session.getReference( User.class, 1L ) )
 							.executeUpdate();
 
 					assertThat( affectedComments ).isEqualTo( 2 );
+					assertThat( statementInspector.getSqlQueries() ).hasSize( 1 );
+					assertThat( statementInspector.getSqlQueries().get( 0 ) ).matches( (sql) -> {
+						return sql.contains( " join " ) || sql.contains( "exists" );
+					} );
 				}
 		);
 	}