Merge pull request #825 from jamesrdf/issues/#380-bind-join

Fix #380: Always evaluate Extension/BIND before join

Author: James Leigh

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

@@ -16,6 +16,7 @@
 
 import org.eclipse.rdf4j.query.BindingSet;
 import org.eclipse.rdf4j.query.Dataset;
+import org.eclipse.rdf4j.query.algebra.Extension;
 import org.eclipse.rdf4j.query.algebra.Join;
 import org.eclipse.rdf4j.query.algebra.LeftJoin;
 import org.eclipse.rdf4j.query.algebra.StatementPattern;
@@ -88,9 +89,18 @@ public void meet(Join node) {
 				// Reorder the (recursive) join arguments to a more optimal sequence
 				List<TupleExpr> orderedJoinArgs = new ArrayList<TupleExpr>(joinArgs.size());
 
+				// Reorder the subselects and extensions to a more optimal sequence
+				List<TupleExpr> priorityArgs = new ArrayList<TupleExpr>(joinArgs.size());
+
 				// first get all subselects and order them
 				List<TupleExpr> orderedSubselects = reorderSubselects(getSubSelects(joinArgs));
 				joinArgs.removeAll(orderedSubselects);
+				priorityArgs.addAll(orderedSubselects);
+
+				// second get all extensions (BIND clause)
+				List<Extension> orderedExtensions = getExtensions(joinArgs);
+				joinArgs.removeAll(orderedExtensions);
+				priorityArgs.addAll(orderedExtensions);
 
 				// We order all remaining join arguments based on cardinality and
 				// variable frequency statistics
@@ -131,11 +141,11 @@ public void meet(Join node) {
 				}
 
 				// Build new join hierarchy
-				TupleExpr subselectJoins = null;
-				if (orderedSubselects.size() > 0) {
-					subselectJoins = orderedSubselects.get(0);
-					for (int i = 1; i < orderedSubselects.size(); i++) {
-						subselectJoins = new Join(subselectJoins, orderedSubselects.get(i));
+				TupleExpr priorityJoins = null;
+				if (priorityArgs.size() > 0) {
+					priorityJoins = priorityArgs.get(0);
+					for (int i = 1; i < priorityArgs.size(); i++) {
+						priorityJoins = new Join(priorityJoins, priorityArgs.get(i));
 					}
 				}
 
@@ -149,17 +159,17 @@ public void meet(Join node) {
 						replacement = new Join(orderedJoinArgs.get(i), replacement);
 					}
 
-					if (subselectJoins != null) {
-						replacement = new Join(subselectJoins, replacement);
+					if (priorityJoins != null) {
+						replacement = new Join(priorityJoins, replacement);
 					}
 
 					// Replace old join hierarchy
 					node.replaceWith(replacement);
 
 				}
 				else {
-					// only subselect joins involved in this query.
-					node.replaceWith(subselectJoins);
+					// only subselect/priority joins involved in this query.
+					node.replaceWith(priorityJoins);
 				}
 			}
 			finally {
@@ -198,6 +208,16 @@ protected <M extends Map<Var, Integer>> M getVarFreqMap(List<Var> varList, M var
 			return varFreqMap;
 		}
 
+		protected List<Extension> getExtensions(List<TupleExpr> expressions) {
+			List<Extension> extensions = new ArrayList<Extension>();
+			for (TupleExpr expr : expressions) {
+				if (expr instanceof Extension) {
+					extensions.add((Extension)expr);
+				}
+			}
+			return extensions;
+		}
+
 		protected List<TupleExpr> getSubSelects(List<TupleExpr> expressions) {
 			List<TupleExpr> subselects = new ArrayList<TupleExpr>();
 

core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java

@@ -13,10 +13,16 @@
 import org.eclipse.rdf4j.query.MalformedQueryException;
 import org.eclipse.rdf4j.query.QueryLanguage;
 import org.eclipse.rdf4j.query.UnsupportedQueryLanguageException;
+import org.eclipse.rdf4j.query.algebra.BinaryTupleOperator;
+import org.eclipse.rdf4j.query.algebra.Extension;
 import org.eclipse.rdf4j.query.algebra.QueryModelNode;
 import org.eclipse.rdf4j.query.algebra.QueryRoot;
+import org.eclipse.rdf4j.query.algebra.TupleExpr;
+import org.eclipse.rdf4j.query.algebra.UnaryTupleOperator;
 import org.eclipse.rdf4j.query.parser.ParsedQuery;
 import org.eclipse.rdf4j.query.parser.QueryParserUtil;
+import org.eclipse.rdf4j.query.parser.sparql.SPARQLParser;
+import org.junit.Assert;
 import org.junit.Test;
 
 /**
@@ -59,6 +65,36 @@ public void testContextOptimization()
 		testOptimizer(expectedQuery, query);
 	}
 
+	@Test
+	public void testSES2116JoinBind()
+		throws Exception
+	{
+
+		StringBuilder qb = new StringBuilder();
+		qb.append("SELECT ?subject ?name ?row {\n"
+				+ "  ?subject <http://localhost/table_1> ?uri .\n" + "  BIND(STR(?uri) AS ?name)\n"
+				+ "  ?table <http://linked.opendata.cz/ontology/odcs/tabular/hasRow> ?row .\n"
+				+ "  ?table <http://linked.opendata.cz/ontology/odcs/tabular/symbolicName> ?name .\n" + "}");
+
+		SPARQLParser parser = new SPARQLParser();
+		ParsedQuery q = parser.parseQuery(qb.toString(), null);
+		QueryJoinOptimizer opt = new QueryJoinOptimizer();
+		QueryRoot optRoot = new QueryRoot(q.getTupleExpr());
+		opt.optimize(optRoot, null, null);
+		TupleExpr leaf = findLeaf(optRoot);
+		Assert.assertTrue("Extension must be evaluated before StatementPattern", leaf.getParentNode() instanceof Extension);
+	}
+
+	private TupleExpr findLeaf(TupleExpr expr) {
+		if (expr instanceof UnaryTupleOperator) {
+			return findLeaf(((UnaryTupleOperator)expr).getArg());
+		} else if (expr instanceof BinaryTupleOperator) {
+			return findLeaf(((BinaryTupleOperator)expr).getLeftArg());
+		} else {
+			return expr;
+		}
+	}
+
 	private void testOptimizer(String expectedQuery, String actualQuery)
 		throws MalformedQueryException, UnsupportedQueryLanguageException
 	{