Skip to content

Commit 4d3a158

Browse files
committed
GH-4872 fix
1 parent 59ebafa commit 4d3a158

1 file changed

Lines changed: 48 additions & 4 deletions

File tree

  • core/queryalgebra/evaluation/src/main/java/org/eclipse/rdf4j/query/algebra/evaluation/optimizer

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

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
package org.eclipse.rdf4j.query.algebra.evaluation.optimizer;
1313

14+
import java.util.Objects;
1415
import java.util.Set;
1516

1617
import org.eclipse.rdf4j.query.BindingSet;
@@ -31,6 +32,7 @@
3132
import org.eclipse.rdf4j.query.algebra.StatementPattern;
3233
import org.eclipse.rdf4j.query.algebra.TupleExpr;
3334
import org.eclipse.rdf4j.query.algebra.Union;
35+
import org.eclipse.rdf4j.query.algebra.VariableScopeChange;
3436
import org.eclipse.rdf4j.query.algebra.evaluation.QueryOptimizer;
3537
import org.eclipse.rdf4j.query.algebra.helpers.AbstractQueryModelVisitor;
3638
import org.eclipse.rdf4j.query.algebra.helpers.AbstractSimpleQueryModelVisitor;
@@ -78,9 +80,47 @@ public class FilterOptimizer implements QueryOptimizer {
7880

7981
@Override
8082
public void optimize(TupleExpr tupleExpr, Dataset dataset, BindingSet bindings) {
81-
tupleExpr.visit(new FilterUnMerger());
82-
tupleExpr.visit(new FilterOrganizer());
83-
tupleExpr.visit(new FilterMerger());
83+
Objects.requireNonNull(tupleExpr, "tupleExpr must not be null");
84+
optimizeScope(tupleExpr);
85+
}
86+
87+
/**
88+
* Bottom‑up optimisation: optimise each child subtree that starts a new variable scope, then run the three filter
89+
* passes on the current subtree.
90+
*/
91+
private void optimizeScope(TupleExpr expr) {
92+
// 1) recurse into nested scopes first
93+
expr.visit(new AbstractSimpleQueryModelVisitor<>(false) {
94+
final TupleExpr current = expr;
95+
96+
@Override
97+
public void meet(Filter node) throws RuntimeException {
98+
if (node != current && node.isVariableScopeChange()) {
99+
optimizeScope(node);
100+
// do NOT traverse further into that subtree with this visitor
101+
} else {
102+
super.meet(node);
103+
}
104+
}
105+
106+
});
107+
108+
// 2) run the three filter passes for *this* scope
109+
expr.visit(new FilterUnMerger());
110+
expr.visit(new FilterOrganizer());
111+
expr.visit(new FilterMerger());
112+
}
113+
114+
/**
115+
* Copies the {@code isVariableScopeChange} flag from one node to another if both implement
116+
* {@link VariableScopeChange}.
117+
*/
118+
private static void transferScopeChange(QueryModelNode source, QueryModelNode target) {
119+
if (source instanceof VariableScopeChange && target instanceof VariableScopeChange) {
120+
VariableScopeChange src = (VariableScopeChange) source;
121+
VariableScopeChange tgt = (VariableScopeChange) target;
122+
tgt.setVariableScopeChange(src.isVariableScopeChange());
123+
}
84124
}
85125

86126
private static class FilterUnMerger extends AbstractSimpleQueryModelVisitor<RuntimeException> {
@@ -95,6 +135,7 @@ public void meet(Filter filter) {
95135
And and = (And) filter.getCondition();
96136
filter.setCondition(and.getLeftArg().clone());
97137
Filter newFilter = new Filter(filter.getArg().clone(), and.getRightArg().clone());
138+
transferScopeChange(filter, newFilter); // preserve scope flag
98139
filter.replaceChildNode(filter.getArg(), newFilter);
99140
}
100141
super.meet(filter);
@@ -117,6 +158,7 @@ public void meet(Filter filter) {
117158
And merge = new And(childFilter.getCondition().clone(), filter.getCondition().clone());
118159

119160
Filter newFilter = new Filter(childFilter.getArg().clone(), merge);
161+
transferScopeChange(filter, newFilter); // both have same scope flag
120162
parent.replaceChildNode(filter, newFilter);
121163
}
122164
}
@@ -190,6 +232,7 @@ public void meet(LeftJoin leftJoin) {
190232
public void meet(Union union) {
191233
Filter clone = new Filter();
192234
clone.setCondition(filter.getCondition().clone());
235+
transferScopeChange(filter, clone);
193236

194237
relocate(filter, union.getLeftArg());
195238
relocate(clone, union.getRightArg());
@@ -202,6 +245,7 @@ public void meet(Union union) {
202245
public void meet(Difference node) {
203246
Filter clone = new Filter();
204247
clone.setCondition(filter.getCondition().clone());
248+
transferScopeChange(filter, clone);
205249

206250
relocate(filter, node.getLeftArg());
207251
relocate(clone, node.getRightArg());
@@ -214,6 +258,7 @@ public void meet(Difference node) {
214258
public void meet(Intersection node) {
215259
Filter clone = new Filter();
216260
clone.setCondition(filter.getCondition().clone());
261+
transferScopeChange(filter, clone);
217262

218263
relocate(filter, node.getLeftArg());
219264
relocate(clone, node.getRightArg());
@@ -278,5 +323,4 @@ private void relocate(Filter filter, TupleExpr newFilterArg) {
278323
}
279324
}
280325
}
281-
282326
}

0 commit comments

Comments
 (0)