Merge pull request #3707 from nguyenm100/GH-3667-Subqueries-Bind-Opti…

…mizer-Fix GH-3667: Optimize subqueries and bind statements
eclipse-rdf4j · Mar 5, 2022 · 4197391 · 4197391
2 parents 8c8552c + 66f346f
commit 4197391
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 7 deletions.
diff --git a/...ion/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java b/...ion/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java
@@ -88,6 +88,17 @@ public void meet(StatementPattern node) throws RuntimeException {
 			node.setResultSizeEstimate(Math.max(statistics.getCardinality(node), node.getResultSizeEstimate()));
 		}
 
+		private void optimizePriorityJoin(Set<String> origBoundVars, TupleExpr join) {
+
+			Set<String> saveBoundVars = boundVars;
+			try {
+				boundVars = new HashSet<>(origBoundVars);
+				join.visit(this);
+			} finally {
+				boundVars = saveBoundVars;
+			}
+		}
+
 		@Override
 		public void meet(Join node) {
 
@@ -179,6 +190,13 @@ public void meet(Join node) {
 					// Replace old join hierarchy
 					node.replaceWith(replacement);
 
+					// we optimize after the replacement call above in case the optimize call below
+					// recurses back into this function and we need all the node's parent/child pointers
+					// set up correctly for replacement to work on subsequent calls
+					if (priorityJoins != null) {
+						optimizePriorityJoin(origBoundVars, priorityJoins);
+					}
+
 				} else {
 					// only subselect/priority joins involved in this query.
 					node.replaceWith(priorityJoins);

diff --git a/...src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java b/...src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java
@@ -11,17 +11,15 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.eclipse.rdf4j.RDF4JException;
+import org.eclipse.rdf4j.model.Statement;
 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.Join;
-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.algebra.*;
 import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
 import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
 import org.eclipse.rdf4j.query.parser.ParsedQuery;
@@ -145,6 +143,45 @@ public void testValues() throws RDF4JException {
 		testOptimizer(expectedQuery, query);
 	}
 
+	@Test
+	public void testOptionalWithSubSelect() throws RDF4JException {
+		String query = String.join("\n", "",
+				"prefix ex: <ex:> ",
+				"select * where {",
+				"optional { ?b ex:z ?q . }",
+				"{",
+				"	select ?b ?a ?x where {",
+				"	   ex:b ?a ?x. ",
+				"      ex:b ex:a ?x. ",
+				"}",
+				"}",
+				"}"
+		);
+
+		// we expect the subselect to be optimized too.
+		// ex:b ex:a ?x.
+		// ex:b ?a ?x.
+
+		SPARQLParser parser = new SPARQLParser();
+		ParsedQuery q = parser.parseQuery(query, null);
+		QueryJoinOptimizer opt = new QueryJoinOptimizer();
+		QueryRoot optRoot = new QueryRoot(q.getTupleExpr());
+		opt.optimize(optRoot, null, null);
+
+		StatementFinder stmtFinder = new StatementFinder();
+		optRoot.visit(stmtFinder);
+		List<StatementPattern> stmts = stmtFinder.getStatements();
+
+		assertEquals(stmts.size(), 3);
+		assertEquals(stmts.get(0).getSubjectVar().getValue().stringValue(), "ex:b");
+		assertEquals(stmts.get(0).getPredicateVar().getValue().stringValue(), "ex:a");
+		assertEquals(stmts.get(0).getObjectVar().getValue(), null);
+		assertEquals(stmts.get(1).getSubjectVar().getValue().stringValue(), "ex:b");
+		assertEquals(stmts.get(1).getPredicateVar().getValue(), null);
+		assertEquals(stmts.get(1).getObjectVar().getValue(), null);
+
+	}
+
 	@Override
 	public QueryJoinOptimizer getOptimizer() {
 		return new QueryJoinOptimizer();
@@ -190,4 +227,18 @@ public Join getJoin() {
 		}
 	}
 
+	class StatementFinder extends AbstractQueryModelVisitor<RuntimeException> {
+
+		private List<StatementPattern> statements = new ArrayList<>();
+
+		@Override
+		public void meet(StatementPattern st) {
+			this.statements.add(st);
+		}
+
+		public List<StatementPattern> getStatements() {
+			return statements;
+		}
+	}
+
 }