Merge pull request #797 from jamesrdf/issues/#695-join-projection

Fix #695: Don't treat path modifiers as subqueries

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java

@@ -202,7 +202,7 @@ protected List<TupleExpr> getSubSelects(List<TupleExpr> expressions) {
 			List<TupleExpr> subselects = new ArrayList<TupleExpr>();
 
 			for (TupleExpr expr : expressions) {
-				if (TupleExprs.containsProjection(expr)) {
+				if (TupleExprs.containsSubquery(expr)) {
 					subselects.add(expr);
 				}
 			}

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/StrictEvaluationStrategy.java

@@ -911,7 +911,7 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(Join jo
 			return new ServiceJoinIterator(leftIter, (Service)join.getRightArg(), bindings, this);
 		}
 
-		if (TupleExprs.containsProjection(join.getRightArg())) {
+		if (TupleExprs.containsSubquery(join.getRightArg())) {
 			return new HashJoinIteration(this, join, bindings);
 		}
 		else {
@@ -923,7 +923,7 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(LeftJoi
 			final BindingSet bindings)
 		throws QueryEvaluationException
 	{
-		if (TupleExprs.containsProjection(leftJoin.getRightArg())) {
+		if (TupleExprs.containsSubquery(leftJoin.getRightArg())) {
 			return new HashJoinIteration(this, leftJoin, bindings);
 		}
 

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/limited/LimitedSizeEvaluationStrategy.java

@@ -152,7 +152,7 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(Join jo
 			return new ServiceJoinIterator(leftIter, (Service)join.getRightArg(), bindings, this);
 		}
 
-		if (TupleExprs.containsProjection(join.getRightArg())) {
+		if (TupleExprs.containsSubquery(join.getRightArg())) {
 			return new LimitedSizeHashJoinIteration(this, join, bindings, used, maxSize);
 		}
 		else {

core/queryalgebra/model/src/main/java/org/eclipse/rdf4j/query/algebra/Join.java

@@ -42,7 +42,7 @@ public Join(TupleExpr leftArg, TupleExpr rightArg) {
 	 */
 	@Deprecated
 	public boolean hasSubSelectInRightArg() {
-		return TupleExprs.containsProjection(rightArg);
+		return TupleExprs.containsSubquery(rightArg);
 	}
 
 	public Set<String> getBindingNames() {

core/queryalgebra/model/src/main/java/org/eclipse/rdf4j/query/algebra/Projection.java

@@ -22,6 +22,8 @@ public class Projection extends UnaryTupleOperator {
 
 	private Var projectionContext = null;
 
+	private boolean subquery = true;
+
 	/*--------------*
 	 * Constructors *
 	 *--------------*/
@@ -38,6 +40,11 @@ public Projection(TupleExpr arg, ProjectionElemList elements) {
 		setProjectionElemList(elements);
 	}
 
+	public Projection(TupleExpr arg, ProjectionElemList elements, boolean subquery) {
+		this(arg, elements);
+		this.subquery = subquery;
+	}
+
 	/*---------*
 	 * Methods *
 	 *---------*/
@@ -123,4 +130,12 @@ public void setProjectionContext(Var projectionContext) {
 		this.projectionContext = projectionContext;
 	}
 
+	public boolean isSubquery() {
+		return subquery;
+	}
+
+	public void setSubquery(boolean subquery) {
+		this.subquery = subquery;
+	}
+
 }

core/queryalgebra/model/src/main/java/org/eclipse/rdf4j/query/algebra/helpers/TupleExprs.java

@@ -28,6 +28,36 @@
  */
 public class TupleExprs {
 
+	/**
+	 * Verifies if the supplied {@link TupleExpr} contains a {@link Projection} with the subquery flag set to
+	 * true (default). If the supplied TupleExpr is a {@link Join} or contains a {@link Join}, projections
+	 * inside that Join's arguments will not be taken into account.
+	 *
+	 * @param t
+	 *        a tuple expression.
+	 * @return <code>true</code> if the TupleExpr contains a subquery projection (outside of a Join),
+	 *         <code>false</code> otherwise.
+	 */
+	public static boolean containsSubquery(TupleExpr t) {
+		Deque<TupleExpr> queue = new ArrayDeque<>();
+		queue.add(t);
+		while (!queue.isEmpty()) {
+			TupleExpr n = queue.removeFirst();
+			if (n instanceof Projection && ((Projection)n).isSubquery()) {
+				return true;
+			}
+			else if (n instanceof Join) {
+				// projections already inside a Join need not be
+				// taken into account
+				return false;
+			}
+			else {
+				queue.addAll(getChildren(n));
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Verifies if the supplied {@link TupleExpr} contains a {@link Projection}. If the supplied TupleExpr is
 	 * a {@link Join} or contains a {@link Join}, projections inside that Join's arguments will not be taken
@@ -37,7 +67,9 @@ public class TupleExprs {
 	 *        a tuple expression.
 	 * @return <code>true</code> if the TupleExpr contains a projection (outside of a Join),
 	 *         <code>false</code> otherwise.
+	 * @deprecated Since 2.3. Use {@link TupleExprs#containsSubQuery(TupleExpr)} instead.
 	 */
+	@Deprecated
 	public static boolean containsProjection(TupleExpr t) {
 		Deque<TupleExpr> queue = new ArrayDeque<>();
 		queue.add(t);

core/queryparser/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/TupleExprBuilder.java

@@ -1700,7 +1700,7 @@ private TupleExpr handlePathModifiers(Scope scope, Var subjVar, TupleExpr te, Va
 						pelist.addElement(pe);
 					}
 
-					result = new Distinct(new Projection(union, pelist));
+					result = new Distinct(new Projection(union, pelist, false));
 				}
 				else {
 					// upperbound is abitrary-length

core/sail/federation/src/main/java/org/eclipse/rdf4j/sail/federation/evaluation/FederationStrategy.java

@@ -91,7 +91,7 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(NaryJoi
 		for (int i = 1, n = join.getNumberOfArguments(); i < n; i++) {
 
 			TupleExpr rightArg = join.getArg(i);
-			if (TupleExprs.containsProjection(rightArg)
+			if (TupleExprs.containsSubquery(rightArg)
 					|| (rightArg instanceof OwnedTupleExpr && ((OwnedTupleExpr)rightArg).hasQuery()))
 			{
 				TupleExpr leftArg = join.getArg(i - 1);