Skip to content

Commit

Permalink
HHH-13058 fix issue left join root cannot be replaced by correlated p…
Browse files Browse the repository at this point in the history
…arent in subquery
  • Loading branch information
NathanQingyangXu committed Sep 1, 2020
1 parent efa7e66 commit 2662edf
Show file tree
Hide file tree
Showing 7 changed files with 322 additions and 17 deletions.
Expand Up @@ -22,4 +22,7 @@ public interface FromImplementor<Z,X> extends PathImplementor<X>, From<Z,X> {
FromImplementor<Z,X> correlateTo(CriteriaSubqueryImpl subquery);
void prepareCorrelationDelegate(FromImplementor<Z,X> parent);
FromImplementor<Z, X> getCorrelationParent();
default boolean canBeReplacedByCorrelatedParentInSubQuery() {
return isCorrelated();
}
}
Expand Up @@ -318,18 +318,33 @@ private void renderFromClause(StringBuilder jpaqlQuery, RenderingContext renderi
final FromImplementor correlationParent = correlationRoot.getCorrelationParent();
correlationParent.prepareAlias( renderingContext );
final String correlationRootAlias = correlationParent.getAlias();
for ( Join<?, ?> correlationJoin : correlationRoot.getJoins() ) {
final JoinImplementor correlationJoinImpl = (JoinImplementor) correlationJoin;
// IMPL NOTE: reuse the sep from above!
if ( correlationRoot.canBeReplacedByCorrelatedParentInSubQuery() ) {
for ( Join<?, ?> correlationJoin : correlationRoot.getJoins() ) {
final JoinImplementor correlationJoinImpl = (JoinImplementor) correlationJoin;
// IMPL NOTE: reuse the sep from above!
jpaqlQuery.append( sep );
correlationJoinImpl.prepareAlias( renderingContext );
jpaqlQuery.append( correlationRootAlias )
.append( '.' )
.append( correlationJoinImpl.getAttribute().getName() )
.append( " as " )
.append( correlationJoinImpl.getAlias() );
sep = ", ";
renderJoins( jpaqlQuery, renderingContext, correlationJoinImpl.getJoins() );
}
}
else {
correlationRoot.prepareAlias( renderingContext );
jpaqlQuery.append( sep );
correlationJoinImpl.prepareAlias( renderingContext );
jpaqlQuery.append( correlationRootAlias )
.append( '.' )
.append( correlationJoinImpl.getAttribute().getName() )
.append( " as " )
.append( correlationJoinImpl.getAlias() );
sep = ", ";
renderJoins( jpaqlQuery, renderingContext, correlationJoinImpl.getJoins() );
jpaqlQuery.append( correlationRoot.renderTableExpression( renderingContext ) );
renderJoins( jpaqlQuery, renderingContext, correlationRoot.getJoins() );
if ( correlationRoot instanceof Root ) {
Set<TreatedRoot> treats = ( (RootImpl) correlationRoot ).getTreats();
for ( TreatedRoot treat : treats ) {
renderJoins( jpaqlQuery, renderingContext, treat.getJoins() );
}
}
}
}
}
Expand All @@ -341,20 +356,48 @@ private void renderFromClause(StringBuilder jpaqlQuery, RenderingContext renderi
}

protected void renderWhereClause(StringBuilder jpaqlQuery, RenderingContext renderingContext) {
if ( getRestriction() == null ) {
final String correlationRestrictionWhereFragment = getCorrelationRestrictionsWhereFragment();
if ( getRestriction() == null && correlationRestrictionWhereFragment.isEmpty() ) {
return;
}

renderingContext.getClauseStack().push( Clause.WHERE );
try {
jpaqlQuery.append( " where " )
.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
jpaqlQuery.append( " where " );
jpaqlQuery.append( correlationRestrictionWhereFragment );
if ( getRestriction() != null ) {
if ( !correlationRestrictionWhereFragment.isEmpty() ) {
jpaqlQuery.append( " and ( " );
}
jpaqlQuery.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
if ( !correlationRestrictionWhereFragment.isEmpty() ) {
jpaqlQuery.append( " )" );
}
}
}
finally {
renderingContext.getClauseStack().pop();
}
}

private String getCorrelationRestrictionsWhereFragment() {
if ( !isSubQuery || correlationRoots == null ) {
return "";
}
StringBuilder buffer = new StringBuilder();
String sep = "";
for ( FromImplementor<?, ?> correlationRoot : correlationRoots ) {
if ( !correlationRoot.canBeReplacedByCorrelatedParentInSubQuery() ) {
buffer.append( sep );
sep = " and ";
buffer.append( correlationRoot.getAlias() )
.append( "=" )
.append( correlationRoot.getCorrelationParent().getAlias() );
}
}
return buffer.toString();
}

protected void renderGroupByClause(StringBuilder jpaqlQuery, RenderingContext renderingContext) {
if ( getGroupings().isEmpty() ) {
return;
Expand Down Expand Up @@ -395,7 +438,7 @@ private void renderHavingClause(StringBuilder jpaqlQuery, RenderingContext rende
private void renderJoins(
StringBuilder jpaqlQuery,
RenderingContext renderingContext,
Collection<Join<?,?>> joins) {
Collection<? extends Join<?,?>> joins) {
if ( joins == null ) {
return;
}
Expand Down Expand Up @@ -428,7 +471,7 @@ private String renderJoinType(JoinType joinType) {
private void renderFetches(
StringBuilder jpaqlQuery,
RenderingContext renderingContext,
Collection<Fetch> fetches) {
Collection<? extends Fetch> fetches) {
if ( fetches == null ) {
return;
}
Expand Down
Expand Up @@ -81,7 +81,7 @@ protected boolean canBeDereferenced() {
@Override
public void prepareAlias(RenderingContext renderingContext) {
if ( getAlias() == null ) {
if ( isCorrelated() ) {
if ( canBeReplacedByCorrelatedParentInSubQuery() ) {
setAlias( getCorrelationParent().getAlias() );
}
else {
Expand Down Expand Up @@ -207,7 +207,7 @@ public void prepareCorrelationDelegate(FromImplementor<Z, X> parent) {

@Override
public String getAlias() {
return isCorrelated() ? getCorrelationParent().getAlias() : super.getAlias();
return canBeReplacedByCorrelatedParentInSubQuery() ? getCorrelationParent().getAlias() : super.getAlias();
}

// JOINS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -589,4 +589,22 @@ public <X, Y> Fetch<X, Y> fetch(String attributeName, JoinType jt) {
return (Fetch<X, Y>) fetch( (SingularAttribute) attribute, jt );
}
}

@Override
public boolean canBeReplacedByCorrelatedParentInSubQuery() {
if ( correlationParent == null ) {
return false;
}
if ( joins == null ) {
return true;
}
for ( Join<X, ?> join : joins ) {
// HHH-13058: substitution implies INNER JOIN
if ( join.getJoinType() == JoinType.LEFT ) {
return false;
}
assert join.getJoinType() == JoinType.INNER;
}
return true;
}
}
@@ -0,0 +1,133 @@
package org.hibernate.query.criteria.internal.hhh13058;

import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import javax.persistence.criteria.CriteriaBuilder;
import javax.persistence.criteria.CriteriaQuery;
import javax.persistence.criteria.From;
import javax.persistence.criteria.JoinType;
import javax.persistence.criteria.Root;
import javax.persistence.criteria.Subquery;

import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase;

import org.hibernate.testing.TestForIssue;
import org.junit.Before;
import org.junit.Test;

import static org.hamcrest.CoreMatchers.is;
import static org.hibernate.testing.transaction.TransactionUtil.doInJPA;
import static org.junit.Assert.assertThat;

/**
* @author Archie Cobbs
* @author Nathan Xu
*/
@TestForIssue( jiraKey = "HHH-13058" )
public class HHH13058Test extends BaseEntityManagerFunctionalTestCase {

private Set<Site> validSites;

private Task taskWithoutPatient;
private Task taskWithPatientWithoutSite;
private Task taskWithPatient1WithValidSite1;
private Task taskWithPatient2WithValidSite1;
private Task taskWithPatient3WithValidSite2;
private Task taskWithPatientWithInvalidSite;

@Override
public Class<?>[] getAnnotatedClasses() {
return new Class<?>[] {
Task.class,
Patient.class,
Site.class
};
}

@Before
public void setUp() {
doInJPA( this::entityManagerFactory, entityManager -> {
final Site validSite1 = new Site();
final Site validSite2 = new Site();
final Site invalidSite = new Site();

entityManager.persist( validSite1 );
entityManager.persist( validSite2 );
entityManager.persist( invalidSite );

validSites = new HashSet<>( Arrays.asList( validSite1, validSite2 ) );

final Patient patientWithoutSite = new Patient();
final Patient patient1WithValidSite1 = new Patient( validSite1 );
final Patient patient2WithValidSite1 = new Patient( validSite1 );
final Patient patient3WithValidSite2 = new Patient( validSite2 );
final Patient patientWithInvalidSite = new Patient( invalidSite );

entityManager.persist( patientWithoutSite );
entityManager.persist( patient1WithValidSite1 );
entityManager.persist( patient2WithValidSite1 );
entityManager.persist( patient3WithValidSite2 );
entityManager.persist( patientWithInvalidSite );

taskWithoutPatient = new Task();
taskWithoutPatient.description = "taskWithoutPatient";

taskWithPatientWithoutSite = new Task( patientWithoutSite );
taskWithPatientWithoutSite.description = "taskWithPatientWithoutSite";

taskWithPatient1WithValidSite1 = new Task( patient1WithValidSite1 );
taskWithPatient1WithValidSite1.description = "taskWithPatient1WithValidSite1";

taskWithPatient2WithValidSite1 = new Task( patient2WithValidSite1 );
taskWithPatient2WithValidSite1.description = "taskWithPatient2WithValidSite1";

taskWithPatient3WithValidSite2 = new Task( patient3WithValidSite2 );
taskWithPatient3WithValidSite2.description = "taskWithPatient3WithValidSite2";

taskWithPatientWithInvalidSite = new Task( patientWithInvalidSite );
taskWithPatientWithInvalidSite.description = "taskWithPatientWithInvalidSite";

entityManager.persist( taskWithoutPatient );
entityManager.persist( taskWithPatientWithoutSite );
entityManager.persist( taskWithPatient1WithValidSite1 );
entityManager.persist( taskWithPatient2WithValidSite1 );
entityManager.persist( taskWithPatient3WithValidSite2 );
entityManager.persist( taskWithPatientWithInvalidSite );
} );
}

@Test
public void testCorrelateSubQueryLeftJoin() {
doInJPA( this::entityManagerFactory, entityManager -> {
final CriteriaBuilder builder = entityManager.getCriteriaBuilder();
final CriteriaQuery<Task> outerQuery = builder.createQuery( Task.class );
final Root<Task> outerTask = outerQuery.from( Task.class );

final Subquery<Task> subquery = outerQuery.subquery( Task.class );
final Root<Task> subtask = subquery.correlate( outerTask );
final From<Task, Patient> patient = subtask.join( Task_.patient, JoinType.LEFT );
final From<Patient, Site> site = patient.join( Patient_.site, JoinType.LEFT );
outerQuery.where(
builder.exists(
subquery.select( subtask )
.where(
builder.or(
patient.isNull(),
site.in( validSites )
)
)
)
);
final List<Task> tasks = entityManager.createQuery( outerQuery ).getResultList();
assertThat( new HashSet<>( tasks ), is( new HashSet<>( Arrays.asList(
taskWithoutPatient,
taskWithPatient1WithValidSite1,
taskWithPatient2WithValidSite1,
taskWithPatient3WithValidSite2
) ) ) );

} );
}
}
@@ -0,0 +1,32 @@
package org.hibernate.query.criteria.internal.hhh13058;

import javax.persistence.Entity;
import javax.persistence.FetchType;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.ManyToOne;
import javax.persistence.Table;

/**
* @author Archie Cobbs
* @author Nathan Xu
*/
@Entity(name = "Patient")
@Table(name = "Patient")
public class Patient {

@Id
@GeneratedValue
Long id;

@ManyToOne(fetch = FetchType.LAZY)
Site site;

public Patient() {
}

public Patient(Site site) {
this.site = site;
}

}
@@ -0,0 +1,20 @@
package org.hibernate.query.criteria.internal.hhh13058;

import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.Table;

/**
* @author Archie Cobbs
* @author Nathan Xu
*/
@Entity(name = "Site")
@Table(name = "Site")
public class Site {

@Id
@GeneratedValue
Long id;

}

0 comments on commit 2662edf

Please sign in to comment.