Skip to content

Commit 7cc8af7

Browse files
beikovsebersole
beikov
authored and
sebersole
committed
HHH-19883 Respect predicates defined on treated join nodes
1 parent b57180a commit 7cc8af7

12 files changed

+451
-49
lines changed

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public class QualifiedJoinPathConsumer implements DotIdentifierConsumer {
4545
private final SqmJoinType joinType;
4646
private final boolean fetch;
4747
private final String alias;
48+
private final boolean allowReuse;
4849

4950
private ConsumerDelegate delegate;
5051
private boolean nested;
@@ -54,11 +55,13 @@ public QualifiedJoinPathConsumer(
5455
SqmJoinType joinType,
5556
boolean fetch,
5657
String alias,
58+
boolean allowReuse,
5759
SqmCreationState creationState) {
5860
this.sqmRoot = sqmRoot;
5961
this.joinType = joinType;
6062
this.fetch = fetch;
6163
this.alias = alias;
64+
this.allowReuse = allowReuse;
6265
this.creationState = creationState;
6366
}
6467

@@ -72,6 +75,8 @@ public QualifiedJoinPathConsumer(
7275
this.joinType = joinType;
7376
this.fetch = fetch;
7477
this.alias = alias;
78+
// This constructor is only used for entity names, so no need for join reuse
79+
this.allowReuse = false;
7580
this.creationState = creationState;
7681
this.delegate = new AttributeJoinDelegate(
7782
sqmFrom,
@@ -102,7 +107,13 @@ public void consumeIdentifier(String identifier, boolean isBase, boolean isTermi
102107
}
103108
else {
104109
assert delegate != null;
105-
delegate.consumeIdentifier( identifier, !nested && isTerminal, !( nested && isTerminal ) );
110+
delegate.consumeIdentifier(
111+
identifier,
112+
!nested && isTerminal,
113+
// Non-nested joins shall allow reuse, but nested ones (i.e. in treat)
114+
// only allow join reuse for non-terminal parts
115+
allowReuse && (!nested || !isTerminal)
116+
);
106117
}
107118
}
108119

@@ -192,7 +203,12 @@ private AttributeJoinDelegate resolveAlias(String identifier, boolean isTerminal
192203
if ( allowReuse ) {
193204
if ( !isTerminal ) {
194205
for ( SqmJoin<?, ?> sqmJoin : lhs.getSqmJoins() ) {
195-
if ( sqmJoin.getAlias() == null && sqmJoin.getModel() == subPathSource ) {
206+
// In order for an HQL join to be reusable, is must have the same path source,
207+
if ( sqmJoin.getModel() == subPathSource
208+
// must not have a join condition
209+
&& sqmJoin.getJoinPredicate() == null
210+
// and the same join type
211+
&& sqmJoin.getSqmJoinType() == joinType ) {
196212
return sqmJoin;
197213
}
198214
}

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2188,10 +2188,12 @@ protected <X> void consumeJoin(HqlParser.JoinContext parserJoin, SqmRoot<X> sqmR
21882188
throw new SemanticException( "The 'from' clause of a subquery has a 'fetch'", query );
21892189
}
21902190

2191-
dotIdentifierConsumerStack.push( new QualifiedJoinPathConsumer( sqmRoot, joinType, fetch, alias, this ) );
2191+
final var joinRestrictionContext = parserJoin.joinRestriction();
2192+
// Joins are allowed to be reused if they don't have a join condition
2193+
final var allowReuse = joinRestrictionContext == null;
2194+
dotIdentifierConsumerStack.push( new QualifiedJoinPathConsumer( sqmRoot, joinType, fetch, alias, allowReuse, this ) );
21922195
try {
21932196
final SqmJoin<X, ?> join = getJoin( sqmRoot, joinType, qualifiedJoinTargetContext, alias, fetch );
2194-
final var joinRestrictionContext = parserJoin.joinRestriction();
21952197
if ( join instanceof SqmEntityJoin<?,?> || join instanceof SqmDerivedJoin<?> || join instanceof SqmCteJoin<?> ) {
21962198
sqmRoot.addSqmJoin( join );
21972199
}
@@ -2329,7 +2331,7 @@ protected void consumeJpaCollectionJoin(HqlParser.JpaCollectionJoinContext ctx,
23292331
final String alias = extractAlias( ctx.variable() );
23302332
dotIdentifierConsumerStack.push(
23312333
// According to JPA spec 4.4.6 this is an inner join
2332-
new QualifiedJoinPathConsumer( sqmRoot, SqmJoinType.INNER, false, alias, this )
2334+
new QualifiedJoinPathConsumer( sqmRoot, SqmJoinType.INNER, false, alias, true, this )
23332335
);
23342336

23352337
try {

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

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@
120120
import org.hibernate.query.sqm.mutation.internal.SqmInsertStrategyHelper;
121121
import org.hibernate.query.sqm.produce.function.internal.PatternRenderer;
122122
import org.hibernate.query.sqm.spi.BaseSemanticQueryWalker;
123+
import org.hibernate.query.sqm.spi.SqmCreationHelper;
123124
import org.hibernate.query.sqm.sql.internal.AnyDiscriminatorPathInterpretation;
124125
import org.hibernate.query.sqm.sql.internal.AsWrappedExpression;
125126
import org.hibernate.query.sqm.sql.internal.BasicValuedPathInterpretation;
@@ -226,6 +227,7 @@
226227
import org.hibernate.query.sqm.tree.from.SqmFunctionJoin;
227228
import org.hibernate.query.sqm.tree.from.SqmJoin;
228229
import org.hibernate.query.sqm.tree.from.SqmRoot;
230+
import org.hibernate.query.sqm.tree.from.SqmTreatedAttributeJoin;
229231
import org.hibernate.query.sqm.tree.insert.SqmConflictClause;
230232
import org.hibernate.query.sqm.tree.insert.SqmConflictUpdateAction;
231233
import org.hibernate.query.sqm.tree.insert.SqmInsertSelectStatement;
@@ -2684,7 +2686,12 @@ protected void consumeFromClauseCorrelatedRoot(SqmRoot<?> sqmRoot) {
26842686
// as roots anyway, so nothing to worry about
26852687

26862688
// LOG.tracef( "Resolved SqmRoot [%s] to correlated TableGroup [%s]", sqmRoot, tableGroup );
2687-
consumeExplicitJoins( from, tableGroup );
2689+
if ( from instanceof SqmRoot<?> correlatedRoot ) {
2690+
consumeJoins( correlatedRoot, fromClauseIndex, tableGroup );
2691+
}
2692+
else {
2693+
consumeExplicitJoins( from, tableGroup );
2694+
}
26882695
return;
26892696
}
26902697
else {
@@ -3339,6 +3346,38 @@ private TableGroup consumeAttributeJoin(
33393346
.findSubPart( pathSource.getPathName(),
33403347
resolveExplicitTreatTarget( sqmJoin, this ) );
33413348

3349+
final List<SqmTreatedFrom<?, ?, ?>> sqmTreats = sqmJoin.getSqmTreats();
3350+
final SqmPredicate joinPredicate;
3351+
final SqmPredicate[] treatPredicates;
3352+
final boolean hasPredicate;
3353+
if ( !sqmTreats.isEmpty() ) {
3354+
if ( sqmTreats.size() == 1 ) {
3355+
// If there is only a single treat, combine the predicates just as they are
3356+
joinPredicate = SqmCreationHelper.combinePredicates(
3357+
sqmJoin.getJoinPredicate(),
3358+
( (SqmTreatedAttributeJoin<?, ?, ?>) sqmTreats.get( 0 ) ).getJoinPredicate()
3359+
);
3360+
treatPredicates = null;
3361+
hasPredicate = joinPredicate != null;
3362+
}
3363+
else {
3364+
// When there are multiple predicates, we have to apply type filters
3365+
joinPredicate = sqmJoin.getJoinPredicate();
3366+
treatPredicates = new SqmPredicate[sqmTreats.size()];
3367+
boolean hasTreatPredicate = false;
3368+
for ( int i = 0; i < sqmTreats.size(); i++ ) {
3369+
final var p = ( (SqmTreatedAttributeJoin<?, ?, ?>) sqmTreats.get( i ) ).getJoinPredicate();
3370+
treatPredicates[i] = p;
3371+
hasTreatPredicate = hasTreatPredicate || p != null;
3372+
}
3373+
hasPredicate = joinPredicate != null || hasTreatPredicate;
3374+
}
3375+
}
3376+
else {
3377+
joinPredicate = sqmJoin.getJoinPredicate();
3378+
treatPredicates = null;
3379+
hasPredicate = joinPredicate != null;
3380+
}
33423381
final TableGroupJoin joinedTableGroupJoin =
33433382
((TableGroupJoinProducer) modelPart)
33443383
.createTableGroupJoin(
@@ -3348,7 +3387,7 @@ private TableGroup consumeAttributeJoin(
33483387
null,
33493388
sqmJoinType.getCorrespondingSqlJoinType(),
33503389
sqmJoin.isFetched(),
3351-
sqmJoin.getJoinPredicate() != null,
3390+
hasPredicate,
33523391
this
33533392
);
33543393
final TableGroup joinedTableGroup = joinedTableGroupJoin.getJoinedGroup();
@@ -3363,8 +3402,7 @@ private TableGroup consumeAttributeJoin(
33633402
// Since this is an explicit join, we force the initialization of a possible lazy table group
33643403
// to retain the cardinality, but only if this is a non-trivial attribute join.
33653404
// Left or inner singular attribute joins without a predicate can be safely optimized away
3366-
if ( sqmJoin.getJoinPredicate() != null
3367-
|| sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT ) {
3405+
if ( hasPredicate || sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT ) {
33683406
joinedTableGroup.getPrimaryTableReference();
33693407
}
33703408
}
@@ -3399,14 +3437,25 @@ private TableGroup consumeAttributeJoin(
33993437
final TableGroupJoin joinForPredicate;
34003438

34013439
// add any additional join restrictions
3402-
if ( sqmJoin.getJoinPredicate() != null ) {
3440+
if ( hasPredicate ) {
34033441
if ( sqmJoin.isFetched() ) {
34043442
QUERY_MESSAGE_LOGGER.debugf( "Join fetch [%s] is restricted", sqmJoinNavigablePath );
34053443
}
34063444

34073445
final SqmJoin<?, ?> oldJoin = currentlyProcessingJoin;
34083446
currentlyProcessingJoin = sqmJoin;
3409-
final Predicate predicate = visitNestedTopLevelPredicate( sqmJoin.getJoinPredicate() );
3447+
Predicate predicate = joinPredicate == null ? null : visitNestedTopLevelPredicate( joinPredicate );
3448+
if ( treatPredicates != null ) {
3449+
final Junction orPredicate = new Junction( Junction.Nature.DISJUNCTION );
3450+
for ( int i = 0; i < treatPredicates.length; i++ ) {
3451+
final var treatType = (EntityDomainType<?>) sqmTreats.get( i ).getTreatTarget();
3452+
orPredicate.add( combinePredicates(
3453+
createTreatTypeRestriction( sqmJoin, treatType ),
3454+
treatPredicates[i] == null ? null : visitNestedTopLevelPredicate( treatPredicates[i] )
3455+
) );
3456+
}
3457+
predicate = predicate != null ? combinePredicates( predicate, orPredicate ) : orPredicate;
3458+
}
34103459
joinForPredicate = determineJoinForPredicateApply( joinedTableGroupJoin );
34113460
// If translating the join predicate didn't initialize the table group,
34123461
// we can safely apply it on the collection table group instead

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public SqmPath<?> resolvePathPart(
168168
// We can only match singular joins here, as plural path parts are interpreted like sub-queries
169169
if ( sqmJoin instanceof SqmSingularJoin<?, ?> attributeJoin
170170
&& name.equals( sqmJoin.getReferencedPathSource().getPathName() ) ) {
171-
if ( attributeJoin.getOn() == null ) {
171+
if ( attributeJoin.getJoinPredicate() == null ) {
172172
// todo (6.0): to match the expectation of the JPA spec I think we also have to check
173173
// that the join type is INNER or the default join type for the attribute,
174174
// but as far as I understand, in 5.x we expect to ignore this behavior

‎hibernate-core/src/main/java/org/hibernate/query/sqm/tree/from/SqmFromClause.java‎

Lines changed: 75 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
import org.hibernate.query.sqm.tree.SqmCacheable;
1313
import org.hibernate.query.sqm.tree.SqmCopyContext;
14+
import org.hibernate.query.sqm.tree.SqmJoinType;
1415
import org.hibernate.query.sqm.tree.SqmRenderContext;
16+
import org.hibernate.query.sqm.tree.domain.SqmTreatedFrom;
1517
import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
1618

1719
import static java.util.Collections.emptyList;
@@ -113,62 +115,98 @@ public void appendHqlString(StringBuilder sb, SqmRenderContext context) {
113115
}
114116

115117
public static void appendJoins(SqmFrom<?, ?> sqmFrom, StringBuilder sb, SqmRenderContext context) {
116-
for ( SqmJoin<?, ?> sqmJoin : sqmFrom.getSqmJoins() ) {
117-
switch ( sqmJoin.getSqmJoinType() ) {
118-
case LEFT:
119-
sb.append( " left join " );
120-
break;
121-
case RIGHT:
122-
sb.append( " right join " );
123-
break;
124-
case INNER:
125-
sb.append( " join " );
126-
break;
127-
case FULL:
128-
sb.append( " full join " );
129-
break;
130-
case CROSS:
131-
sb.append( " cross join " );
132-
break;
133-
}
118+
if ( sqmFrom instanceof SqmRoot<?> root && root.getOrderedJoins() != null ) {
119+
appendJoins( sqmFrom, root.getOrderedJoins(), sb, false, context );
120+
}
121+
else {
122+
appendJoins( sqmFrom, sqmFrom.getSqmJoins(), sb, true, context );
123+
}
124+
}
125+
126+
private static void appendJoins(SqmFrom<?, ?> sqmFrom, List<? extends SqmJoin<?, ?>> joins, StringBuilder sb, boolean transitive, SqmRenderContext context) {
127+
for ( SqmJoin<?, ?> sqmJoin : joins ) {
128+
appendJoinType( sb, sqmJoin.getSqmJoinType() );
134129
if ( sqmJoin instanceof SqmAttributeJoin<?, ?> attributeJoin ) {
135-
if ( sqmFrom instanceof SqmTreatedPath<?, ?> treatedPath ) {
136-
sb.append( "treat(" );
137-
treatedPath.getWrappedPath().appendHqlString( sb, context );
138-
// sb.append( treatedPath.getWrappedPath().resolveAlias( context ) );
139-
sb.append( " as " ).append( treatedPath.getTreatTarget().getTypeName() ).append( ')' );
130+
final List<SqmTreatedFrom<?, ?, ?>> sqmTreats = attributeJoin.getSqmTreats();
131+
if ( attributeJoin.isImplicitJoin() && !sqmTreats.isEmpty() ) {
132+
for ( int i = 0; i < sqmTreats.size(); i++ ) {
133+
final var treatJoin = (SqmTreatedAttributeJoin<?, ?, ?>) sqmTreats.get( i );
134+
if ( i != 0 ) {
135+
appendJoinType( sb, sqmJoin.getSqmJoinType() );
136+
}
137+
sb.append( "treat(" );
138+
appendAttributeJoin( sqmFrom, sb, context, attributeJoin );
139+
sb.append( " as " );
140+
sb.append( treatJoin.getTreatTarget().getTypeName() );
141+
sb.append( ')' );
142+
appendJoinAliasAndOnClause( sb, context, treatJoin );
143+
if ( transitive ) {
144+
appendJoins( treatJoin, sb, context );
145+
}
146+
}
140147
}
141148
else {
142-
sb.append( sqmFrom.resolveAlias( context ) );
143-
}
144-
sb.append( '.' ).append( attributeJoin.getAttribute().getName() );
145-
sb.append( ' ' ).append( sqmJoin.resolveAlias( context ) );
146-
if ( attributeJoin.getJoinPredicate() != null ) {
147-
sb.append( " on " );
148-
attributeJoin.getJoinPredicate().appendHqlString( sb, context );
149+
appendAttributeJoin( sqmFrom, sb, context, attributeJoin );
150+
appendJoinAliasAndOnClause( sb, context, attributeJoin );
151+
if ( transitive ) {
152+
appendJoins( attributeJoin, sb, context );
153+
appendTreatJoins( sqmJoin, sb, context );
154+
}
149155
}
150-
appendJoins( sqmJoin, sb, context );
151156
}
152157
else if ( sqmJoin instanceof SqmCrossJoin<?> sqmCrossJoin ) {
153158
sb.append( sqmCrossJoin.getEntityName() );
154159
sb.append( ' ' ).append( sqmCrossJoin.resolveAlias( context ) );
155-
appendJoins( sqmJoin, sb, context );
160+
if ( transitive ) {
161+
appendJoins( sqmJoin, sb, context );
162+
appendTreatJoins( sqmJoin, sb, context );
163+
}
156164
}
157165
else if ( sqmJoin instanceof SqmEntityJoin<?, ?> sqmEntityJoin ) {
158166
sb.append( sqmEntityJoin.getEntityName() );
159-
sb.append( ' ' ).append( sqmJoin.resolveAlias( context ) );
160-
if ( sqmEntityJoin.getJoinPredicate() != null ) {
161-
sb.append( " on " );
162-
sqmEntityJoin.getJoinPredicate().appendHqlString( sb, context );
167+
appendJoinAliasAndOnClause( sb, context, sqmEntityJoin );
168+
if ( transitive ) {
169+
appendJoins( sqmJoin, sb, context );
170+
appendTreatJoins( sqmJoin, sb, context );
163171
}
164-
appendJoins( sqmJoin, sb, context );
165172
}
166173
else {
167174
throw new UnsupportedOperationException( "Unsupported join: " + sqmJoin );
168175
}
169176
}
170177
}
171178

179+
private static void appendJoinAliasAndOnClause(StringBuilder sb, SqmRenderContext context, SqmJoin<?, ?> join) {
180+
sb.append( ' ' ).append( join.resolveAlias( context ) );
181+
if ( join.getJoinPredicate() != null ) {
182+
sb.append( " on " );
183+
join.getJoinPredicate().appendHqlString( sb, context );
184+
}
185+
}
186+
187+
private static void appendAttributeJoin(SqmFrom<?, ?> sqmFrom, StringBuilder sb, SqmRenderContext context, SqmAttributeJoin<?, ?> attributeJoin) {
188+
if ( sqmFrom instanceof SqmTreatedPath<?, ?> treatedPath ) {
189+
sb.append( "treat(" );
190+
treatedPath.getWrappedPath().appendHqlString( sb, context );
191+
// sb.append( treatedPath.getWrappedPath().resolveAlias( context ) );
192+
sb.append( " as " ).append( treatedPath.getTreatTarget().getTypeName() ).append( ')' );
193+
}
194+
else {
195+
sb.append( sqmFrom.resolveAlias( context ) );
196+
}
197+
sb.append( '.' ).append( attributeJoin.getAttribute().getName() );
198+
}
199+
200+
private static void appendJoinType(StringBuilder sb, SqmJoinType sqmJoinType) {
201+
sb.append( switch ( sqmJoinType ) {
202+
case LEFT -> " left join ";
203+
case RIGHT -> " right join ";
204+
case INNER -> " join ";
205+
case FULL -> " full join ";
206+
case CROSS -> " cross join ";
207+
} );
208+
}
209+
172210
private void appendJoins(SqmFrom<?, ?> sqmFrom, String correlationPrefix, StringBuilder sb, SqmRenderContext context) {
173211
String separator = "";
174212
for ( SqmJoin<?, ?> sqmJoin : sqmFrom.getSqmJoins() ) {

0 commit comments

Comments
 (0)