Skip to content

Commit 47cb10b

Browse files
hmottestadhmottestad
committed
fix bugs
1 parent c63b138 commit 47cb10b

4 files changed

Lines changed: 194 additions & 11 deletions

File tree

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

Lines changed: 102 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313

1414
import java.util.ArrayList;
1515
import java.util.HashMap;
16+
import java.util.HashSet;
17+
import java.util.LinkedHashMap;
1618
import java.util.LinkedHashSet;
1719
import java.util.List;
1820
import java.util.Map;
@@ -215,8 +217,11 @@ private PlanningTarget tryExtractPlanningTarget(Join node, List<TupleExpr> opera
215217
return null;
216218
}
217219

220+
boolean preserveOuterOperators = filterPartition.filterRewrites()
221+
.stream()
222+
.anyMatch(filterRewrite -> filterRewrite.residualCondition() != null);
218223
List<LmdbLftjPlan.OutputBinding> outputBindings = collectOutputBindings(projection, extension, visibleVariables,
219-
filterPartition.requiredVariables());
224+
filterPartition.requiredVariables(), preserveOuterOperators);
220225
if (outputBindings == null) {
221226
return null;
222227
}
@@ -312,7 +317,10 @@ private boolean isNamedVariable(Var var) {
312317
}
313318

314319
private List<LmdbLftjPlan.OutputBinding> collectOutputBindings(Projection projection, Extension extension,
315-
List<String> visibleVariables, List<String> requiredVariables) {
320+
List<String> visibleVariables, List<String> requiredVariables, boolean preserveOuterOperators) {
321+
if (preserveOuterOperators) {
322+
return collectVisibleInputBindings(projection, extension, visibleVariables, requiredVariables);
323+
}
316324
if (projection == null) {
317325
return extension == null ? List.of() : null;
318326
}
@@ -322,20 +330,53 @@ private List<LmdbLftjPlan.OutputBinding> collectOutputBindings(Projection projec
322330
}
323331
Set<String> visible = Set.copyOf(visibleVariables);
324332
List<LmdbLftjPlan.OutputBinding> outputBindings = new ArrayList<>();
325-
Set<String> coveredSources = new LinkedHashSet<>();
333+
Map<String, String> outputSourcesByName = new LinkedHashMap<>();
326334
for (ProjectionElem projectionElem : projection.getProjectionElemList().getElements()) {
327335
String sourceVariable = resolveProjectedSource(projectionElem, extensionBindings, visible);
328336
if (sourceVariable == null) {
329337
return null;
330338
}
331-
outputBindings.add(new LmdbLftjPlan.OutputBinding(
332-
projectionElem.getProjectionAlias().orElse(projectionElem.getName()),
333-
sourceVariable));
334-
coveredSources.add(sourceVariable);
339+
String outputName = projectionElem.getProjectionAlias().orElse(projectionElem.getName());
340+
String previousSource = outputSourcesByName.putIfAbsent(outputName, sourceVariable);
341+
if (previousSource != null && !previousSource.equals(sourceVariable)) {
342+
return null;
343+
}
344+
outputBindings.add(new LmdbLftjPlan.OutputBinding(outputName, sourceVariable));
335345
}
336346
for (String requiredVariable : requiredVariables) {
337-
if (visible.contains(requiredVariable) && coveredSources.add(requiredVariable)) {
347+
if (!visible.contains(requiredVariable)) {
348+
continue;
349+
}
350+
String previousSource = outputSourcesByName.get(requiredVariable);
351+
if (previousSource == null) {
338352
outputBindings.add(new LmdbLftjPlan.OutputBinding(requiredVariable, requiredVariable));
353+
outputSourcesByName.put(requiredVariable, requiredVariable);
354+
} else if (!previousSource.equals(requiredVariable)) {
355+
return null;
356+
}
357+
}
358+
return outputBindings;
359+
}
360+
361+
private List<LmdbLftjPlan.OutputBinding> collectVisibleInputBindings(Projection projection, Extension extension,
362+
List<String> visibleVariables, List<String> requiredVariables) {
363+
LinkedHashSet<String> neededInputs = new LinkedHashSet<>(requiredVariables);
364+
if (projection != null) {
365+
for (ProjectionElem projectionElem : projection.getProjectionElemList().getElements()) {
366+
neededInputs.add(projectionElem.getName());
367+
}
368+
} else if (extension != null) {
369+
neededInputs.addAll(extension.getBindingNames());
370+
} else {
371+
neededInputs.addAll(visibleVariables);
372+
}
373+
if (extension != null) {
374+
neededInputs = resolveExtensionInputs(neededInputs, extension, Set.copyOf(visibleVariables));
375+
}
376+
List<LmdbLftjPlan.OutputBinding> outputBindings = new ArrayList<>();
377+
for (String visibleVariable : visibleVariables) {
378+
if (neededInputs.contains(visibleVariable)) {
379+
outputBindings.add(new LmdbLftjPlan.OutputBinding(visibleVariable, visibleVariable));
339380
}
340381
}
341382
return outputBindings;
@@ -436,6 +477,59 @@ public void meet(Var node) {
436477
});
437478
}
438479

480+
private LinkedHashSet<String> resolveExtensionInputs(Set<String> requiredNames, Extension extension,
481+
Set<String> visibleVariables) {
482+
Map<String, ValueExpr> expressionsByName = new LinkedHashMap<>();
483+
for (ExtensionElem element : extension.getElements()) {
484+
expressionsByName.put(element.getName(), element.getExpr());
485+
}
486+
LinkedHashSet<String> resolvedInputs = new LinkedHashSet<>();
487+
for (String requiredName : requiredNames) {
488+
collectExtensionInputs(requiredName, visibleVariables, expressionsByName, resolvedInputs, new HashSet<>());
489+
}
490+
return resolvedInputs;
491+
}
492+
493+
private void collectExtensionInputs(String variableName, Set<String> visibleVariables,
494+
Map<String, ValueExpr> expressionsByName, Set<String> resolvedInputs, Set<String> visiting) {
495+
if (visibleVariables.contains(variableName)) {
496+
resolvedInputs.add(variableName);
497+
return;
498+
}
499+
ValueExpr expression = expressionsByName.get(variableName);
500+
if (expression == null || !visiting.add(variableName)) {
501+
return;
502+
}
503+
collectExtensionInputs(expression, visibleVariables, expressionsByName, resolvedInputs, visiting);
504+
visiting.remove(variableName);
505+
}
506+
507+
private void collectExtensionInputs(ValueExpr expression, Set<String> visibleVariables,
508+
Map<String, ValueExpr> expressionsByName, Set<String> resolvedInputs, Set<String> visiting) {
509+
expression.visit(new AbstractQueryModelVisitor<RuntimeException>() {
510+
@Override
511+
public void meet(Var node) {
512+
if (!isNamedVariable(node)) {
513+
return;
514+
}
515+
String variableName = node.getName();
516+
if (visibleVariables.contains(variableName)) {
517+
resolvedInputs.add(variableName);
518+
return;
519+
}
520+
ValueExpr nestedExpression = expressionsByName.get(variableName);
521+
if (nestedExpression != null && visiting.add(variableName)) {
522+
try {
523+
collectExtensionInputs(nestedExpression, visibleVariables, expressionsByName, resolvedInputs,
524+
visiting);
525+
} finally {
526+
visiting.remove(variableName);
527+
}
528+
}
529+
}
530+
});
531+
}
532+
439533
private Map<String, String> collectExtensionBindings(Extension extension, List<String> visibleVariables) {
440534
if (extension == null) {
441535
return Map.of();

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

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,31 @@ void fullCodegenShouldFuseSupportedFilterAndProjectionIntoLftjPlan() throws Exce
540540
}
541541
}
542542

543+
@Test
544+
void fullCodegenShouldMatchFallbackForAliasedProjectionWithResidualFilter() throws Exception {
545+
String query = foafCycleAliasQueryWithResidualFilter();
546+
547+
try (FullCodegenFixture fallbackFixture = new FullCodegenFixture(4, false, false, null);
548+
FullCodegenFixture fullFixture = new FullCodegenFixture(4, true, true,
549+
LmdbLftjFullCodegenCompiler.INSTANCE)) {
550+
List<String> fallbackRows = executeQueryRows(fallbackFixture.repository, query);
551+
try (SailRepositoryConnection connection = fullFixture.repository.getConnection()) {
552+
assertThat(connection.prepareTupleQuery(query)
553+
.explain(Explanation.Level.Optimized)
554+
.toString())
555+
.contains("LmdbLftjTupleExpr")
556+
.contains("Filter")
557+
.contains("Projection");
558+
}
559+
List<String> fullRows = executeQueryRows(fullFixture.repository, query);
560+
561+
assertThat(fallbackRows).hasSize(6);
562+
assertThat(fullRows)
563+
.withFailMessage("fallback=%s full=%s", fallbackRows, fullRows)
564+
.containsExactlyElementsOf(fallbackRows);
565+
}
566+
}
567+
543568
@Test
544569
void fullCodegenFoafBenchmarkSequentialQueriesShouldKeepUsingGeneratedFactories() throws Exception {
545570
FoafCliqueQueryBenchmark benchmark = configuredFoafBenchmark();
@@ -976,6 +1001,16 @@ private String foafCycleAliasQuery(int size) {
9761001
return builder.toString();
9771002
}
9781003

1004+
private String foafCycleAliasQueryWithResidualFilter() {
1005+
return "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n"
1006+
+ "SELECT (?a AS ?x) (?b AS ?y) (?c AS ?z) WHERE {\n"
1007+
+ " ?a foaf:knows ?b .\n"
1008+
+ " ?b foaf:knows ?c .\n"
1009+
+ " ?c foaf:knows ?a .\n"
1010+
+ " FILTER (?a != ?b && ?a != ?c && ?b != ?c && STRSTARTS(STR(?a), \"urn:person:1\"))\n"
1011+
+ "}\n";
1012+
}
1013+
9791014
private List<String> executeQueryRows(SailRepository repository, String query) {
9801015
try (SailRepositoryConnection connection = repository.getConnection()) {
9811016
List<String> rows = new ArrayList<>();
@@ -1235,14 +1270,19 @@ private static final class FullCodegenFixture implements AutoCloseable {
12351270
private final File dataDir;
12361271

12371272
private FullCodegenFixture() throws IOException {
1238-
this(4, LmdbLftjFullCodegenCompiler.INSTANCE);
1273+
this(4, true, true, LmdbLftjFullCodegenCompiler.INSTANCE);
12391274
}
12401275

12411276
private FullCodegenFixture(int personCount, LmdbLftjCodegenCompiler compiler) throws IOException {
1277+
this(personCount, true, true, compiler);
1278+
}
1279+
1280+
private FullCodegenFixture(int personCount, boolean lftjEnabled, boolean lftjCodegenEnabled,
1281+
LmdbLftjCodegenCompiler compiler) throws IOException {
12421282
dataDir = Files.createTempDirectory("rdf4j-lmdb-full-codegen-test").toFile();
12431283
LmdbStoreConfig config = new LmdbStoreConfig("spoc,sopc,psoc,posc,ospc,opsc");
1244-
config.setLftjEnabled(true);
1245-
config.setLftjCodegenEnabled(true);
1284+
config.setLftjEnabled(lftjEnabled);
1285+
config.setLftjCodegenEnabled(lftjCodegenEnabled);
12461286
config.setForceSync(false);
12471287
config.setValueDBSize(64L * 1024 * 1024);
12481288
config.setTripleDBSize(config.getValueDBSize());

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ void chainedAliasExtensionRowsShouldMatchRegularEvaluation(@TempDir java.nio.fil
6363
assertRowsMatch(tempDir, chainedAliasExtensionQuery());
6464
}
6565

66+
@Test
67+
void aliasedProjectionWithResidualFilterRowsShouldMatchRegularEvaluation(@TempDir java.nio.file.Path tempDir) {
68+
assertRowsMatch(tempDir, aliasedProjectionWithResidualFilterQuery());
69+
}
70+
6671
@Test
6772
void aliasedCycleRowsShouldMatchRegularEvaluationWithBoundSourceVariables(@TempDir java.nio.file.Path tempDir) {
6873
assertEquals(List.of(
@@ -257,4 +262,14 @@ private String chainedAliasExtensionQuery() {
257262
+ " BIND(?x AS ?y)\n"
258263
+ "}\n";
259264
}
265+
266+
private String aliasedProjectionWithResidualFilterQuery() {
267+
return "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n"
268+
+ "SELECT (?a AS ?x) (?b AS ?y) (?c AS ?z) WHERE {\n"
269+
+ " ?a foaf:knows ?b .\n"
270+
+ " ?b foaf:knows ?c .\n"
271+
+ " ?c foaf:knows ?a .\n"
272+
+ " FILTER (?a != ?b && ?a != ?c && ?b != ?c && STRSTARTS(STR(?a), \"urn:person:1\"))\n"
273+
+ "}\n";
274+
}
260275
}

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,30 @@ void optimizeShouldFuseParsedAliasProjectionQuery() throws Exception {
149149
new LmdbLftjPlan.InequalityConstraint("b", "c")), lftj.plan().inequalityConstraints());
150150
}
151151

152+
@Test
153+
void optimizeShouldExposeSourceBindingsWhenResidualFilterKeepsProjectionOutside() throws Exception {
154+
TestQueryAccess queryAccess = new TestQueryAccess();
155+
LmdbLftjOptimizer optimizer = new LmdbLftjOptimizer(
156+
new LmdbLftjTripleSource(new EmptyTripleSource(), queryAccess));
157+
158+
TupleExpr tupleExpr = parsedQueryRoot(aliasProjectionWithResidualFilterQuery());
159+
160+
optimizer.optimize(tupleExpr, (Dataset) null, EmptyBindingSet.getInstance());
161+
162+
LmdbLftjTupleExpr lftj = assertInstanceOf(LmdbLftjTupleExpr.class,
163+
findNode(tupleExpr, LmdbLftjTupleExpr.class));
164+
assertEquals(List.of(
165+
new LmdbLftjPlan.OutputBinding("a", "a"),
166+
new LmdbLftjPlan.OutputBinding("b", "b"),
167+
new LmdbLftjPlan.OutputBinding("c", "c")), lftj.plan().outputBindings());
168+
assertEquals(List.of(
169+
new LmdbLftjPlan.InequalityConstraint("a", "b"),
170+
new LmdbLftjPlan.InequalityConstraint("a", "c"),
171+
new LmdbLftjPlan.InequalityConstraint("b", "c")), lftj.plan().inequalityConstraints());
172+
assertInstanceOf(Filter.class, findNode(tupleExpr, Filter.class));
173+
assertInstanceOf(Projection.class, findNode(tupleExpr, Projection.class));
174+
}
175+
152176
@Test
153177
void optimizerPipelineShouldFuseParsedAliasProjectionQuery() throws Exception {
154178
TestQueryAccess queryAccess = new TestQueryAccess();
@@ -408,6 +432,16 @@ private String aliasProjectionQuery() {
408432
+ "}\n";
409433
}
410434

435+
private String aliasProjectionWithResidualFilterQuery() {
436+
return "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n"
437+
+ "SELECT (?a AS ?x) (?b AS ?y) (?c AS ?z) WHERE {\n"
438+
+ " ?a foaf:knows ?b .\n"
439+
+ " ?b foaf:knows ?c .\n"
440+
+ " ?c foaf:knows ?a .\n"
441+
+ " FILTER (?a != ?b && ?a != ?c && ?b != ?c && STRSTARTS(STR(?a), \"urn:person:1\"))\n"
442+
+ "}\n";
443+
}
444+
411445
private String reorderedDuplicateAliasProjectionQuery() {
412446
return "PREFIX foaf: <http://xmlns.com/foaf/0.1/>\n"
413447
+ "SELECT (?c AS ?z) (?a AS ?x) (?a AS ?x2) WHERE {\n"

0 commit comments

Comments
 (0)