diff --git a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java b/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java index 210cf97394a..4fac2169a6f 100644 --- a/core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizer.java +++ b/core/queryalgebra/evaluation/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 origBoundVars, TupleExpr join) { + + Set 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/core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java b/core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java index 0975c6fb33f..9fbf4546c96 100644 --- a/core/queryalgebra/evaluation/src/test/java/org/eclipse/rdf4j/query/algebra/evaluation/impl/QueryJoinOptimizerTest.java +++ b/core/queryalgebra/evaluation/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: ", + "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 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 { + + private List statements = new ArrayList<>(); + + @Override + public void meet(StatementPattern st) { + this.statements.add(st); + } + + public List getStatements() { + return statements; + } + } + }