HHH-4768 - Bug in how Criteria Subquery correlations are handled

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@18481 1b8cb986-b30d-0410-93ca-fae66ebed9b2

entitymanager/src/main/java/org/hibernate/ejb/criteria/CriteriaSubqueryImpl.java

@@ -26,11 +26,9 @@ package org.hibernate.ejb.criteria;
 import java.io.Serializable;
 import java.util.List;
 import java.util.Set;
-import java.util.HashSet;
 import javax.persistence.criteria.AbstractQuery;
 import javax.persistence.criteria.CollectionJoin;
 import javax.persistence.criteria.Expression;
-import javax.persistence.criteria.Fetch;
 import javax.persistence.criteria.Join;
 import javax.persistence.criteria.ListJoin;
 import javax.persistence.criteria.MapJoin;
@@ -53,18 +51,6 @@ public class CriteriaSubqueryImpl<T> extends ExpressionImpl<T> implements Subque
 	private final AbstractQuery<?> parent;
 	private final QueryStructure<T> queryStructure;
 
-	private Set<Join<?, ?>> correlatedJoins = new HashSet<Join<?,?>>();
-
-	private final FromImplementor.JoinScope joinScope = new FromImplementor.JoinScope() {
-		public void addJoin(Join join) {
-			correlatedJoins.add( join );
-		}
-
-		public void addFetch(Fetch fetch) {
-			throw new UnsupportedOperationException( "Cannot define fetch from a subquery correlation" );
-		}
-	};
-
 	public CriteriaSubqueryImpl(
 			CriteriaBuilderImpl criteriaBuilder,
 			Class<T> javaType,
@@ -74,15 +60,6 @@ public class CriteriaSubqueryImpl<T> extends ExpressionImpl<T> implements Subque
 		this.queryStructure = new QueryStructure<T>( this, criteriaBuilder );
 	}
 
-	/**
-	 * Get the scope used to scope joins to this subquery.
-	 *
-	 * @return The subquery's join scope.
-	 */
-	public FromImplementor.JoinScope getJoinScope() {
-		return joinScope;
-	}
-
 	/**
 	 * {@inheritDoc}
 	 */
@@ -235,49 +212,61 @@ public class CriteriaSubqueryImpl<T> extends ExpressionImpl<T> implements Subque
 	 * {@inheritDoc}
 	 */
     public Set<Join<?, ?>> getCorrelatedJoins() {
-		return correlatedJoins;
+		return queryStructure.collectCorrelatedJoins();
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <Y> Root<Y> correlate(Root<Y> source) {
-		return ( ( RootImpl<Y> ) source ).correlateTo( this );
+		final RootImpl<Y> correlation = ( ( RootImpl<Y> ) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <X, Y> Join<X, Y> correlate(Join<X, Y> source) {
-		return ( (JoinImplementor<X,Y>) source ).correlateTo( this );
+		final JoinImplementor<X,Y> correlation = ( (JoinImplementor<X,Y>) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <X, Y> CollectionJoin<X, Y> correlate(CollectionJoin<X, Y> source) {
-		return ( (CollectionJoinImplementor<X,Y>) source ).correlateTo( this );
+		final CollectionJoinImplementor<X,Y> correlation = ( (CollectionJoinImplementor<X,Y>) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <X, Y> SetJoin<X, Y> correlate(SetJoin<X, Y> source) {
-		return ( (SetJoinImplementor<X,Y>) source ).correlateTo( this );
+		final SetJoinImplementor<X,Y> correlation = ( (SetJoinImplementor<X,Y>) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <X, Y> ListJoin<X, Y> correlate(ListJoin<X, Y> source) {
-		return ( (ListJoinImplementor<X,Y>) source ).correlateTo( this );
+		final ListJoinImplementor<X,Y> correlation = ( (ListJoinImplementor<X,Y>) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	public <X, K, V> MapJoin<X, K, V> correlate(MapJoin<X, K, V> source) {
-		return ( (MapJoinImplementor<X, K, V>) source ).correlateTo( this );
+		final MapJoinImplementor<X, K, V> correlation = ( (MapJoinImplementor<X, K, V>) source ).correlateTo( this );
+		queryStructure.addCorrelationRoot( correlation );
+		return correlation;
 	}
 
 	/**

entitymanager/src/main/java/org/hibernate/ejb/criteria/FromImplementor.java

@@ -37,14 +37,8 @@ public interface FromImplementor<Z,X> extends PathImplementor<X>, From<Z,X> {
 	public void prepareAlias(CriteriaQueryCompiler.RenderingContext renderingContext);
 	public String renderTableExpression(CriteriaQueryCompiler.RenderingContext renderingContext);
 
-	/**
-	 * Helper contract used to define who/what keeps track of joins and fetches made from this <tt>FROM</tt>.
-	 */
-	public static interface JoinScope<X> extends Serializable {
-		public void addJoin(Join<X, ?> join);
-		public void addFetch(Fetch<X,?> fetch);
-	}
 
 	public FromImplementor<Z,X> correlateTo(CriteriaSubqueryImpl subquery);
-	public void prepareCorrelationDelegate(JoinScope<X> joinScope, FromImplementor<Z,X> parent);
+	public void prepareCorrelationDelegate(FromImplementor<Z,X> parent);
+	public FromImplementor<Z, X> getCorrelationParent();
 }

entitymanager/src/main/java/org/hibernate/ejb/criteria/QueryStructure.java

@@ -60,15 +60,18 @@ import org.hibernate.ejb.criteria.path.RootImpl;
 public class QueryStructure<T> implements Serializable {
 	private final AbstractQuery<T> owner;
 	private final CriteriaBuilderImpl criteriaBuilder;
+	private final boolean isSubQuery;
 
 	public QueryStructure(AbstractQuery<T> owner, CriteriaBuilderImpl criteriaBuilder) {
 		this.owner = owner;
 		this.criteriaBuilder = criteriaBuilder;
+		this.isSubQuery = Subquery.class.isInstance( owner );
 	}
 
 	private boolean distinct;
 	private Selection<? extends T> selection;
 	private Set<Root<?>> roots = new HashSet<Root<?>>();
+	private Set<FromImplementor> correlationRoots;
 	private Predicate restriction;
 	private List<Expression<?>> groupings = Collections.emptyList();
 	private Predicate having;
@@ -148,6 +151,36 @@ public class QueryStructure<T> implements Serializable {
 	}
 
 
+	// CORRELATION ROOTS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+	public void addCorrelationRoot(FromImplementor fromImplementor) {
+		if ( !isSubQuery ) {
+			throw new IllegalStateException( "Query is not identified as sub-query" );
+		}
+		if ( correlationRoots == null ) {
+			correlationRoots = new HashSet<FromImplementor>();
+		}
+		correlationRoots.add( fromImplementor );
+	}
+
+	public Set<Join<?, ?>> collectCorrelatedJoins() {
+		if ( !isSubQuery ) {
+			throw new IllegalStateException( "Query is not identified as sub-query" );
+		}
+		final Set<Join<?, ?>> correlatedJoins;
+		if ( correlationRoots != null ) {
+			correlatedJoins = new HashSet<Join<?,?>>();
+			for ( FromImplementor<?,?> correlationRoot : correlationRoots ) {
+				correlatedJoins.addAll( correlationRoot.getJoins() );
+			}
+		}
+		else {
+			correlatedJoins = Collections.emptySet();
+		}
+		return correlatedJoins;
+	}
+
+
 	// RESTRICTIONS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 	public Predicate getRestriction() {
@@ -213,14 +246,61 @@ public class QueryStructure<T> implements Serializable {
 			jpaqlQuery.append( "distinct " );
 		}
 		if ( getSelection() == null ) {
-			// we should have only a single root (query validation should have checked this...)
+			jpaqlQuery.append( locateImplicitSelection().renderProjection( renderingContext ) );
-			final Root root = getRoots().iterator().next();
-			jpaqlQuery.append( ( (Renderable) root ).renderProjection( renderingContext) );
 		}
 		else {
 			jpaqlQuery.append( ( (Renderable) getSelection() ).renderProjection( renderingContext ) );
 		}
 
+		renderFromClause( jpaqlQuery, renderingContext );
+
+		if ( getRestriction() != null) {
+			jpaqlQuery.append( " where " )
+					.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
+		}
+
+		if ( ! getGroupings().isEmpty() ) {
+			jpaqlQuery.append( " group by " );
+			String sep = "";
+			for ( Expression grouping : getGroupings() ) {
+				jpaqlQuery.append( sep )
+						.append( ( (Renderable) grouping ).render( renderingContext ) );
+				sep = ", ";
+			}
+
+			if ( getHaving() != null ) {
+				jpaqlQuery.append( " having " )
+						.append( ( (Renderable) getHaving() ).render( renderingContext ) );
+			}
+		}
+	}
+
+	private FromImplementor locateImplicitSelection() {
+		FromImplementor implicitSelection = null;
+
+		if ( ! isSubQuery ) {
+			// we should have only a single root (query validation should have checked this...)
+			implicitSelection = (FromImplementor) getRoots().iterator().next();
+		}
+		else {
+			// we should only have a single "root" which can act as the implicit selection
+			final Set<Join<?, ?>> correlatedJoins = collectCorrelatedJoins();
+			if ( correlatedJoins != null ) {
+				if ( correlatedJoins.size() == 1 ) {
+					implicitSelection = (FromImplementor) correlatedJoins.iterator().next();
+				}
+			}
+		}
+
+		if ( implicitSelection == null ) {
+			throw new IllegalStateException( "No explicit selection and an implicit one cold not be determined" );
+		}
+
+		return implicitSelection;
+	}
+
+	@SuppressWarnings({ "unchecked" })
+	private void renderFromClause(StringBuilder jpaqlQuery, CriteriaQueryCompiler.RenderingContext renderingContext) {
 		jpaqlQuery.append( " from " );
 		String sep = "";
 		for ( Root root : getRoots() ) {
@@ -235,23 +315,26 @@ public class QueryStructure<T> implements Serializable {
 			renderFetches( jpaqlQuery, renderingContext, root.getFetches() );
 		}
 
-		if ( getRestriction() != null) {
+		if ( isSubQuery ) {
-			jpaqlQuery.append( " where " )
+			if ( correlationRoots != null ) {
-					.append( ( (Renderable) getRestriction() ).render( renderingContext ) );
+				for ( FromImplementor<?,?> correlationRoot : correlationRoots ) {
-		}
+					final FromImplementor correlationParent = correlationRoot.getCorrelationParent();
-
+					correlationParent.prepareAlias( renderingContext );
-		if ( ! getGroupings().isEmpty() ) {
+					final String correlationRootAlias = correlationParent.getAlias();
-			jpaqlQuery.append( " group by " );
+					for ( Join<?,?> correlationJoin : correlationRoot.getJoins() ) {
-			sep = "";
+						final JoinImplementor correlationJoinImpl = (JoinImplementor) correlationJoin;
-			for ( Expression grouping : getGroupings() ) {
+						// IMPL NOTE: reuse the sep from above!
-				jpaqlQuery.append( sep )
+						jpaqlQuery.append( sep );
-						.append( ( (Renderable) grouping ).render( renderingContext ) );
+						correlationJoinImpl.prepareAlias( renderingContext );
-				sep = ", ";
+						jpaqlQuery.append( correlationRootAlias )
-			}
+								.append( '.' )
-
+								.append( correlationJoinImpl.getAttribute().getName() )
-			if ( getHaving() != null ) {
+								.append( " as " )
-				jpaqlQuery.append( " having " )
+								.append( correlationJoinImpl.getAlias() );
-						.append( ( (Renderable) getHaving() ).render( renderingContext ) );
+						sep = ", ";
+						renderJoins( jpaqlQuery, renderingContext, correlationJoinImpl.getJoins() );
+					}
+				}
 			}
 		}
 	}

entitymanager/src/main/java/org/hibernate/ejb/criteria/path/AbstractFromImpl.java

@@ -102,10 +102,21 @@ public abstract class AbstractFromImpl<Z,X>
 	 */
 	public void prepareAlias(CriteriaQueryCompiler.RenderingContext renderingContext) {
 		if ( getAlias() == null ) {
-			setAlias( renderingContext.generateAlias() );
+			if ( isCorrelated() ) {
+				setAlias( getCorrelationParent().getAlias() );
+			}
+			else {
+				setAlias( renderingContext.generateAlias() );
+			}
 		}
 	}
 
+	@Override
+	public String renderProjection(CriteriaQueryCompiler.RenderingContext renderingContext) {
+		prepareAlias( renderingContext );
+		return getAlias();
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
@@ -137,8 +148,24 @@ public abstract class AbstractFromImpl<Z,X>
 
 	// CORRELATION ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-	private From<Z,X> correlationParent;
+	// IMPL NOTE : another means from handling correlations is to create a series of
-	private JoinScope<X> joinScope = new JoinScope<X>() {
+	//		specialized From implementations that represent the correlation roots.  While
+	//		that may be cleaner code-wise, it is certainly means creating a lot of "extra"
+	//		classes since we'd need one for each Subquery#correlate method
+
+	private FromImplementor<Z,X> correlationParent;
+
+	private JoinScope<X> joinScope = new BasicJoinScope();
+
+	/**
+	 * Helper contract used to define who/what keeps track of joins and fetches made from this <tt>FROM</tt>.
+	 */
+	public static interface JoinScope<X> extends Serializable {
+		public void addJoin(Join<X, ?> join);
+		public void addFetch(Fetch<X,?> fetch);
+	}
+
+	protected class BasicJoinScope implements JoinScope<X> {
 		public void addJoin(Join<X, ?> join) {
 			if ( joins == null ) {
 				joins = new LinkedHashSet<Join<X,?>>();
@@ -152,7 +179,20 @@ public abstract class AbstractFromImpl<Z,X>
 			}
 			fetches.add( fetch );
 		}
-	};
+	}
+
+	protected class CorrelationJoinScope implements JoinScope<X> {
+		public void addJoin(Join<X, ?> join) {
+			if ( joins == null ) {
+				joins = new LinkedHashSet<Join<X,?>>();
+			}
+			joins.add( join );
+		}
+
+		public void addFetch(Fetch<X, ?> fetch) {
+			throw new UnsupportedOperationException( "Cannot define fetch from a subquery correlation" );
+		}
+	}
 
 	/**
 	 * {@inheritDoc}
@@ -164,7 +204,7 @@ public abstract class AbstractFromImpl<Z,X>
 	/**
 	 * {@inheritDoc}
 	 */
-	public From<Z,X> getCorrelationParent() {
+	public FromImplementor<Z,X> getCorrelationParent() {
 		return correlationParent;
 	}
 
@@ -174,17 +214,21 @@ public abstract class AbstractFromImpl<Z,X>
 	@SuppressWarnings({ "unchecked" })
 	public FromImplementor<Z, X> correlateTo(CriteriaSubqueryImpl subquery) {
 		final FromImplementor<Z, X> correlationDelegate = createCorrelationDelegate();
-		correlationDelegate.prepareCorrelationDelegate( subquery.getJoinScope(), this );
+		correlationDelegate.prepareCorrelationDelegate( this );
 		return correlationDelegate;
 	}
 
 	protected abstract FromImplementor<Z, X> createCorrelationDelegate();
 
-	public void prepareCorrelationDelegate(JoinScope<X> joinScope, FromImplementor<Z, X> parent) {
+	public void prepareCorrelationDelegate(FromImplementor<Z, X> parent) {
-		this.joinScope = joinScope;
+		this.joinScope = new CorrelationJoinScope();
 		this.correlationParent = parent;
 	}
 
+	@Override
+	public String getAlias() {
+		return isCorrelated() ? getCorrelationParent().getAlias() : super.getAlias();
+	}
 
 	// JOINS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

entitymanager/src/test/java/org/hibernate/ejb/criteria/subquery/CorrelatedSubqueryTest.java

@@ -0,0 +1,130 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Inc. or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Inc.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ */
+package org.hibernate.ejb.criteria.subquery;
+
+import java.util.List;
+import javax.persistence.EntityManager;
+import javax.persistence.TypedQuery;
+import javax.persistence.criteria.CriteriaBuilder;
+import javax.persistence.criteria.CriteriaQuery;
+import javax.persistence.criteria.Join;
+import javax.persistence.criteria.Root;
+import javax.persistence.criteria.Subquery;
+
+import org.hibernate.ejb.metamodel.AbstractMetamodelSpecificTest;
+import org.hibernate.ejb.metamodel.Customer;
+import org.hibernate.ejb.metamodel.Customer_;
+import org.hibernate.ejb.metamodel.LineItem;
+import org.hibernate.ejb.metamodel.LineItem_;
+import org.hibernate.ejb.metamodel.Order;
+import org.hibernate.ejb.metamodel.Order_;
+
+/**
+ * TODO : javadoc
+ *
+ * @author Steve Ebersole
+ */
+public class CorrelatedSubqueryTest extends AbstractMetamodelSpecificTest {
+	public void testBasicCorrelation() {
+		CriteriaBuilder builder = factory.getCriteriaBuilder();
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+
+		CriteriaQuery<Customer> criteria = builder.createQuery( Customer.class );
+		Root<Customer> customer = criteria.from( Customer.class );
+		criteria.select( customer );
+		Subquery<Order> orderSubquery = criteria.subquery( Order.class );
+		Root<Customer> customerCorrelationRoot = orderSubquery.correlate( customer );
+		Join<Customer, Order> customerOrderCorrelationJoin = customerCorrelationRoot.join( Customer_.orders );
+		orderSubquery.select( customerOrderCorrelationJoin );
+		criteria.where( builder.not( builder.exists( orderSubquery ) ) );
+		em.createQuery( criteria ).getResultList();
+
+		em.getTransaction().commit();
+		em.close();
+	}
+
+	public void testRestrictedCorrelation() {
+		CriteriaBuilder builder = factory.getCriteriaBuilder();
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+
+		CriteriaQuery<Order> criteria = builder.createQuery( Order.class );
+		Root<Order> orderRoot = criteria.from( Order.class );
+		criteria.select( orderRoot );
+		// create correlated subquery
+		Subquery<Customer> customerSubquery = criteria.subquery( Customer.class );
+		Root<Order> orderRootCorrelation = customerSubquery.correlate( orderRoot );
+		Join<Order, Customer> orderCustomerJoin = orderRootCorrelation.join( Order_.customer );
+		customerSubquery.where( builder.like( orderCustomerJoin.get( Customer_.name ), "%Caruso" ) )
+				.select( orderCustomerJoin );
+		criteria.where( builder.exists( customerSubquery ) );
+		em.createQuery( criteria ).getResultList();
+
+		em.getTransaction().commit();
+		em.close();
+	}
+
+	public void testCorrelationExplicitSelectionCorrelation() {
+		CriteriaBuilder builder = factory.getCriteriaBuilder();
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+
+		CriteriaQuery<Customer> customerCriteria = builder.createQuery( Customer.class );
+		Root<Customer> customer = customerCriteria.from( Customer.class );
+		Join<Customer, Order> o = customer.join( Customer_.orders );
+		Subquery<Order> sq = customerCriteria.subquery(Order.class);
+		Join<Customer, Order> sqo = sq.correlate(o);
+		Join<Order, LineItem> sql = sqo.join(Order_.lineItems);
+		sq.where( builder.gt(sql.get( LineItem_.quantity), 3) );
+		// use the correlation itself as the subquery selection (initially caused problems wrt aliases)
+		sq.select(sqo);
+		customerCriteria.select(customer).distinct(true);
+		customerCriteria.where(builder.exists(sq));
+		em.createQuery( customerCriteria ).getResultList();
+
+		em.getTransaction().commit();
+		em.close();
+	}
+
+	public void testRestrictedCorrelationNoExplicitSelection() {
+		CriteriaBuilder builder = factory.getCriteriaBuilder();
+		EntityManager em = getOrCreateEntityManager();
+		em.getTransaction().begin();
+
+		CriteriaQuery<Order> criteria = builder.createQuery( Order.class );
+		Root<Order> orderRoot = criteria.from( Order.class );
+		criteria.select( orderRoot );
+		// create correlated subquery
+		Subquery<Customer> customerSubquery = criteria.subquery( Customer.class );
+		Root<Order> orderRootCorrelation = customerSubquery.correlate( orderRoot );
+		Join<Order, Customer> orderCustomerJoin = orderRootCorrelation.join( "customer" );
+		customerSubquery.where( builder.like( orderCustomerJoin.<String>get( "name" ), "%Caruso" ) );
+		criteria.where( builder.exists( customerSubquery ) );
+		em.createQuery( criteria ).getResultList();
+
+		em.getTransaction().commit();
+		em.close();
+	}
+}