Merge pull request #855 from jamesrdf/issues/#79-bind-error

Fix #79: Use null binding for unbound variable solutions

Author: James Leigh

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

@@ -86,7 +86,6 @@ public void setBinding(Binding binding) {
 	}
 
 	public void setBinding(String name, Value value) {
-		assert value != null : "null value for variable " + name;
 		bindings.put(name, value);
 	}
 

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

@@ -348,7 +348,7 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(Service
 		if (serviceRef.hasValue())
 			serviceUri = serviceRef.getValue().stringValue();
 		else {
-			if (bindings != null && bindings.hasBinding(serviceRef.getName())) {
+			if (bindings != null && bindings.getValue(serviceRef.getName()) != null) {
 				serviceUri = bindings.getBinding(serviceRef.getName()).getValue().stringValue();
 			}
 			else {
@@ -469,6 +469,13 @@ public CloseableIteration<BindingSet, QueryEvaluationException> evaluate(Stateme
 		CloseableIteration<? extends Statement, QueryEvaluationException> stIter3 = null;
 		ConvertingIteration<Statement, BindingSet, QueryEvaluationException> result = null;
 
+		if (isUnbound(subjVar, bindings) || isUnbound(predVar, bindings) || isUnbound(objVar, bindings)
+				|| isUnbound(conVar, bindings))
+		{
+			// the variable must remain unbound for this solution see https://www.w3.org/TR/sparql11-query/#assignment
+			return new EmptyIteration<BindingSet, QueryEvaluationException>();
+		}
+
 		boolean allGood = false;
 		try {
 			try {
@@ -653,6 +660,15 @@ protected BindingSet convert(Statement st) {
 		}
 	}
 
+	protected boolean isUnbound(Var var, BindingSet bindings) {
+		if (var == null) {
+			return false;
+		}
+		else {
+			return bindings.hasBinding(var.getName()) && bindings.getValue(var.getName()) == null;
+		}
+	}
+
 	protected Value getVarValue(Var var, BindingSet bindings) {
 		if (var == null) {
 			return null;

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

@@ -57,7 +57,10 @@ public BindingSet convert(BindingSet sourceBindings)
 				}
 				catch (ValueExprEvaluationException e) {
 					// silently ignore type errors in extension arguments. They should not cause the 
-					// query to fail but just result in no additional binding.
+					// query to fail but result in no bindings for this solution
+					// see https://www.w3.org/TR/sparql11-query/#assignment
+					// use null as place holder for unbound variables that must remain so
+					targetBindings.setBinding(extElem.getName(), null);
 				}
 			}
 		}

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

@@ -261,7 +261,11 @@ private void createIteration()
 		throws QueryEvaluationException
 	{
 
-		if (currentLength == 0L) {
+		if (isUnbound(startVar, bindings) || isUnbound(endVar, bindings)) {
+			// the variable must remain unbound for this solution see https://www.w3.org/TR/sparql11-query/#assignment
+			currentIter = new EmptyIteration<BindingSet, QueryEvaluationException>();
+		}
+		else if (currentLength == 0L) {
 			ZeroLengthPath zlp = new ZeroLengthPath(scope, startVar, endVar, contextVar);
 			currentIter = this.evaluationStrategyImpl.evaluate(zlp, bindings);
 			currentLength++;
@@ -334,6 +338,15 @@ else if (currentLength == 1) {
 		}
 	}
 
+	protected boolean isUnbound(Var var, BindingSet bindings) {
+		if (var == null) {
+			return false;
+		}
+		else {
+			return bindings.hasBinding(var.getName()) && bindings.getValue(var.getName()) == null;
+		}
+	}
+
 	protected Set<ValuePair> makeSet() {
 		return new HashSet<ValuePair>(64, 0.9f);
 	}

core/repository/sparql/src/main/java/org/eclipse/rdf4j/repository/sparql/query/SPARQLQueryBindingSet.java

@@ -88,7 +88,6 @@ public void setBinding(Binding binding) {
 	}
 
 	public void setBinding(String name, Value value) {
-		assert value != null : "null value for variable " + name;
 		bindings.put(name, value);
 	}
 

testsuites/sparql/src/main/java/org/eclipse/rdf4j/query/parser/sparql/ComplexSPARQLQueryTest.java

@@ -2333,6 +2333,48 @@ public void testSES1979MinMaxInf()
 
 	}
 
+	@Test
+	public void testSES2250BindErrors() throws Exception {
+		StringBuilder ub = new StringBuilder();
+		ub.append("insert data { <urn:test:subj> <urn:test:pred> _:blank }");
+
+		conn.prepareUpdate(QueryLanguage.SPARQL, ub.toString()).execute();
+
+		StringBuilder qb = new StringBuilder();
+		qb.append("SELECT * {\n"
+				+ "    ?s1 ?p1 ?blank . "
+				+ "    FILTER(isBlank(?blank))"
+				+ "    BIND (iri(?blank) as ?biri)"
+				+ "    ?biri ?p2 ?o2 ."
+				+ "}");
+
+		TupleQuery tq = conn.prepareTupleQuery(QueryLanguage.SPARQL, qb.toString());
+		try (TupleQueryResult evaluate = tq.evaluate();) {
+			assertFalse("The query should not return a result", evaluate.hasNext());
+		}
+	}
+
+	@Test
+	public void testSES2250BindErrorsInPath() throws Exception {
+		StringBuilder ub = new StringBuilder();
+		ub.append("insert data { <urn:test:subj> <urn:test:pred> _:blank }");
+
+		conn.prepareUpdate(QueryLanguage.SPARQL, ub.toString()).execute();
+
+		StringBuilder qb = new StringBuilder();
+		qb.append("SELECT * {\n"
+				+ "    ?s1 ?p1 ?blank . "
+				+ "    FILTER(isBlank(?blank))"
+				+ "    BIND (iri(?blank) as ?biri)"
+				+ "    ?biri <urn:test:pred>* ?o2 ."
+				+ "}");
+
+		TupleQuery tq = conn.prepareTupleQuery(QueryLanguage.SPARQL, qb.toString());
+		try (TupleQueryResult evaluate = tq.evaluate();) {
+			assertFalse("The query should not return a result", evaluate.hasNext());
+		}
+	}
+
 	private boolean containsSolution(List<BindingSet> result, Binding... solution) {
 		final MapBindingSet bs = new MapBindingSet();
 		for (Binding b : solution) {