HHH-19883 Respect predicates defined on treated join nodes

Author: beikov

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

@@ -27,6 +27,7 @@
 import org.hibernate.query.sqm.tree.from.SqmEntityJoin;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
 import org.hibernate.query.sqm.tree.from.SqmJoin;
+import org.hibernate.query.sqm.tree.from.SqmQualifiedJoin;
 import org.hibernate.query.sqm.tree.from.SqmRoot;
 
 import org.jboss.logging.Logger;
@@ -47,6 +48,7 @@ public class QualifiedJoinPathConsumer implements DotIdentifierConsumer {
 	private final SqmJoinType joinType;
 	private final boolean fetch;
 	private final String alias;
+	private final boolean allowReuse;
 
 	private ConsumerDelegate delegate;
 	private boolean nested;
@@ -56,11 +58,13 @@ public QualifiedJoinPathConsumer(
 			SqmJoinType joinType,
 			boolean fetch,
 			String alias,
+			boolean allowReuse,
 			SqmCreationState creationState) {
 		this.sqmRoot = sqmRoot;
 		this.joinType = joinType;
 		this.fetch = fetch;
 		this.alias = alias;
+		this.allowReuse = allowReuse;
 		this.creationState = creationState;
 	}
 
@@ -74,6 +78,8 @@ public QualifiedJoinPathConsumer(
 		this.joinType = joinType;
 		this.fetch = fetch;
 		this.alias = alias;
+		// This constructor is only used for entity names, so no need for join reuse
+		this.allowReuse = false;
 		this.creationState = creationState;
 		this.delegate = new AttributeJoinDelegate(
 				sqmFrom,
@@ -104,7 +110,13 @@ public void consumeIdentifier(String identifier, boolean isBase, boolean isTermi
 		}
 		else {
 			assert delegate != null;
-			delegate.consumeIdentifier( identifier, !nested && isTerminal, !( nested && isTerminal ) );
+			delegate.consumeIdentifier(
+					identifier,
+					!nested && isTerminal,
+					// Non-nested joins shall allow reuse, but nested ones (i.e. in treat)
+					// only allow join reuse for non-terminal parts
+					allowReuse && (!nested || !isTerminal)
+			);
 		}
 	}
 
@@ -197,7 +209,12 @@ private AttributeJoinDelegate resolveAlias(String identifier, boolean isTerminal
 		if ( allowReuse ) {
 			if ( !isTerminal ) {
 				for ( SqmJoin<?, ?> sqmJoin : lhs.getSqmJoins() ) {
-					if ( sqmJoin.getAlias() == null && sqmJoin.getModel() == subPathSource ) {
+					// In order for an HQL join to be reusable, is must have the same path source,
+					if ( sqmJoin.getModel() == subPathSource
+						// must not have a join condition
+						&& ( (SqmQualifiedJoin<?, ?>) sqmJoin ).getJoinPredicate() == null
+						// and the same join type
+						&& sqmJoin.getSqmJoinType() == joinType ) {
 						return sqmJoin;
 					}
 				}

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

@@ -2219,10 +2219,12 @@ protected <X> void consumeJoin(HqlParser.JoinContext parserJoin, SqmRoot<X> sqmR
 			throw new SemanticException( "The 'from' clause of a subquery has a 'fetch'", query );
 		}
 
-		dotIdentifierConsumerStack.push( new QualifiedJoinPathConsumer( sqmRoot, joinType, fetch, alias, this ) );
+		final HqlParser.JoinRestrictionContext joinRestrictionContext = parserJoin.joinRestriction();
+		// Joins are allowed to be reused if they don't have a join condition
+		final boolean allowReuse = joinRestrictionContext == null;
+		dotIdentifierConsumerStack.push( new QualifiedJoinPathConsumer( sqmRoot, joinType, fetch, alias, allowReuse, this ) );
 		try {
 			final SqmQualifiedJoin<X, ?> join = getJoin( sqmRoot, joinType, qualifiedJoinTargetContext, alias, fetch );
-			final HqlParser.JoinRestrictionContext joinRestrictionContext = parserJoin.joinRestriction();
 			if ( join instanceof SqmEntityJoin<?> || join instanceof SqmDerivedJoin<?> || join instanceof SqmCteJoin<?> ) {
 				sqmRoot.addSqmJoin( join );
 			}
@@ -2338,7 +2340,7 @@ protected void consumeJpaCollectionJoin(HqlParser.JpaCollectionJoinContext ctx,
 		final String alias = extractAlias( ctx.variable() );
 		dotIdentifierConsumerStack.push(
 				// According to JPA spec 4.4.6 this is an inner join
-				new QualifiedJoinPathConsumer( sqmRoot, SqmJoinType.INNER, false, alias, this )
+				new QualifiedJoinPathConsumer( sqmRoot, SqmJoinType.INNER, false, alias, true, this )
 		);
 
 		try {

hibernate-core/src/main/java/org/hibernate/query/sqm/spi/SqmCreationHelper.java

@@ -8,11 +8,19 @@
 
 import org.hibernate.metamodel.mapping.CollectionPart;
 import org.hibernate.metamodel.model.domain.PluralPersistentAttribute;
+import org.hibernate.query.criteria.JpaPredicate;
+import org.hibernate.query.sqm.tree.predicate.SqmJunctionPredicate;
+import org.hibernate.query.sqm.tree.predicate.SqmPredicate;
 import org.hibernate.spi.NavigablePath;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 
+import java.util.List;
 import java.util.concurrent.atomic.AtomicLong;
 
+import jakarta.persistence.criteria.Predicate;
+
+import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty;
+
 /**
  * @author Steve Ebersole
  */
@@ -68,6 +76,87 @@ public static NavigablePath buildSubNavigablePath(SqmPath<?> lhs, String subNavi
 		return buildSubNavigablePath( navigablePath, subNavigable, alias );
 	}
 
+	public static SqmPredicate combinePredicates(SqmPredicate baseRestriction, List<Predicate> incomingRestrictions) {
+		if ( isEmpty( incomingRestrictions ) ) {
+			return baseRestriction;
+		}
+
+		SqmPredicate combined = combinePredicates( null, baseRestriction );
+		for ( int i = 0; i < incomingRestrictions.size(); i++ ) {
+			combined = combinePredicates( combined, (SqmPredicate) incomingRestrictions.get(i) );
+		}
+		return combined;
+	}
+
+	public static SqmPredicate combinePredicates(SqmPredicate baseRestriction, JpaPredicate... incomingRestrictions) {
+		if ( isEmpty( incomingRestrictions ) ) {
+			return baseRestriction;
+		}
+
+		SqmPredicate combined = combinePredicates( null, baseRestriction );
+		for ( int i = 0; i < incomingRestrictions.length; i++ ) {
+			combined = combinePredicates( combined, incomingRestrictions[i] );
+		}
+		return combined;
+	}
+
+	public static SqmPredicate combinePredicates(SqmPredicate baseRestriction, Predicate... incomingRestrictions) {
+		if ( isEmpty( incomingRestrictions ) ) {
+			return baseRestriction;
+		}
+
+		SqmPredicate combined = combinePredicates( null, baseRestriction );
+		for ( int i = 0; i < incomingRestrictions.length; i++ ) {
+			combined = combinePredicates( combined, incomingRestrictions[i] );
+		}
+		return combined;
+	}
+
+
+	public static SqmPredicate combinePredicates(SqmPredicate baseRestriction, SqmPredicate incomingRestriction) {
+		if ( baseRestriction == null ) {
+			return incomingRestriction;
+		}
+
+		if ( incomingRestriction == null ) {
+			return baseRestriction;
+		}
+
+		final SqmJunctionPredicate combinedPredicate;
+
+		if ( baseRestriction instanceof SqmJunctionPredicate ) {
+			final SqmJunctionPredicate junction = (SqmJunctionPredicate) baseRestriction;
+			// we already had multiple before
+			if ( junction.getPredicates().isEmpty() ) {
+				return incomingRestriction;
+			}
+
+			if ( junction.getOperator() == Predicate.BooleanOperator.AND ) {
+				combinedPredicate = junction;
+			}
+			else {
+				combinedPredicate = new SqmJunctionPredicate(
+						Predicate.BooleanOperator.AND,
+						baseRestriction.getExpressible(),
+						baseRestriction.nodeBuilder()
+				);
+				combinedPredicate.getPredicates().add( baseRestriction );
+			}
+		}
+		else {
+			combinedPredicate = new SqmJunctionPredicate(
+					Predicate.BooleanOperator.AND,
+					baseRestriction.getExpressible(),
+					baseRestriction.nodeBuilder()
+			);
+			combinedPredicate.getPredicates().add( baseRestriction );
+		}
+
+		combinedPredicate.getPredicates().add( incomingRestriction );
+
+		return combinedPredicate;
+	}
+
 	private SqmCreationHelper() {
 	}
 

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

@@ -128,6 +128,7 @@
 import org.hibernate.query.sqm.mutation.internal.SqmInsertStrategyHelper;
 import org.hibernate.query.sqm.produce.function.internal.PatternRenderer;
 import org.hibernate.query.sqm.spi.BaseSemanticQueryWalker;
+import org.hibernate.query.sqm.spi.SqmCreationHelper;
 import org.hibernate.query.sqm.sql.internal.AnyDiscriminatorPathInterpretation;
 import org.hibernate.query.sqm.sql.internal.AsWrappedExpression;
 import org.hibernate.query.sqm.sql.internal.BasicValuedPathInterpretation;
@@ -227,6 +228,7 @@
 import org.hibernate.query.sqm.tree.from.SqmFrom;
 import org.hibernate.query.sqm.tree.from.SqmFromClause;
 import org.hibernate.query.sqm.tree.from.SqmJoin;
+import org.hibernate.query.sqm.tree.from.SqmQualifiedJoin;
 import org.hibernate.query.sqm.tree.from.SqmRoot;
 import org.hibernate.query.sqm.tree.insert.SqmConflictClause;
 import org.hibernate.query.sqm.tree.insert.SqmConflictUpdateAction;
@@ -385,7 +387,6 @@
 import org.hibernate.sql.results.graph.FetchParent;
 import org.hibernate.sql.results.graph.Fetchable;
 import org.hibernate.sql.results.graph.FetchableContainer;
-import org.hibernate.sql.results.graph.collection.internal.EagerCollectionFetch;
 import org.hibernate.sql.results.graph.entity.EntityResultGraphNode;
 import org.hibernate.sql.results.graph.instantiation.internal.DynamicInstantiation;
 import org.hibernate.sql.results.graph.internal.ImmutableFetchList;
@@ -2684,7 +2685,12 @@ protected void consumeFromClauseCorrelatedRoot(SqmRoot<?> sqmRoot) {
 			// as roots anyway, so nothing to worry about
 
 			log.tracef( "Resolved SqmRoot [%s] to correlated TableGroup [%s]", sqmRoot, tableGroup );
-			consumeExplicitJoins( from, tableGroup );
+			if ( from instanceof SqmRoot<?> ) {
+				consumeJoins( (SqmRoot<?>) from, fromClauseIndex, tableGroup );
+			}
+			else {
+				consumeExplicitJoins( from, tableGroup );
+			}
 			return;
 		}
 		else {
@@ -3347,6 +3353,39 @@ private TableGroup consumeAttributeJoin(
 				SqmMappingModelHelper.resolveExplicitTreatTarget( sqmJoin, this )
 		);
 
+		final List<SqmFrom<?, ?>> sqmTreats = sqmJoin.getSqmTreats();
+		final SqmPredicate joinPredicate;
+		final SqmPredicate[] treatPredicates;
+		final boolean hasPredicate;
+		if ( !sqmTreats.isEmpty() ) {
+			if ( sqmTreats.size() == 1 ) {
+				// If there is only a single treat, combine the predicates just as they are
+				joinPredicate = SqmCreationHelper.combinePredicates(
+						sqmJoin.getJoinPredicate(),
+						( (SqmQualifiedJoin<?, ?>) sqmTreats.get( 0 ) ).getJoinPredicate()
+				);
+				treatPredicates = null;
+				hasPredicate = joinPredicate != null;
+			}
+			else {
+				// When there are multiple predicates, we have to apply type filters
+				joinPredicate = sqmJoin.getJoinPredicate();
+				treatPredicates = new SqmPredicate[sqmTreats.size()];
+				boolean hasTreatPredicate = false;
+				for ( int i = 0; i < sqmTreats.size(); i++ ) {
+					final var p = ( (SqmQualifiedJoin<?, ?>) sqmTreats.get( i ) ).getJoinPredicate();
+					treatPredicates[i] = p;
+					hasTreatPredicate = hasTreatPredicate || p != null;
+				}
+				hasPredicate = joinPredicate != null || hasTreatPredicate;
+			}
+		}
+		else {
+			joinPredicate = sqmJoin.getJoinPredicate();
+			treatPredicates = null;
+			hasPredicate = joinPredicate != null;
+		}
+
 		if ( pathSource instanceof PluralPersistentAttribute ) {
 			assert modelPart instanceof PluralAttributeMapping;
 
@@ -3363,7 +3402,7 @@ private TableGroup consumeAttributeJoin(
 					null,
 					sqmJoinType.getCorrespondingSqlJoinType(),
 					sqmJoin.isFetched(),
-					sqmJoin.getJoinPredicate() != null,
+					hasPredicate,
 					this
 			);
 
@@ -3379,7 +3418,7 @@ private TableGroup consumeAttributeJoin(
 					null,
 					sqmJoinType.getCorrespondingSqlJoinType(),
 					sqmJoin.isFetched(),
-					sqmJoin.getJoinPredicate() != null,
+					hasPredicate,
 					this
 			);
 
@@ -3388,7 +3427,7 @@ private TableGroup consumeAttributeJoin(
 			// Since this is an explicit join, we force the initialization of a possible lazy table group
 			// to retain the cardinality, but only if this is a non-trivial attribute join.
 			// Left or inner singular attribute joins without a predicate can be safely optimized away
-			if ( sqmJoin.getJoinPredicate() != null || sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT ) {
+			if ( hasPredicate || sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT ) {
 				joinedTableGroup.getPrimaryTableReference();
 			}
 		}
@@ -3425,14 +3464,26 @@ private TableGroup consumeAttributeJoin(
 		final TableGroupJoin joinForPredicate;
 
 		// add any additional join restrictions
-		if ( sqmJoin.getJoinPredicate() != null ) {
+		if ( hasPredicate ) {
 			if ( sqmJoin.isFetched() ) {
 				QueryLogging.QUERY_MESSAGE_LOGGER.debugf( "Join fetch [%s] is restricted", sqmJoinNavigablePath );
 			}
 
 			final SqmJoin<?, ?> oldJoin = currentlyProcessingJoin;
 			currentlyProcessingJoin = sqmJoin;
-			final Predicate predicate = visitNestedTopLevelPredicate( sqmJoin.getJoinPredicate() );
+			Predicate predicate = joinPredicate == null ? null : visitNestedTopLevelPredicate( joinPredicate );
+			if ( treatPredicates != null ) {
+				final Junction orPredicate = new Junction( Junction.Nature.DISJUNCTION );
+				for ( int i = 0; i < treatPredicates.length; i++ ) {
+					final EntityDomainType<?> treatType =
+							(EntityDomainType<?>) ( (SqmTreatedPath<?, ?>) sqmTreats.get( i ) ).getTreatTarget();
+					orPredicate.add( combinePredicates(
+							createTreatTypeRestriction( sqmJoin, treatType ),
+							treatPredicates[i] == null ? null : visitNestedTopLevelPredicate( treatPredicates[i] )
+					) );
+				}
+				predicate = predicate != null ? combinePredicates( predicate, orPredicate ) : orPredicate;
+			}
 			joinForPredicate = TableGroupJoinHelper.determineJoinForPredicateApply( joinedTableGroupJoin );
 			// If translating the join predicate didn't initialize the table group,
 			// we can safely apply it on the collection table group instead

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmFrom.java

@@ -166,7 +166,7 @@ public SqmPath<?> resolvePathPart(
 			if ( sqmJoin instanceof SqmSingularJoin<?, ?>
 					&& name.equals( sqmJoin.getReferencedPathSource().getPathName() ) ) {
 				final SqmAttributeJoin<?, ?> attributeJoin = (SqmAttributeJoin<?, ?>) sqmJoin;
-				if ( attributeJoin.getOn() == null ) {
+				if ( attributeJoin.getJoinPredicate() == null ) {
 					// todo (6.0): to match the expectation of the JPA spec I think we also have to check
 					//  that the join type is INNER or the default join type for the attribute,
 					//  but as far as I understand, in 5.x we expect to ignore this behavior