Skip to content

Commit 6365204

Browse files
NathanQingyangXubeikov
NathanQingyangXu
authored and
beikov
committed
HHH-13058 fix issue left join root cannot be replaced by correlated parent in subquery
1 parent afd6818 commit 6365204

File tree

7 files changed

+322
-17
lines changed

7 files changed

+322
-17
lines changed

‎hibernate-core/src/main/java/org/hibernate/query/criteria/internal/FromImplementor.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,7 @@ public interface FromImplementor<Z,X> extends PathImplementor<X>, From<Z,X> {
2222
FromImplementor<Z,X> correlateTo(CriteriaSubqueryImpl subquery);
2323
void prepareCorrelationDelegate(FromImplementor<Z,X> parent);
2424
FromImplementor<Z, X> getCorrelationParent();
25+
default boolean canBeReplacedByCorrelatedParentInSubQuery() {
26+
return isCorrelated();
27+
}
2528
}

‎hibernate-core/src/main/java/org/hibernate/query/criteria/internal/QueryStructure.java

Lines changed: 58 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -318,18 +318,33 @@ private void renderFromClause(StringBuilder jpaqlQuery, RenderingContext renderi
318318
final FromImplementor correlationParent = correlationRoot.getCorrelationParent();
319319
correlationParent.prepareAlias( renderingContext );
320320
final String correlationRootAlias = correlationParent.getAlias();
321-
for ( Join<?, ?> correlationJoin : correlationRoot.getJoins() ) {
322-
final JoinImplementor correlationJoinImpl = (JoinImplementor) correlationJoin;
323-
// IMPL NOTE: reuse the sep from above!
321+
if ( correlationRoot.canBeReplacedByCorrelatedParentInSubQuery() ) {
322+
for ( Join<?, ?> correlationJoin : correlationRoot.getJoins() ) {
323+
final JoinImplementor correlationJoinImpl = (JoinImplementor) correlationJoin;
324+
// IMPL NOTE: reuse the sep from above!
325+
jpaqlQuery.append( sep );
326+
correlationJoinImpl.prepareAlias( renderingContext );
327+
jpaqlQuery.append( correlationRootAlias )
328+
.append( '.' )
329+
.append( correlationJoinImpl.getAttribute().getName() )
330+
.append( " as " )
331+
.append( correlationJoinImpl.getAlias() );
332+
sep = ", ";
333+
renderJoins( jpaqlQuery, renderingContext, correlationJoinImpl.getJoins() );
334+
}
335+
}
336+
else {
337+
correlationRoot.prepareAlias( renderingContext );
324338
jpaqlQuery.append( sep );
325-
correlationJoinImpl.prepareAlias( renderingContext );
326-
jpaqlQuery.append( correlationRootAlias )
327-
.append( '.' )
328-
.append( correlationJoinImpl.getAttribute().getName() )
329-
.append( " as " )
330-
.append( correlationJoinImpl.getAlias() );
331339
sep = ", ";
332-
renderJoins( jpaqlQuery, renderingContext, correlationJoinImpl.getJoins() );
340+
jpaqlQuery.append( correlationRoot.renderTableExpression( renderingContext ) );
341+
renderJoins( jpaqlQuery, renderingContext, correlationRoot.getJoins() );
342+
if ( correlationRoot instanceof Root ) {
343+
Set<TreatedRoot> treats = ( (RootImpl) correlationRoot ).getTreats();
344+
for ( TreatedRoot treat : treats ) {
345+
renderJoins( jpaqlQuery, renderingContext, treat.getJoins() );
346+
}
347+
}
333348
}
334349
}
335350
}
@@ -341,20 +356,48 @@ private void renderFromClause(StringBuilder jpaqlQuery, RenderingContext renderi
341356
}
342357

343358
protected void renderWhereClause(StringBuilder jpaqlQuery, RenderingContext renderingContext) {
344-
if ( getRestriction() == null ) {
359+
final String correlationRestrictionWhereFragment = getCorrelationRestrictionsWhereFragment();
360+
if ( getRestriction() == null && correlationRestrictionWhereFragment.isEmpty() ) {
345361
return;
346362
}
347363

348364
renderingContext.getClauseStack().push( Clause.WHERE );
349365
try {
350-
jpaqlQuery.append( " where " )
351-
.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
366+
jpaqlQuery.append( " where " );
367+
jpaqlQuery.append( correlationRestrictionWhereFragment );
368+
if ( getRestriction() != null ) {
369+
if ( !correlationRestrictionWhereFragment.isEmpty() ) {
370+
jpaqlQuery.append( " and ( " );
371+
}
372+
jpaqlQuery.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
373+
if ( !correlationRestrictionWhereFragment.isEmpty() ) {
374+
jpaqlQuery.append( " )" );
375+
}
376+
}
352377
}
353378
finally {
354379
renderingContext.getClauseStack().pop();
355380
}
356381
}
357382

383+
private String getCorrelationRestrictionsWhereFragment() {
384+
if ( !isSubQuery || correlationRoots == null ) {
385+
return "";
386+
}
387+
StringBuilder buffer = new StringBuilder();
388+
String sep = "";
389+
for ( FromImplementor<?, ?> correlationRoot : correlationRoots ) {
390+
if ( !correlationRoot.canBeReplacedByCorrelatedParentInSubQuery() ) {
391+
buffer.append( sep );
392+
sep = " and ";
393+
buffer.append( correlationRoot.getAlias() )
394+
.append( "=" )
395+
.append( correlationRoot.getCorrelationParent().getAlias() );
396+
}
397+
}
398+
return buffer.toString();
399+
}
400+
358401
protected void renderGroupByClause(StringBuilder jpaqlQuery, RenderingContext renderingContext) {
359402
if ( getGroupings().isEmpty() ) {
360403
return;
@@ -395,7 +438,7 @@ private void renderHavingClause(StringBuilder jpaqlQuery, RenderingContext rende
395438
private void renderJoins(
396439
StringBuilder jpaqlQuery,
397440
RenderingContext renderingContext,
398-
Collection<Join<?,?>> joins) {
441+
Collection<? extends Join<?,?>> joins) {
399442
if ( joins == null ) {
400443
return;
401444
}
@@ -428,7 +471,7 @@ private String renderJoinType(JoinType joinType) {
428471
private void renderFetches(
429472
StringBuilder jpaqlQuery,
430473
RenderingContext renderingContext,
431-
Collection<Fetch> fetches) {
474+
Collection<? extends Fetch> fetches) {
432475
if ( fetches == null ) {
433476
return;
434477
}

‎hibernate-core/src/main/java/org/hibernate/query/criteria/internal/path/AbstractFromImpl.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ protected boolean canBeDereferenced() {
8181
@Override
8282
public void prepareAlias(RenderingContext renderingContext) {
8383
if ( getAlias() == null ) {
84-
if ( isCorrelated() ) {
84+
if ( canBeReplacedByCorrelatedParentInSubQuery() ) {
8585
setAlias( getCorrelationParent().getAlias() );
8686
}
8787
else {
@@ -207,7 +207,7 @@ public void prepareCorrelationDelegate(FromImplementor<Z, X> parent) {
207207

208208
@Override
209209
public String getAlias() {
210-
return isCorrelated() ? getCorrelationParent().getAlias() : super.getAlias();
210+
return canBeReplacedByCorrelatedParentInSubQuery() ? getCorrelationParent().getAlias() : super.getAlias();
211211
}
212212

213213
// JOINS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -589,4 +589,22 @@ public <X, Y> Fetch<X, Y> fetch(String attributeName, JoinType jt) {
589589
return (Fetch<X, Y>) fetch( (SingularAttribute) attribute, jt );
590590
}
591591
}
592+
593+
@Override
594+
public boolean canBeReplacedByCorrelatedParentInSubQuery() {
595+
if ( correlationParent == null ) {
596+
return false;
597+
}
598+
if ( joins == null ) {
599+
return true;
600+
}
601+
for ( Join<X, ?> join : joins ) {
602+
// HHH-13058: substitution implies INNER JOIN
603+
if ( join.getJoinType() == JoinType.LEFT ) {
604+
return false;
605+
}
606+
assert join.getJoinType() == JoinType.INNER;
607+
}
608+
return true;
609+
}
592610
}

‎hibernate-core/src/test/java/org/hibernate/query/criteria/internal/hhh13058/HHH13058Test.java

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
package org.hibernate.query.criteria.internal.hhh13058;
2+
3+
import java.util.Arrays;
4+
import java.util.HashSet;
5+
import java.util.List;
6+
import java.util.Set;
7+
import javax.persistence.criteria.CriteriaBuilder;
8+
import javax.persistence.criteria.CriteriaQuery;
9+
import javax.persistence.criteria.From;
10+
import javax.persistence.criteria.JoinType;
11+
import javax.persistence.criteria.Root;
12+
import javax.persistence.criteria.Subquery;
13+
14+
import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;
15+
16+
import org.hibernate.testing.TestForIssue;
17+
import org.junit.Before;
18+
import org.junit.Test;
19+
20+
import static org.hamcrest.CoreMatchers.is;
21+
import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
22+
import static org.junit.Assert.assertThat;
23+
24+
/**
25+
* @author Archie Cobbs
26+
* @author Nathan Xu
27+
*/
28+
@TestForIssue( jiraKey = "HHH-13058" )
29+
public class HHH13058Test extends BaseEntityManagerFunctionalTestCase {
30+
31+
private Set<Site> validSites;
32+
33+
private Task taskWithoutPatient;
34+
private Task taskWithPatientWithoutSite;
35+
private Task taskWithPatient1WithValidSite1;
36+
private Task taskWithPatient2WithValidSite1;
37+
private Task taskWithPatient3WithValidSite2;
38+
private Task taskWithPatientWithInvalidSite;
39+
40+
@Override
41+
public Class<?>[] getAnnotatedClasses() {
42+
return new Class<?>[] {
43+
Task.class,
44+
Patient.class,
45+
Site.class
46+
};
47+
}
48+
49+
@Before
50+
public void setUp() {
51+
doInJPA( this::entityManagerFactory, entityManager -> {
52+
final Site validSite1 = new Site();
53+
final Site validSite2 = new Site();
54+
final Site invalidSite = new Site();
55+
56+
entityManager.persist( validSite1 );
57+
entityManager.persist( validSite2 );
58+
entityManager.persist( invalidSite );
59+
60+
validSites = new HashSet<>( Arrays.asList( validSite1, validSite2 ) );
61+
62+
final Patient patientWithoutSite = new Patient();
63+
final Patient patient1WithValidSite1 = new Patient( validSite1 );
64+
final Patient patient2WithValidSite1 = new Patient( validSite1 );
65+
final Patient patient3WithValidSite2 = new Patient( validSite2 );
66+
final Patient patientWithInvalidSite = new Patient( invalidSite );
67+
68+
entityManager.persist( patientWithoutSite );
69+
entityManager.persist( patient1WithValidSite1 );
70+
entityManager.persist( patient2WithValidSite1 );
71+
entityManager.persist( patient3WithValidSite2 );
72+
entityManager.persist( patientWithInvalidSite );
73+
74+
taskWithoutPatient = new Task();
75+
taskWithoutPatient.description = "taskWithoutPatient";
76+
77+
taskWithPatientWithoutSite = new Task( patientWithoutSite );
78+
taskWithPatientWithoutSite.description = "taskWithPatientWithoutSite";
79+
80+
taskWithPatient1WithValidSite1 = new Task( patient1WithValidSite1 );
81+
taskWithPatient1WithValidSite1.description = "taskWithPatient1WithValidSite1";
82+
83+
taskWithPatient2WithValidSite1 = new Task( patient2WithValidSite1 );
84+
taskWithPatient2WithValidSite1.description = "taskWithPatient2WithValidSite1";
85+
86+
taskWithPatient3WithValidSite2 = new Task( patient3WithValidSite2 );
87+
taskWithPatient3WithValidSite2.description = "taskWithPatient3WithValidSite2";
88+
89+
taskWithPatientWithInvalidSite = new Task( patientWithInvalidSite );
90+
taskWithPatientWithInvalidSite.description = "taskWithPatientWithInvalidSite";
91+
92+
entityManager.persist( taskWithoutPatient );
93+
entityManager.persist( taskWithPatientWithoutSite );
94+
entityManager.persist( taskWithPatient1WithValidSite1 );
95+
entityManager.persist( taskWithPatient2WithValidSite1 );
96+
entityManager.persist( taskWithPatient3WithValidSite2 );
97+
entityManager.persist( taskWithPatientWithInvalidSite );
98+
} );
99+
}
100+
101+
@Test
102+
public void testCorrelateSubQueryLeftJoin() {
103+
doInJPA( this::entityManagerFactory, entityManager -> {
104+
final CriteriaBuilder builder = entityManager.getCriteriaBuilder();
105+
final CriteriaQuery<Task> outerQuery = builder.createQuery( Task.class );
106+
final Root<Task> outerTask = outerQuery.from( Task.class );
107+
108+
final Subquery<Task> subquery = outerQuery.subquery( Task.class );
109+
final Root<Task> subtask = subquery.correlate( outerTask );
110+
final From<Task, Patient> patient = subtask.join( Task_.patient, JoinType.LEFT );
111+
final From<Patient, Site> site = patient.join( Patient_.site, JoinType.LEFT );
112+
outerQuery.where(
113+
builder.exists(
114+
subquery.select( subtask )
115+
.where(
116+
builder.or(
117+
patient.isNull(),
118+
site.in( validSites )
119+
)
120+
)
121+
)
122+
);
123+
final List<Task> tasks = entityManager.createQuery( outerQuery ).getResultList();
124+
assertThat( new HashSet<>( tasks ), is( new HashSet<>( Arrays.asList(
125+
taskWithoutPatient,
126+
taskWithPatient1WithValidSite1,
127+
taskWithPatient2WithValidSite1,
128+
taskWithPatient3WithValidSite2
129+
) ) ) );
130+
131+
} );
132+
}
133+
}

‎hibernate-core/src/test/java/org/hibernate/query/criteria/internal/hhh13058/Patient.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package org.hibernate.query.criteria.internal.hhh13058;
2+
3+
import javax.persistence.Entity;
4+
import javax.persistence.FetchType;
5+
import javax.persistence.GeneratedValue;
6+
import javax.persistence.Id;
7+
import javax.persistence.ManyToOne;
8+
import javax.persistence.Table;
9+
10+
/**
11+
* @author Archie Cobbs
12+
* @author Nathan Xu
13+
*/
14+
@Entity(name = "Patient")
15+
@Table(name = "Patient")
16+
public class Patient {
17+
18+
@Id
19+
@GeneratedValue
20+
Long id;
21+
22+
@ManyToOne(fetch = FetchType.LAZY)
23+
Site site;
24+
25+
public Patient() {
26+
}
27+
28+
public Patient(Site site) {
29+
this.site = site;
30+
}
31+
32+
}

‎hibernate-core/src/test/java/org/hibernate/query/criteria/internal/hhh13058/Site.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
package org.hibernate.query.criteria.internal.hhh13058;
2+
3+
import javax.persistence.Entity;
4+
import javax.persistence.GeneratedValue;
5+
import javax.persistence.Id;
6+
import javax.persistence.Table;
7+
8+
/**
9+
* @author Archie Cobbs
10+
* @author Nathan Xu
11+
*/
12+
@Entity(name = "Site")
13+
@Table(name = "Site")
14+
public class Site {
15+
16+
@Id
17+
@GeneratedValue
18+
Long id;
19+
20+
}

0 commit comments

Comments
 (0)