Fix root validation for subqueries that appear in the on clause

hibernate-core/src/main/java/org/hibernate/query/hql/internal/QualifiedJoinPredicatePathConsumer.java

@@ -10,12 +10,15 @@ import java.util.Locale;
 
 import org.hibernate.query.SemanticException;
 import org.hibernate.query.hql.spi.SemanticPathPart;
+import org.hibernate.query.hql.spi.SqmCreationProcessingState;
 import org.hibernate.query.hql.spi.SqmCreationState;
 import org.hibernate.query.sqm.tree.SqmQuery;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
+import org.hibernate.query.sqm.tree.from.SqmFromClause;
 import org.hibernate.query.sqm.tree.from.SqmQualifiedJoin;
 import org.hibernate.query.sqm.tree.from.SqmRoot;
 import org.hibernate.query.sqm.tree.select.SqmQuerySpec;
+import org.hibernate.query.sqm.tree.select.SqmSelectQuery;
 import org.hibernate.query.sqm.tree.select.SqmSubQuery;
 
 /**
@@ -40,17 +43,29 @@ public class QualifiedJoinPredicatePathConsumer extends BasicDotIdentifierConsum
 			@Override
 			protected void validateAsRoot(SqmFrom<?, ?> pathRoot) {
 				final SqmRoot<?> root = pathRoot.findRoot();
-				if ( root != sqmJoin.findRoot() ) {
+				final SqmRoot<?> joinRoot = sqmJoin.findRoot();
-					final SqmQuery<?> processingQuery = getCreationState().getCurrentProcessingState().getProcessingQuery();
+				if ( root != joinRoot ) {
-					if ( processingQuery instanceof SqmSubQuery<?> ) {
+					// The root of a path within a query doesn't have the same root as the current join we are processing.
-						final SqmQuerySpec<?> querySpec = ( (SqmSubQuery<?>) processingQuery ).getQuerySpec();
+					// The aim of this check is to prevent uses of different "spaces" i.e. `from A a, B b join b.id = a.id` would be illegal
-						// If this "foreign" from element is used in a sub query
+					SqmCreationProcessingState processingState = getCreationState().getCurrentProcessingState();
-						// This is only an error if the from element is actually part of the sub query
+					// First, we need to find out if the current join is part of current processing query
-						if ( querySpec.getFromClause() == null || !querySpec.getFromClause().getRoots().contains( root ) ) {
+					final SqmQuery<?> currentProcessingQuery = processingState.getProcessingQuery();
-							super.validateAsRoot( pathRoot );
+					if ( currentProcessingQuery instanceof SqmSelectQuery<?> ) {
+						final SqmQuerySpec<?> querySpec = ( (SqmSelectQuery<?>) currentProcessingQuery ).getQuerySpec();
+						final SqmFromClause fromClause = querySpec.getFromClause();
+						// If the current processing query contains the root of the current join,
+						// then the root of the processing path must be a root of one of the parent queries
+						if ( fromClause != null && fromClause.getRoots().contains( joinRoot ) ) {
+							validateAsRootOnParentQueryClosure( pathRoot, root, processingState.getParentProcessingState() );
 							return;
 						}
 					}
+					// If the current join is not part of the processing query, this must be a sub query in the ON clause
+					// in which case the path root is allowed to occur in the current processing query as root
+					if ( currentProcessingQuery instanceof SqmSubQuery<?> ) {
+						validateAsRootOnParentQueryClosure( pathRoot, root, processingState );
+						return;
+					}
 					throw new SemanticException(
 							String.format(
 									Locale.ROOT,
@@ -63,6 +78,35 @@ public class QualifiedJoinPredicatePathConsumer extends BasicDotIdentifierConsum
 
 				super.validateAsRoot( pathRoot );
 			}
+
+			private void validateAsRootOnParentQueryClosure(
+					SqmFrom<?, ?> pathRoot,
+					SqmRoot<?> root,
+					SqmCreationProcessingState processingState) {
+				while ( processingState != null ) {
+					final SqmQuery<?> processingQuery = processingState.getProcessingQuery();
+					if ( processingQuery instanceof SqmSelectQuery<?> ) {
+						final SqmQuerySpec<?> querySpec = ( (SqmSelectQuery<?>) processingQuery ).getQuerySpec();
+						final SqmFromClause fromClause = querySpec.getFromClause();
+						// If we are in a sub query, the "foreign" from element could be one of the sub query roots,
+						// which is totally fine. The aim of this check is to prevent uses of different "spaces"
+						// i.e. `from A a, B b join b.id = a.id` would be illegal
+						if ( fromClause != null && fromClause.getRoots().contains( root ) ) {
+							super.validateAsRoot( pathRoot );
+							return;
+						}
+					}
+					processingState = processingState.getParentProcessingState();
+				}
+				throw new SemanticException(
+						String.format(
+								Locale.ROOT,
+								"SqmQualifiedJoin predicate referred to SqmRoot [`%s`] other than the join's root [`%s`]",
+								pathRoot.getNavigablePath().getFullPath(),
+								sqmJoin.getNavigablePath().getFullPath()
+						)
+				);
+			}
 		};
 	}
 }

hibernate-core/src/main/java/org/hibernate/query/hql/internal/SemanticQueryBuilder.java

@@ -810,7 +810,7 @@ public class SemanticQueryBuilder<R> extends HqlParserBaseVisitor<Object> implem
 		}
 
 		// visit from-clause first!!!
-		sqmQuerySpec.setFromClause( visitFromClause( (HqlParser.FromClauseContext) ctx.getChild( fromIndex ) ) );
+		visitFromClause( (HqlParser.FromClauseContext) ctx.getChild( fromIndex ) );
 
 		final SqmSelectClause selectClause;
 		if ( fromIndex == 1 ) {
@@ -1392,16 +1392,18 @@ public class SemanticQueryBuilder<R> extends HqlParserBaseVisitor<Object> implem
 		final SqmFromClause fromClause;
 		if ( parserFromClause == null ) {
 			fromClause = new SqmFromClause();
+			currentQuerySpec().setFromClause( fromClause );
 		}
 		else {
 			final int size = parserFromClause.getChildCount();
 			// Shift 1 bit instead of division by 2
 			final int estimatedSize = size >> 1;
 			fromClause = new SqmFromClause( estimatedSize );
+			currentQuerySpec().setFromClause( fromClause );
 			for ( int i = 0; i < size; i++ ) {
 				final ParseTree parseTree = parserFromClause.getChild( i );
 				if ( parseTree instanceof HqlParser.EntityWithJoinsContext ) {
-					fromClause.addRoot( visitEntityWithJoins( (HqlParser.EntityWithJoinsContext) parseTree ) );
+					visitEntityWithJoins( (HqlParser.EntityWithJoinsContext) parseTree );
 				}
 			}
 		}
@@ -1411,6 +1413,7 @@ public class SemanticQueryBuilder<R> extends HqlParserBaseVisitor<Object> implem
 	@Override
 	public SqmRoot<?> visitEntityWithJoins(HqlParser.EntityWithJoinsContext parserSpace) {
 		final SqmRoot<?> sqmRoot = visitRootEntity( (HqlParser.RootEntityContext) parserSpace.getChild( 0 ) );
+		currentQuerySpec().getFromClause().addRoot( sqmRoot );
 		final int size = parserSpace.getChildCount();
 		for ( int i = 1; i < size; i++ ) {
 			final ParseTree parseTree = parserSpace.getChild( i );

hibernate-core/src/test/java/org/hibernate/orm/test/join/OuterJoinTest.java

@@ -377,6 +377,40 @@ public class OuterJoinTest {
 		} );
 	}
 
+	@Test
+	@SkipForDialect(dialectClass = TiDBDialect.class, reason = "TiDB db does not support subqueries for ON condition")
+	public void testSubQueryInOnClause(EntityManagerFactoryScope scope) {
+		scope.inTransaction(
+				em -> {
+					List<Tuple> resultList = em.createQuery(
+							"SELECT 1 " +
+									"FROM A a " +
+									"INNER JOIN B b ON exists(select 1 from C c where c.key = a.key) ",
+							Tuple.class
+					).getResultList();
+
+					assertEquals( 6, resultList.size() );
+				}
+		);
+	}
+
+	@Test
+	@SkipForDialect(dialectClass = TiDBDialect.class, reason = "TiDB db does not support subqueries for ON condition")
+	public void testSubQueryCorrelationInOnClause(EntityManagerFactoryScope scope) {
+		scope.inTransaction(
+				em -> {
+					List<Tuple> resultList = em.createQuery(
+							"SELECT 1 " +
+									"FROM A a " +
+									"INNER JOIN B b ON exists(select 1 from a.association c where c.key = a.key) ",
+							Tuple.class
+					).getResultList();
+
+					assertEquals( 3, resultList.size() );
+				}
+		);
+	}
+
 	@MappedSuperclass
 	public static class BaseEntity {