Merge pull request #2403 from eclipse/GH-2187-parent-querymodelnode

GH-2187 keep parent reference in querymodelnode consistent

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

@@ -112,7 +112,8 @@ public void meet(ArbitraryLengthPath node) {
 			// the length of the path is unknown but expected to be _at least_ twice that of a normal
 			// statement pattern.
 			cardinality = 2.0 * getCardinality(
-					new StatementPattern(node.getSubjectVar(), pathVar, node.getObjectVar(), node.getContextVar()));
+					new StatementPattern(node.getSubjectVar().clone(), pathVar, node.getObjectVar().clone(),
+							node.getContextVar() != null ? node.getContextVar().clone() : null));
 		}
 
 		@Override

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

@@ -0,0 +1,54 @@
+/******************************************************************************* 
+ * Copyright (c) 2021 Eclipse RDF4J contributors. 
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Eclipse Distribution License v1.0 
+ * which accompanies this distribution, and is available at 
+ * http://www.eclipse.org/org/documents/edl-v10.php. 
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation.impl;
+
+import java.util.ArrayDeque;
+
+import org.eclipse.rdf4j.query.BindingSet;
+import org.eclipse.rdf4j.query.Dataset;
+import org.eclipse.rdf4j.query.algebra.QueryModelNode;
+import org.eclipse.rdf4j.query.algebra.TupleExpr;
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizer;
+import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Cleans up {@link QueryModelNode#getParentNode()} references that have become inconsistent with the actual algebra
+ * tree structure due to optimization operations. Typically used at the very end of the optimization pipeline.
+ * 
+ * @author Jeen Broekstra
+ */
+public class ParentReferenceCleaner implements QueryOptimizer {
+
+	private static final Logger logger = LoggerFactory.getLogger(ParentReferenceCleaner.class);
+
+	@Override
+	public void optimize(TupleExpr tupleExpr, Dataset dataset, BindingSet bindings) {
+		tupleExpr.visit(new ParentFixingVisitor());
+	}
+
+	private class ParentFixingVisitor extends AbstractQueryModelVisitor<RuntimeException> {
+
+		private final ArrayDeque<QueryModelNode> ancestors = new ArrayDeque<>();
+
+		@Override
+		protected void meetNode(QueryModelNode node) throws RuntimeException {
+			QueryModelNode expectedParent = ancestors.peekLast();
+			if (node.getParentNode() != expectedParent) {
+				logger.debug("unexpected parent for node {}: {} (expected {})", node, node.getParentNode(),
+						expectedParent);
+				node.setParentNode(expectedParent);
+			}
+
+			ancestors.addLast(node);
+			super.meetNode(node);
+			ancestors.pollLast();
+		}
+	}
+}

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

@@ -56,7 +56,8 @@ public Iterable<QueryOptimizer> getOptimizers() {
 				new QueryJoinOptimizer(evaluationStatistics),
 				new IterativeEvaluationOptimizer(),
 				new FilterOptimizer(),
-				new OrderLimitOptimizer());
+				new OrderLimitOptimizer(),
+				new ParentReferenceCleaner());
 	}
 
 }

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

@@ -260,7 +260,8 @@ private void createIteration() throws QueryEvaluationException {
 			// the variable must remain unbound for this solution see https://www.w3.org/TR/sparql11-query/#assignment
 			currentIter = new EmptyIteration<>();
 		} else if (currentLength == 0L) {
-			ZeroLengthPath zlp = new ZeroLengthPath(scope, startVar, endVar, contextVar);
+			ZeroLengthPath zlp = new ZeroLengthPath(scope, startVar.clone(), endVar.clone(),
+					contextVar != null ? contextVar.clone() : null);
 			currentIter = this.evaluationStrategyImpl.evaluate(zlp, bindings);
 			currentLength++;
 		} else if (currentLength == 1) {

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

@@ -0,0 +1,95 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Eclipse RDF4J contributors.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Distribution License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.rdf4j.query.QueryLanguage;
+import org.eclipse.rdf4j.query.algebra.QueryModelNode;
+import org.eclipse.rdf4j.query.algebra.TupleExpr;
+import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
+import org.eclipse.rdf4j.query.impl.EmptyBindingSet;
+import org.eclipse.rdf4j.query.parser.ParsedTupleQuery;
+import org.eclipse.rdf4j.query.parser.QueryParserUtil;
+import org.junit.jupiter.api.Test;
+
+public abstract class QueryOptimizerTest {
+
+	@Test
+	public void testParentReferencesConsistent_pathExpressions() {
+		ParsedTupleQuery query = QueryParserUtil.parseTupleQuery(QueryLanguage.SPARQL,
+				"select * where {?a ?b ?c. ?c <http://a>* ?d}", null);
+
+		TupleExpr expr = query.getTupleExpr();
+
+		getOptimizer().optimize(expr, null, EmptyBindingSet.getInstance());
+
+		ParentCheckingVisitor checker = new ParentCheckingVisitor();
+		expr.visit(checker);
+
+		assertThat(checker.getInconsistentNodes()).isEmpty();
+	}
+
+	@Test
+	public void testParentReferencesConsistent_filter() {
+		ParsedTupleQuery query = QueryParserUtil.parseTupleQuery(QueryLanguage.SPARQL,
+				"select * where {?a ?b ?c. FILTER(?c = <urn:foo>) }", null);
+
+		TupleExpr expr = query.getTupleExpr();
+
+		getOptimizer().optimize(expr, null, EmptyBindingSet.getInstance());
+
+		ParentCheckingVisitor checker = new ParentCheckingVisitor();
+		expr.visit(checker);
+
+		assertThat(checker.getInconsistentNodes()).isEmpty();
+	}
+
+	@Test
+	public void testParentReferencesConsistent_subselect() {
+		ParsedTupleQuery query = QueryParserUtil.parseTupleQuery(QueryLanguage.SPARQL,
+				"select * where {?a ?b ?c. { select ?a ?z where { ?a a ?z } }}", null);
+
+		TupleExpr expr = query.getTupleExpr();
+
+		getOptimizer().optimize(expr, null, EmptyBindingSet.getInstance());
+
+		ParentCheckingVisitor checker = new ParentCheckingVisitor();
+		expr.visit(checker);
+
+		assertThat(checker.getInconsistentNodes()).isEmpty();
+	}
+
+	public abstract QueryOptimizer getOptimizer();
+
+	private class ParentCheckingVisitor extends AbstractQueryModelVisitor<RuntimeException> {
+		private final ArrayDeque<QueryModelNode> ancestors = new ArrayDeque<>();
+		private List<QueryModelNode> inconsistentNodes = new ArrayList<>();
+
+		@Override
+		protected void meetNode(QueryModelNode node) throws RuntimeException {
+			QueryModelNode expectedParent = ancestors.peekLast();
+			if (node.getParentNode() != expectedParent) {
+				inconsistentNodes.add(node);
+			}
+
+			ancestors.addLast(node);
+			super.meetNode(node);
+			ancestors.pollLast();
+		}
+
+		public List<QueryModelNode> getInconsistentNodes() {
+			return inconsistentNodes;
+		}
+	}
+
+}

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

@@ -0,0 +1,19 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Eclipse RDF4J contributors.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Distribution License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/org/documents/edl-v10.php.
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation.impl;
+
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
+
+public class BindingAssignerTest extends QueryOptimizerTest {
+
+	@Override
+	public BindingAssigner getOptimizer() {
+		return new BindingAssigner();
+	}
+
+}

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

@@ -0,0 +1,23 @@
+/******************************************************************************* 
+ * Copyright (c) 2021 Eclipse RDF4J contributors. 
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Eclipse Distribution License v1.0 
+ * which accompanies this distribution, and is available at 
+ * http://www.eclipse.org/org/documents/edl-v10.php. 
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation.impl;
+
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
+
+/**
+ * @author jeen
+ *
+ */
+public class CompareOptimizerTest extends QueryOptimizerTest {
+
+	@Override
+	public CompareOptimizer getOptimizer() {
+		return new CompareOptimizer();
+	}
+
+}

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

@@ -0,0 +1,23 @@
+/******************************************************************************* 
+ * Copyright (c) 2021 Eclipse RDF4J contributors. 
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Eclipse Distribution License v1.0 
+ * which accompanies this distribution, and is available at 
+ * http://www.eclipse.org/org/documents/edl-v10.php. 
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation.impl;
+
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
+
+/**
+ * @author jeen
+ *
+ */
+public class ConjunctiveConstraintSplitterTest extends QueryOptimizerTest {
+
+	@Override
+	public ConjunctiveConstraintSplitter getOptimizer() {
+		return new ConjunctiveConstraintSplitter();
+	}
+
+}

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

@@ -7,9 +7,10 @@
  *******************************************************************************/
 package org.eclipse.rdf4j.query.algebra.evaluation.impl;
 
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
 
 import org.eclipse.rdf4j.RDF4JException;
 import org.eclipse.rdf4j.common.iteration.CloseableIteration;
@@ -24,15 +25,16 @@
 import org.eclipse.rdf4j.query.algebra.TupleExpr;
 import org.eclipse.rdf4j.query.algebra.evaluation.EvaluationStrategy;
 import org.eclipse.rdf4j.query.algebra.evaluation.QueryBindingSet;
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
 import org.eclipse.rdf4j.query.algebra.helpers.QueryModelVisitorBase;
 import org.eclipse.rdf4j.query.impl.EmptyBindingSet;
 import org.eclipse.rdf4j.query.parser.ParsedQuery;
 import org.eclipse.rdf4j.query.parser.QueryParserUtil;
-import org.junit.Test;
+import org.junit.jupiter.api.Test;
 
 /**
  */
-public class ConstantOptimizerTest {
+public class ConstantOptimizerTest extends QueryOptimizerTest {
 
 	@Test
 	public void testAndOptimization() throws RDF4JException {
@@ -57,7 +59,7 @@ public void testAndOptimization() throws RDF4JException {
 		TupleExpr optimized = optimize(pq.getTupleExpr().clone(), constants, strategy);
 
 		optimized.visit(finder);
-		assertFalse("optimized query should no longer contain && operator", finder.logicalAndfound);
+		assertThat(finder.logicalAndfound).isFalse();
 
 		CloseableIteration<BindingSet, QueryEvaluationException> result = strategy.evaluate(optimized,
 				new EmptyBindingSet());
@@ -92,7 +94,7 @@ public void testFunctionOptimization() throws RDF4JException {
 		TupleExpr optimized = optimize(pq.getTupleExpr().clone(), constants, strategy);
 
 		optimized.visit(finder);
-		assertFalse("optimized query should no longer contain function call", finder.functionCallFound);
+		assertThat(finder.functionCallFound).isFalse();
 
 		CloseableIteration<BindingSet, QueryEvaluationException> result = strategy.evaluate(optimized,
 				new EmptyBindingSet());
@@ -136,4 +138,9 @@ private TupleExpr optimize(TupleExpr expr, BindingSet bs, EvaluationStrategy str
 		new ConstantOptimizer(strategy).optimize(optRoot, null, bs);
 		return optRoot;
 	}
+
+	@Override
+	public ConstantOptimizer getOptimizer() {
+		return new ConstantOptimizer(mock(StrictEvaluationStrategy.class));
+	}
 }

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

@@ -0,0 +1,23 @@
+/******************************************************************************* 
+ * Copyright (c) 2021 Eclipse RDF4J contributors. 
+ * All rights reserved. This program and the accompanying materials 
+ * are made available under the terms of the Eclipse Distribution License v1.0 
+ * which accompanies this distribution, and is available at 
+ * http://www.eclipse.org/org/documents/edl-v10.php. 
+ *******************************************************************************/
+package org.eclipse.rdf4j.query.algebra.evaluation.impl;
+
+import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizerTest;
+
+/**
+ * @author jeen
+ *
+ */
+public class DisjunctiveConstraintOptimizerTest extends QueryOptimizerTest {
+
+	@Override
+	public DisjunctiveConstraintOptimizer getOptimizer() {
+		return new DisjunctiveConstraintOptimizer();
+	}
+
+}