GH-4646 remove intermediate joins

Author: JervenBolleman

core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/PathIteration.java

@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.rdf4j.query.algebra.evaluation.iterator;
 
+import java.util.HashSet;
 import java.util.Queue;
 import java.util.Set;
 
@@ -66,6 +67,8 @@ public class PathIteration extends LookAheadIteration<BindingSet, QueryEvaluatio
 
 	private static final String JOINVAR_PREFIX = "intermediate_join_";
 
+	private final Set<String> namedIntermediateJoins = new HashSet<>();
+
 	public PathIteration(EvaluationStrategy strategy, Scope scope, Var startVar,
 			TupleExpr pathExpression, Var endVar, Var contextVar, long minLength, BindingSet bindings)
 			throws QueryEvaluationException {
@@ -104,10 +107,10 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
 
 			while (currentIter != null && currentIter.hasNext()) {
 				BindingSet potentialNextElement = currentIter.next();
-				MutableBindingSet nextElement;
+				QueryBindingSet nextElement;
 				// if it is not a compatible type of BindingSet
 				if (potentialNextElement instanceof QueryBindingSet) {
-					nextElement = (MutableBindingSet) potentialNextElement;
+					nextElement = (QueryBindingSet) potentialNextElement;
 				} else {
 					nextElement = new QueryBindingSet(potentialNextElement);
 				}
@@ -121,22 +124,10 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
 					}
 				}
 
-				Value v1, v2;
-
-				if (startVarFixed && endVarFixed && currentLength > 2) {
-					v1 = getVarValue(startVar, startVarFixed, nextElement);
-					v2 = nextElement.getValue("END_" + JOINVAR_PREFIX + this.hashCode());
-				} else if (startVarFixed && endVarFixed && currentLength == 2) {
-					v1 = getVarValue(startVar, startVarFixed, nextElement);
-					v2 = nextElement.getValue(JOINVAR_PREFIX + (currentLength - 1) + "_" + this.hashCode());
-				} else {
-					v1 = getVarValue(startVar, startVarFixed, nextElement);
-					v2 = getVarValue(endVar, endVarFixed, nextElement);
-				}
+				ValuePair vp = valuePairFromStartAndEnd(nextElement);
 
-				if (!isCyclicPath(v1, v2)) {
+				if (!isCyclicPath(vp)) {
 
-					ValuePair vp = new ValuePair(v1, v2);
 					if (reportedValues.contains(vp)) {
 						// new arbitrary-length path semantics: filter out
 						// duplicates
@@ -151,38 +142,38 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
 
 					if (startVarFixed && endVarFixed) {
 						Value endValue = getVarValue(endVar, endVarFixed, nextElement);
-						if (endValue.equals(v2)) {
+						if (endValue.equals(vp.endValue)) {
 							add(reportedValues, vp);
-							if (!v1.equals(v2)) {
+							if (!vp.startValue.equals(vp.endValue)) {
 								addToQueue(valueQueue, vp);
 							}
 							if (!nextElement.hasBinding(startVar.getName())) {
-								addBinding(nextElement, startVar.getName(), v1);
+								addBinding(nextElement, startVar.getName(), vp.startValue);
 							}
 							if (!nextElement.hasBinding(endVar.getName())) {
-								addBinding(nextElement, endVar.getName(), v2);
+								addBinding(nextElement, endVar.getName(), vp.endValue);
 							}
-							return nextElement;
+							return removeIntermediateJoinVars(nextElement);
 						} else {
 							if (add(unreportedValues, vp)) {
-								if (!v1.equals(v2)) {
+								if (!vp.startValue.equals(vp.endValue)) {
 									addToQueue(valueQueue, vp);
 								}
 							}
 							continue again;
 						}
 					} else {
 						add(reportedValues, vp);
-						if (!v1.equals(v2)) {
+						if (!vp.startValue.equals(vp.endValue)) {
 							addToQueue(valueQueue, vp);
 						}
 						if (!nextElement.hasBinding(startVar.getName())) {
-							addBinding(nextElement, startVar.getName(), v1);
+							addBinding(nextElement, startVar.getName(), vp.startValue);
 						}
 						if (!nextElement.hasBinding(endVar.getName())) {
-							addBinding(nextElement, endVar.getName(), v2);
+							addBinding(nextElement, endVar.getName(), vp.endValue);
 						}
-						return nextElement;
+						return removeIntermediateJoinVars(nextElement);
 					}
 				} else {
 					continue again;
@@ -198,6 +189,27 @@ protected BindingSet getNextElement() throws QueryEvaluationException {
 		}
 	}
 
+	private BindingSet removeIntermediateJoinVars(QueryBindingSet nextElement) {
+		nextElement.removeAll(namedIntermediateJoins);
+		return nextElement;
+	}
+
+	private ValuePair valuePairFromStartAndEnd(MutableBindingSet nextElement) {
+		Value v1, v2;
+
+		if (startVarFixed && endVarFixed && currentLength > 2) {
+			v1 = getVarValue(startVar, startVarFixed, nextElement);
+			v2 = nextElement.getValue("END_" + JOINVAR_PREFIX + this.hashCode());
+		} else if (startVarFixed && endVarFixed && currentLength == 2) {
+			v1 = getVarValue(startVar, startVarFixed, nextElement);
+			v2 = nextElement.getValue(JOINVAR_PREFIX + (currentLength - 1) + "_" + this.hashCode());
+		} else {
+			v1 = getVarValue(startVar, startVarFixed, nextElement);
+			v2 = getVarValue(endVar, endVarFixed, nextElement);
+		}
+		return new ValuePair(v1, v2);
+	}
+
 	private void addBinding(MutableBindingSet bs, String name, Value value) {
 		bs.addBinding(name, value);
 	}
@@ -242,12 +254,12 @@ private Value getVarValue(Var var, boolean fixedValue, BindingSet bindingSet) {
 		return v;
 	}
 
-	private boolean isCyclicPath(Value v1, Value v2) {
+	private boolean isCyclicPath(ValuePair vp) {
 		if (currentLength <= 2) {
 			return false;
 		}
 
-		return reportedValues.contains(new ValuePair(v1, v2));
+		return reportedValues.contains(vp);
 
 	}
 
@@ -426,9 +438,12 @@ public void meet(Var var) {
 
 	}
 
+	private Var createAnonVar(String varName, Value v, boolean anonymous) {
+		namedIntermediateJoins.add(varName);
+		return new Var(varName, null, anonymous, false);
+	}
+
 	public Var createAnonVar(String varName) {
-		Var var = new Var(varName);
-		var.setAnonymous(true);
-		return var;
+		return createAnonVar(varName, null, true);
 	}
 }

core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/iterator/PathIterationTest.java

@@ -110,6 +110,7 @@ void assertExpected(BindingSet result, Value subClass, Value superClass) {
 		assertEquals(subClass, result.getBinding("subClass").getValue());
 		assertTrue(result.hasBinding("superClass"), "path zlp evaluation should binding for superClass var");
 		assertEquals(superClass, result.getBinding("superClass").getValue());
+		assertEquals(2, result.size());
 	}
 
 	@Test