From 73b021175bbc97cae140d06b7986e5291bc1a497 Mon Sep 17 00:00:00 2001 From: "AD\\aabdoun" Date: Tue, 29 Sep 2026 15:16:10 +0200 Subject: [PATCH] fix(sparql): apply AST simplification for nested OPTIONAL group filter scoping (#613) WhereCompiler.compileGroup() is restored to its original form; the transformation now lives entirely at the AST layer. --- .../impl/sparql/bridge/AstSimplifier.java | 88 +++++++++++++++++++ .../impl/sparql/bridge/WhereCompiler.java | 16 ++-- .../NextSparqlPipelineScopeExecutorTest.java | 59 +++++++++++++ 3 files changed, 156 insertions(+), 7 deletions(-) create mode 100644 src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstSimplifier.java diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstSimplifier.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstSimplifier.java new file mode 100644 index 000000000..cfaa5b07d --- /dev/null +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/AstSimplifier.java @@ -0,0 +1,88 @@ +package fr.inria.corese.core.next.query.impl.sparql.bridge; + +import fr.inria.corese.core.next.query.impl.sparql.ast.GraphAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.GroupGraphPatternAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.MinusAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.OptionalAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.PatternAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.ServiceAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.UnionAst; + +import java.util.ArrayList; +import java.util.List; + +/** + * Applies the SPARQL simplified-group transformation on the AST. + * + *

Per SPARQL 1.1 algebra (§18.2.2), a nested {@link GroupGraphPatternAst} + * whose top-level elements contain no structural operators (OPTIONAL, MINUS, + * UNION) is simple and therefore transparent: its contents are merged + * directly into the enclosing group ({@code { { P } } ≡ { P_elements }}).

+ * + *

This transformation is applied before compilation so that FILTERs inside + * such groups are visible as direct children of the compiled pattern and can be + * correctly detected and deferred to merge time by the OPTIONAL evaluation + * machinery ({@code Exp.optional()}).

+ */ +final class AstSimplifier { + + /** + * Returns a new {@link GroupGraphPatternAst} with the first leading simple + * nested group flattened into the enclosing group, applied recursively. + * + *

Only the first element of a group is eligible for flattening. + * A FILTER in a nested group that follows other patterns (e.g. + * {@code { BGP . { FILTER } }}) must remain scoped to its inner group and + * cannot see variables bound by the preceding BGP — this is the W3C + * {@code filter-nested-2} semantics. Flattening only the leading element + * mirrors the {@code body.size() == 0} condition in + * {@link WhereCompiler#compileGroup}.

+ */ + GroupGraphPatternAst simplify(GroupGraphPatternAst group) { + List result = new ArrayList<>(); + boolean canFlattenNext = true; + for (PatternAst pattern : group.patterns()) { + PatternAst simplified = simplifyPattern(pattern); + if (canFlattenNext + && simplified instanceof GroupGraphPatternAst nested + && isSimple(nested)) { + // First leading simple nested group is transparent: inline its elements + result.addAll(nested.patterns()); + } else { + result.add(simplified); + } + canFlattenNext = false; + } + return new GroupGraphPatternAst(result); + } + + private PatternAst simplifyPattern(PatternAst pattern) { + return switch (pattern) { + case GroupGraphPatternAst g -> simplify(g); + case OptionalAst(PatternAst inner) -> + new OptionalAst(inner instanceof GroupGraphPatternAst g ? simplify(g) : inner); + case MinusAst(GroupGraphPatternAst inner) -> new MinusAst(simplify(inner)); + case UnionAst(GroupGraphPatternAst left, GroupGraphPatternAst right) -> + new UnionAst(simplify(left), simplify(right)); + case GraphAst(TermAst name, GroupGraphPatternAst g) -> + new GraphAst(name, simplify(g)); + case ServiceAst(TermAst endpoint, boolean silent, GroupGraphPatternAst g) -> + new ServiceAst(endpoint, silent, simplify(g)); + default -> pattern; + }; + } + + /** + * A group is simple (transparent) when none of its top-level + * elements is a structural operator (OPTIONAL, MINUS, UNION). + */ + private boolean isSimple(GroupGraphPatternAst group) { + for (PatternAst p : group.patterns()) { + if (p instanceof OptionalAst || p instanceof MinusAst || p instanceof UnionAst) { + return false; + } + } + return true; + } +} diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/WhereCompiler.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/WhereCompiler.java index 548f323e9..e137ae9d1 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/WhereCompiler.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/WhereCompiler.java @@ -84,16 +84,17 @@ Set inScopeVariables() { */ public Exp compile(GroupGraphPatternAst where) { Objects.requireNonNull(where, "where"); - if (inScopeVariables.isEmpty()) { - inScopeVariables = Set.copyOf( - new fr.inria.corese.core.next.query.impl.sparql.parser.semantic.support.VariableScopeAnalyzer() - .collectVisibleVariables(where)); - } Exp cached = compiledPatternCache.get(where); if (cached != null) { return cached; } - Exp compiled = compileGroup(where); + GroupGraphPatternAst simplified = new AstSimplifier().simplify(where); + if (inScopeVariables.isEmpty()) { + inScopeVariables = Set.copyOf( + new fr.inria.corese.core.next.query.impl.sparql.parser.semantic.support.VariableScopeAnalyzer() + .collectVisibleVariables(simplified)); + } + Exp compiled = compileGroup(simplified); compiledPatternCache.put(where, compiled); return compiled; } @@ -158,7 +159,8 @@ case MinusAst(GroupGraphPatternAst pattern) -> { body.add(minusExp); } case GroupGraphPatternAst nested -> { - Exp joined = Exp.create(Type.JOIN, body, compile(nested)); + Exp compiledNested = compile(nested); + Exp joined = Exp.create(Type.JOIN, body, compiledNested); body = Exp.create(Type.AND); body.add(joined); } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java index 02b88c1df..b5bc4a9b3 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java @@ -3,11 +3,14 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; import fr.inria.corese.core.next.data.api.term.BNode; +import fr.inria.corese.core.next.data.api.term.IRI; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -59,6 +62,62 @@ void blankNodeStillJoinsTriplesAcrossExistsFilter(String filter) { } } + /** + * dawg-optional-filter-005-simplified (W3C SPARQL 1.0 / SPARQL 1.1 simplified semantics): + * In {@code OPTIONAL { { BGP . FILTER(?outerVar = x) } }}, the inner {@code { }} is + * transparent — it does not create an evaluation-scope barrier for outer variables. + * The FILTER is postponed and evaluated at merge time (when both the left solution + * carrying {@code ?outerVar} and the right solution carrying the BGP bindings are + * available), giving the correct result: only the row where the FILTER passes + * receives the optional binding. + * + *

This is the "simplified" reading chosen by Corese, aligned with the + * SPARQL 1.1 algebra where an inner group graph pattern with no structural + * sub-patterns (OPTIONAL/MINUS/UNION) flattens into its parent.

+ */ + @Test + @DisplayName("FILTER referencing outer variable inside nested OPTIONAL group — simplified semantics (issue #613)") + void filterReferencingOuterVarInNestedOptionalGroupIsSimplified() { + IRI dcTitle = iri("http://purl.org/dc/elements/1.1/title"); + IRI xPrice = iri("http://example.org/ns#price"); + IRI book1 = iri("http://example.org/books#book1"); + IRI book2 = iri("http://example.org/books#book2"); + IRI book3 = iri("http://example.org/books#book3"); + + insert(book1, dcTitle, valueFactory.createLiteral("TITLE 1")); + insert(book1, xPrice, valueFactory.createLiteral(10)); + insert(book2, dcTitle, valueFactory.createLiteral("TITLE 2")); + insert(book2, xPrice, valueFactory.createLiteral(20)); + insert(book3, dcTitle, valueFactory.createLiteral("TITLE 3")); + + try (var result = executor.evaluateTuple(""" + PREFIX dc: + PREFIX x: + SELECT ?title ?price + WHERE { + ?book dc:title ?title . + OPTIONAL { + { ?book x:price ?price . + FILTER (?title = "TITLE 2") . } + } + } + """)) { + var rows = result.stream().toList(); + assertEquals(3, rows.size(), "All three titles must appear"); + // Only TITLE 2 passes the filter — it alone gets a price binding + long withPrice = rows.stream().filter(r -> r.getValue("price") != null).count(); + assertEquals(1, withPrice, "Exactly one row (TITLE 2) should have a price binding"); + var title2Row = rows.stream() + .filter(r -> "TITLE 2".equals(r.getValue("title").stringValue())) + .findFirst().orElseThrow(); + assertEquals("20", title2Row.getValue("price").stringValue()); + rows.stream() + .filter(r -> !"TITLE 2".equals(r.getValue("title").stringValue())) + .forEach(r -> assertNull(r.getValue("price"), + "Titles other than TITLE 2 must not bind ?price")); + } + } + @Test void constructLabelCreatesAFreshNodeInsteadOfReusingTheWhereBinding() { try (var result = executor.evaluateGraph("""