Skip to content

Commit effefd6

Browse files
hmottestadhmottestad
committed
improvements
1 parent adf6aaf commit effefd6

6 files changed

Lines changed: 174 additions & 22 deletions

File tree

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbDeferredFilterPlacer.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@
2222
import java.util.Optional;
2323
import java.util.Set;
2424

25+
import org.eclipse.rdf4j.query.algebra.Compare;
2526
import org.eclipse.rdf4j.query.algebra.Filter;
2627
import org.eclipse.rdf4j.query.algebra.Not;
2728
import org.eclipse.rdf4j.query.algebra.StatementPattern;
2829
import org.eclipse.rdf4j.query.algebra.TupleExpr;
30+
import org.eclipse.rdf4j.query.algebra.ValueExpr;
2931
import org.eclipse.rdf4j.query.algebra.evaluation.optimizer.JoinOrderPlanner;
32+
import org.eclipse.rdf4j.query.algebra.helpers.AbstractSimpleQueryModelVisitor;
3033

3134
final class LmdbDeferredFilterPlacer {
3235

@@ -297,7 +300,27 @@ private boolean canApplyDeferredFilterToBindingPrefix(DeferredFilter deferredFil
297300
|| !Collections.disjoint(prefixBindingNames, deferredFilter.requiredVars))
298301
&& !prefixBindingNames.containsAll(deferredFilter.requiredVars)
299302
&& availableNames.containsAll(deferredFilter.requiredVars)
300-
&& assignmentBindingNames.containsAll(deferredFilter.requiredVars);
303+
&& !Collections.disjoint(assignmentBindingNames, deferredFilter.requiredVars)
304+
&& canApplySplitPrefixFilter(deferredFilter, assignmentBindingNames);
305+
}
306+
307+
private boolean canApplySplitPrefixFilter(DeferredFilter deferredFilter, Set<String> assignmentBindingNames) {
308+
return assignmentBindingNames.containsAll(deferredFilter.requiredVars)
309+
|| !containsNotEquals(deferredFilter.condition);
310+
}
311+
312+
private boolean containsNotEquals(ValueExpr condition) {
313+
boolean[] contains = { false };
314+
condition.visit(new AbstractSimpleQueryModelVisitor<RuntimeException>() {
315+
@Override
316+
public void meet(Compare node) {
317+
if (node.getOperator() == Compare.CompareOp.NE) {
318+
contains[0] = true;
319+
}
320+
super.meet(node);
321+
}
322+
});
323+
return contains[0];
301324
}
302325

303326
private TupleExpr applyCompatibleLocalDeferredFilters(TupleExpr tupleExpr, List<DeferredFilter> deferredFilters) {

‎core/sail/lmdb/src/main/java/org/eclipse/rdf4j/sail/lmdb/LmdbUnionFilterDistributor.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,9 @@ private static TupleExpr buildBranch(TupleExpr branch, List<TupleExpr> prefixFac
128128
List<DeferredFilter> filters, Set<String> outerBoundVars, BranchOptimizer branchOptimizer,
129129
JoinFactory joinFactory, FilterWrapper filterWrapper) {
130130
TupleExpr root = prependPrefix(branch.clone(), prefixFactors, joinFactory);
131+
root = filterWrapper.wrap(root, filters, "unionBranch");
131132
root = branchOptimizer.optimize(root, outerBoundVars);
132-
return filterWrapper.wrap(root, filters, "unionBranch");
133+
return root;
133134
}
134135

135136
private static TupleExpr prependPrefix(TupleExpr branch, List<TupleExpr> prefixFactors, JoinFactory joinFactory) {

‎core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/LmdbUnionFilterDistributorTest.java‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,27 @@
1515
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
1616
import static org.junit.jupiter.api.Assertions.assertTrue;
1717

18+
import java.util.ArrayDeque;
1819
import java.util.ArrayList;
1920
import java.util.List;
2021
import java.util.Set;
2122

2223
import org.eclipse.rdf4j.model.impl.SimpleValueFactory;
2324
import org.eclipse.rdf4j.query.BindingSet;
2425
import org.eclipse.rdf4j.query.algebra.BindingSetAssignment;
26+
import org.eclipse.rdf4j.query.algebra.Compare;
2527
import org.eclipse.rdf4j.query.algebra.Extension;
2628
import org.eclipse.rdf4j.query.algebra.ExtensionElem;
2729
import org.eclipse.rdf4j.query.algebra.Filter;
2830
import org.eclipse.rdf4j.query.algebra.Join;
2931
import org.eclipse.rdf4j.query.algebra.LeftJoin;
32+
import org.eclipse.rdf4j.query.algebra.Or;
3033
import org.eclipse.rdf4j.query.algebra.StatementPattern;
3134
import org.eclipse.rdf4j.query.algebra.TupleExpr;
3235
import org.eclipse.rdf4j.query.algebra.Union;
36+
import org.eclipse.rdf4j.query.algebra.ValueConstant;
3337
import org.eclipse.rdf4j.query.algebra.Var;
38+
import org.eclipse.rdf4j.query.algebra.evaluation.impl.EvaluationStatistics;
3439
import org.eclipse.rdf4j.query.impl.MapBindingSet;
3540
import org.junit.jupiter.api.Test;
3641

@@ -102,6 +107,95 @@ void distributesFinitePrefixWhileKeepingSuffixFactorsOutsideUnionBranches() {
102107
assertTrue(containsStatementPatternWithObject(root.getRightArg(), "optName"));
103108
}
104109

110+
@Test
111+
void exposesBranchFiltersToBranchOptimizer() {
112+
TupleExpr union = new Union(
113+
new Join(statementPattern("entity", "http://example.com/theme/engineering/type", "kind"),
114+
statementPattern("entity", "http://example.com/theme/engineering/name", "name")),
115+
new Join(statementPattern("entity", "http://example.com/theme/engineering/type", "kind"),
116+
statementPattern("entity", "http://example.com/theme/engineering/name", "name")));
117+
DeferredFilter filter = new DeferredFilter(
118+
new Compare(new Var("name"), new Var("target"), Compare.CompareOp.EQ),
119+
Set.of("name", "target"), 1, 0, null, Set.of(),
120+
new EvaluationStatistics.FilterPassEstimate(-1.0d,
121+
EvaluationStatistics.FilterPassEstimate.Source.UNKNOWN));
122+
boolean[] sawBranchFilter = { false };
123+
124+
TupleExpr distributed = LmdbUnionFilterDistributor.tryDistribute(List.of(values("target", "REQ-1000"), union),
125+
List.of(filter), Set.of(), (tupleExpr, ignored) -> {
126+
sawBranchFilter[0] |= tupleExpr instanceof Filter;
127+
return tupleExpr;
128+
}, Join::new, LmdbUnionFilterDistributorTest::wrapFilters);
129+
130+
assertInstanceOf(Union.class, distributed);
131+
assertTrue(sawBranchFilter[0]);
132+
}
133+
134+
@Test
135+
void placesCorrelatedBranchFilterOnFinitePrefixAssignment() {
136+
StatementPattern namePattern = statementPattern("entity", "http://example.com/theme/engineering/name", "name");
137+
BindingSetAssignment targetValues = values("target", "REQ-1000", "REQ-1001");
138+
DeferredFilter filter = new DeferredFilter(
139+
new Compare(new Var("name"), new Var("target"), Compare.CompareOp.EQ),
140+
Set.of("name", "target"), 1, 0, null, Set.of(),
141+
new EvaluationStatistics.FilterPassEstimate(-1.0d,
142+
EvaluationStatistics.FilterPassEstimate.Source.UNKNOWN));
143+
LmdbDeferredFilterPlacer placer = new LmdbDeferredFilterPlacer((tupleExpr, ignored) -> tupleExpr, Join::new,
144+
LmdbUnionFilterDistributorTest::wrapFilters);
145+
146+
TupleExpr root = placer.buildSegmentRoot(new ArrayDeque<>(List.of(namePattern, targetValues)),
147+
List.of(filter), Set.of());
148+
149+
Join join = assertInstanceOf(Join.class, root);
150+
assertInstanceOf(StatementPattern.class, join.getLeftArg());
151+
Filter assignmentFilter = assertInstanceOf(Filter.class, join.getRightArg());
152+
assertInstanceOf(BindingSetAssignment.class, assignmentFilter.getArg());
153+
}
154+
155+
@Test
156+
void placesCorrelatedOrBranchFilterOnFinitePrefixAssignment() {
157+
StatementPattern namePattern = statementPattern("entity", "http://example.com/theme/engineering/name", "name");
158+
BindingSetAssignment targetValues = values("target", "REQ-1000", "REQ-1001");
159+
DeferredFilter filter = new DeferredFilter(
160+
new Or(new Compare(new Var("name"), new Var("target"), Compare.CompareOp.EQ),
161+
new Compare(new Var("name"), new ValueConstant(VF.createLiteral("REQ-1002")),
162+
Compare.CompareOp.EQ)),
163+
Set.of("name", "target"), 1, 0, null, Set.of(),
164+
new EvaluationStatistics.FilterPassEstimate(-1.0d,
165+
EvaluationStatistics.FilterPassEstimate.Source.UNKNOWN));
166+
LmdbDeferredFilterPlacer placer = new LmdbDeferredFilterPlacer((tupleExpr, ignored) -> tupleExpr, Join::new,
167+
LmdbUnionFilterDistributorTest::wrapFilters);
168+
169+
TupleExpr root = placer.buildSegmentRoot(new ArrayDeque<>(List.of(namePattern, targetValues)),
170+
List.of(filter), Set.of());
171+
172+
Join join = assertInstanceOf(Join.class, root);
173+
assertInstanceOf(StatementPattern.class, join.getLeftArg());
174+
Filter assignmentFilter = assertInstanceOf(Filter.class, join.getRightArg());
175+
assertInstanceOf(BindingSetAssignment.class, assignmentFilter.getArg());
176+
}
177+
178+
@Test
179+
void keepsSplitFiniteInequalityOnBindingWindow() {
180+
BindingSetAssignment userValues = values("u", "user0", "user1", "user2");
181+
BindingSetAssignment peerValues = values("v", "user0", "user1", "user2");
182+
DeferredFilter filter = new DeferredFilter(
183+
new Compare(new Var("u"), new Var("v"), Compare.CompareOp.NE),
184+
Set.of("u", "v"), 1, 0, null, Set.of(),
185+
new EvaluationStatistics.FilterPassEstimate(-1.0d,
186+
EvaluationStatistics.FilterPassEstimate.Source.UNKNOWN));
187+
LmdbDeferredFilterPlacer placer = new LmdbDeferredFilterPlacer((tupleExpr, ignored) -> tupleExpr, Join::new,
188+
LmdbUnionFilterDistributorTest::wrapFilters);
189+
190+
TupleExpr root = placer.buildSegmentRoot(new ArrayDeque<>(List.of(userValues, peerValues)),
191+
List.of(filter), Set.of());
192+
193+
Filter windowFilter = assertInstanceOf(Filter.class, root);
194+
Join finiteWindow = assertInstanceOf(Join.class, windowFilter.getArg());
195+
assertInstanceOf(BindingSetAssignment.class, finiteWindow.getLeftArg());
196+
assertInstanceOf(BindingSetAssignment.class, finiteWindow.getRightArg());
197+
}
198+
105199
private static void assertPrefixInsideExtension(TupleExpr branch, boolean expectOptNameAssignment,
106200
boolean expectOptNamePattern) {
107201
Extension extension = assertInstanceOf(Extension.class, branch);
@@ -133,6 +227,14 @@ private static BindingSetAssignment values(String bindingName, String... values)
133227
return assignment;
134228
}
135229

230+
private static TupleExpr wrapFilters(TupleExpr root, List<DeferredFilter> filters, String placement) {
231+
TupleExpr result = root;
232+
for (DeferredFilter filter : filters) {
233+
result = new Filter(result, filter.condition.clone());
234+
}
235+
return result;
236+
}
237+
136238
private static boolean containsBindingSetAssignmentFor(TupleExpr tupleExpr, String bindingName) {
137239
if (tupleExpr instanceof BindingSetAssignment) {
138240
return ((BindingSetAssignment) tupleExpr).getBindingNames().contains(bindingName);

‎core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/benchmark/LmdbEngineeringThemeQueryRegressionTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,8 @@ private static void assertNamePatternUsesBoundValue(String plan, String label, b
336336
String pattern = statementPatternWindow(plan, predicateIndex, "StatementPattern");
337337
assertContains(pattern, "o: Var (name=name) (bindingState=bound)");
338338
if (requirePlannerMetrics) {
339-
assertContainsAny(pattern, "plannedBoundVars=[name]", "plannedBoundVars=name");
340339
assertContains(pattern, "plannedLookupComponents=[P, O]");
340+
assertContains(pattern, "plannedIndexAccessMode=directLookup");
341341
}
342342
}
343343

‎core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/benchmark/LmdbFlaggedThemeOptimizedQueryRegressionTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,11 +319,13 @@ private static List<String> invariantMismatches(Expectation expectation, Optimiz
319319
if (!snapshot.renderedQuery.contains("SELECT")) {
320320
mismatches.add(key + " should still render an optimized query\n" + snapshot.renderedQuery);
321321
}
322-
if (!snapshot.plan.contains("plannerId=lmdb-sketch")) {
323-
mismatches.add(key + " should use LMDB sketch planning\n" + snapshot.plan);
322+
if (!snapshot.plan.contains("plannerId=lmdb-sketch")
323+
&& !snapshot.plan.contains("plannerId=lmdb-finite-anchor")) {
324+
mismatches.add(key + " should use LMDB sketch or finite-anchor planning\n" + snapshot.plan);
324325
}
325326
if (!snapshot.plan.contains("plannerPath=ROBUST_USED")
326-
&& !snapshot.plan.contains("plannerPath=ANTI_JOIN_RETAINED")) {
327+
&& !snapshot.plan.contains("plannerPath=ANTI_JOIN_RETAINED")
328+
&& !snapshot.plan.contains("plannerPath=CANONICAL_FINITE_ANCHOR")) {
327329
mismatches.add(key + " should use the robust planner path\n" + snapshot.plan);
328330
}
329331
if (snapshot.plan.contains("plannerPath=UNSUPPORTED_SHAPE")) {

‎core/sail/lmdb/src/test/java/org/eclipse/rdf4j/sail/lmdb/benchmark/LmdbThemeTopRegressionSnapshotTest.java‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -564,15 +564,27 @@ private static List<String> socialMediaQ0FastShapeMismatches(String renderedQuer
564564
requireBefore(mismatches, renderedQuery, "FILTER (?u != ?v)",
565565
"?u <http://example.com/theme/social/follows> ?v .",
566566
"self-pair pruning should run before the follows lookup");
567-
requireBefore(mismatches, renderedQuery, "?u <http://example.com/theme/social/follows> ?v .",
568-
"VALUES ?optName",
569-
"finite name anchor should run after the bounded follows lookup");
570-
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
571-
"?u <http://example.com/theme/social/name> ?optName .",
572-
"optName VALUES should make the name lookup exact");
573-
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
574-
"FILTER (?optName IN (\"user0\", \"user1\", \"user2\"))",
575-
"optName VALUES should retain the literal filter");
567+
if (renderedQuery.contains("VALUES ?optName")) {
568+
requireBefore(mismatches, renderedQuery, "?u <http://example.com/theme/social/follows> ?v .",
569+
"VALUES ?optName",
570+
"finite name anchor should run after the bounded follows lookup");
571+
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
572+
"?u <http://example.com/theme/social/name> ?optName .",
573+
"optName VALUES should make the name lookup exact");
574+
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
575+
"FILTER (?optName IN (\"user0\", \"user1\", \"user2\"))",
576+
"optName VALUES should retain the literal filter");
577+
} else {
578+
requireBefore(mismatches, renderedQuery, "?u <http://example.com/theme/social/follows> ?v .",
579+
"?u <http://example.com/theme/social/name> ?optName .",
580+
"optional name lookup should stay after the bounded follows lookup");
581+
requireBefore(mismatches, renderedQuery, "?u <http://example.com/theme/social/name> ?optName .",
582+
"FILTER (?optName IN (\"user0\", \"user1\", \"user2\"))",
583+
"literal optName filter should stay after the optional name lookup");
584+
requireAnyPredicateHeaderContains(mismatches, plan, "http://example.com/theme/social/name",
585+
"plannedLookupComponents=[S, P]",
586+
"optional name lookup should use bound subject access when optName is filtered afterwards");
587+
}
576588
requireBefore(mismatches, renderedQuery, "FILTER (?optName IN (\"user0\", \"user1\", \"user2\"))",
577589
"BIND(CONCAT(STR(?u), STR(?v)) AS ?pair)",
578590
"literal optName filter should remain before projection");
@@ -668,14 +680,26 @@ private static List<String> socialMediaQ10FastShapeMismatches(String renderedQue
668680
requireBefore(mismatches, renderedQuery, "?d <http://example.com/theme/social/follows> ?e .",
669681
"?e <http://example.com/theme/social/follows> ?a .",
670682
"follows cascade should close with e/a after d/e");
671-
requireBefore(mismatches, renderedQuery, "?e <http://example.com/theme/social/follows> ?a .",
672-
"VALUES ?optName",
673-
"optional name finite anchor should stay after the bounded follows cascade");
674-
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
675-
"?e <http://example.com/theme/social/name> ?optName .",
676-
"optName finite anchor should make the optional name lookup exact");
683+
if (renderedQuery.contains("VALUES ?optName")) {
684+
requireBefore(mismatches, renderedQuery, "?e <http://example.com/theme/social/follows> ?a .",
685+
"VALUES ?optName",
686+
"optional name finite anchor should stay after the bounded follows cascade");
687+
requireBefore(mismatches, renderedQuery, "VALUES ?optName",
688+
"?e <http://example.com/theme/social/name> ?optName .",
689+
"optName finite anchor should make the optional name lookup exact");
690+
} else {
691+
requireBefore(mismatches, renderedQuery, "?e <http://example.com/theme/social/follows> ?a .",
692+
"?e <http://example.com/theme/social/name> ?optName .",
693+
"optional name lookup should stay after the bounded follows cascade");
694+
requireBefore(mismatches, renderedQuery, "?e <http://example.com/theme/social/name> ?optName .",
695+
"FILTER ((?optName IN",
696+
"optional name literal filter should stay after the optional name lookup");
697+
requireAnyPredicateHeaderContains(mismatches, plan, "http://example.com/theme/social/name",
698+
"plannedLookupComponents=[S, P]",
699+
"optional name lookup should use bound subject access when optName is filtered afterwards");
700+
}
677701
requireBefore(mismatches, renderedQuery, "?e <http://example.com/theme/social/name> ?optName .",
678-
"FILTER EXISTS",
702+
"EXISTS {",
679703
"correlated EXISTS name probe should run after the optional name lookup");
680704
requireContains(mismatches, renderedQuery, "VALUES ?name { \"user7\" \"user8\" }",
681705
"correlated EXISTS should expose its small literal name domain");

0 commit comments

Comments
 (0)