From a205d24ed4f407a042bc05ff8b5cca818c823d5e Mon Sep 17 00:00:00 2001 From: "AD\\aabdoun" Date: Tue, 22 Sep 2026 14:32:55 +0200 Subject: [PATCH 1/3] [Query] Fix SPARQL blank node BGP scope validation and xsd:double MIN/MAX normalization --- .../core/next/data/api/term/SimpleDouble.java | 4 +- .../impl/sparql/parser/SparqlAstBuilder.java | 56 ++++++++++++++++++- .../listener/SelectQueryAstListener.java | 14 ++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java index dda5b02d8..5c2793da3 100644 --- a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java +++ b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java @@ -41,7 +41,7 @@ public SimpleDouble(String lexicalValue, IRI datatype) { public SimpleDouble(String lexicalValue, IRI datatype, CoreDatatype coreDatatype) { super(datatype == null ? XSDDatatype.DOUBLE.getIRI() : datatype); this.label = Objects.requireNonNull(lexicalValue, "lexicalValue"); - this.value = parseXsdDouble(lexicalValue); + this.value = parseXsdDouble(this.label); CoreDatatype resolved = coreDatatype != null ? coreDatatype : CoreDatatypes.from(this.datatype); this.coreDatatype = (resolved == XSDDatatype.FLOAT || resolved == XSDDatatype.DOUBLE) ? resolved : XSDDatatype.DOUBLE; } @@ -55,7 +55,7 @@ private static double parseXsdDouble(String s) { }; } - @Override +@Override public String getLabel() { return label; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java index 4a88ab9c2..eaf76949d 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java @@ -66,6 +66,28 @@ public abstract class SparqlAstBuilder { */ private final Set blankNodeLabels = new HashSet<>(); + /** + * Monotonically increasing counter used to assign unique BGP scope IDs. + * Incremented at each {@link #enterGroup()} and after each BGP-breaking + * pattern (OPTIONAL, UNION, GRAPH, SERVICE, MINUS, or nested plain group) closes. + */ + private int bgpScopeCounter = 0; + + /** + * Parallel to {@link #groupStack}: the current BGP scope ID for the group at the + * corresponding depth. Pushed in {@link #enterGroup()}, popped in {@link #exitGroup()}, + * and renewed (via {@link #renewBgpScope()}) whenever a BGP-breaking inner pattern + * closes inside the current group. + *

FILTER does NOT renew the scope; all other sub-patterns do. + */ + private final Deque bgpScopeIdStack = new ArrayDeque<>(); + + /** + * Maps each labeled blank node to the BGP scope ID in which it was first encountered. + * A blank node label must not appear in more than one basic graph pattern. + */ + private final Map blankNodeLabelToBgpScope = new HashMap<>(); + /** * Stack of currently open SELECT operations (top-level SELECT and nested SELECT subqueries). */ @@ -345,6 +367,7 @@ public void enterWhereClause() { */ public void enterGroup() { groupStack.push(new ArrayList<>()); + bgpScopeIdStack.push(++bgpScopeCounter); if (selectWhereGroupFollows && hasCurrentSelect()) { selectWhereGroupDepths.push(groupStack.size()); selectWhereGroupFollows = false; @@ -369,11 +392,13 @@ && hasCurrentSelect() && depthBeforePop == selectWhereGroupDepths.peek()) { selectWhereGroupDepths.pop(); List popped = groupStack.pop(); + bgpScopeIdStack.pop(); getCurrentSelectFrame().whereClause = new GroupGraphPatternAst(popped); return; } List popped = groupStack.pop(); + bgpScopeIdStack.pop(); GroupGraphPatternAst group = new GroupGraphPatternAst(popped); appendClosedGroup(group); } @@ -382,23 +407,41 @@ private void appendClosedGroup(GroupGraphPatternAst group) { if (!optionalGroupDepths.isEmpty() && groupStack.size() == optionalGroupDepths.peek()) { optionalGroupDepths.pop(); currentGroup().add(new OptionalAst(group)); + renewBgpScope(); } else if (!minusGroupDepths.isEmpty() && groupStack.size() == minusGroupDepths.peek()) { minusGroupDepths.pop(); currentGroup().add(new MinusAst(group)); + renewBgpScope(); } else if (!existsGroupDepths.isEmpty() && groupStack.size() == existsGroupDepths.peek()) { existsGroupDepths.pop(); capturedExistsStack.push(group); + renewBgpScope(); } else if (!serviceStack.isEmpty() && groupStack.size() == serviceStack.peek().groupDepth()) { ServiceEntry entry = serviceStack.pop(); currentGroup().add(new ServiceAst(entry.endpoint(), entry.silent(), group)); + renewBgpScope(); } else if (!graphStack.isEmpty() && groupStack.size() == graphStack.peek().groupDepth()) { GraphEntry entry = graphStack.pop(); currentGroup().add(new GraphAst(entry.name(), group)); + renewBgpScope(); } else if (groupStack.isEmpty()) { if (hasCurrentSelect()) getCurrentSelectFrame().whereClause = group; else whereClause = group; } else { currentGroup().add(group); + renewBgpScope(); + } + } + + /** + * Renews the BGP scope ID for the current (outer) group after a BGP-breaking pattern + * (OPTIONAL, UNION branch, GRAPH, SERVICE, MINUS, EXISTS, or plain nested group) has closed. + * Any subsequent triple patterns in the outer group belong to a fresh BGP. + */ + private void renewBgpScope() { + if (!bgpScopeIdStack.isEmpty()) { + bgpScopeIdStack.pop(); + bgpScopeIdStack.push(++bgpScopeCounter); } } @@ -655,7 +698,18 @@ public TermAst termFromBlankNode(SparqlParser.BlankNodeContext ctx) { return newAnonymousBlankNode(); } String label = ctx.getText(); - blankNodeLabels.add(label); + int currentScope = bgpScopeIdStack.isEmpty() ? 0 : bgpScopeIdStack.peek(); + Integer registeredScope = blankNodeLabelToBgpScope.get(label); + if (registeredScope != null) { + if (!registeredScope.equals(currentScope)) { + throw new QuerySyntaxException( + "Blank node label '" + label + "' is used in more than one basic graph pattern, " + + "which is not permitted by the SPARQL specification."); + } + } else { + blankNodeLabelToBgpScope.put(label, currentScope); + blankNodeLabels.add(label); + } return this.iri(label); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java index ed4f3a7ed..fcd84df76 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java @@ -1,6 +1,7 @@ package fr.inria.corese.core.next.query.impl.sparql.parser.listener; import fr.inria.corese.core.next.generated.antlr.SparqlParser; +import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlAstBuilder; import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlQueryAstBuilder; import fr.inria.corese.core.next.query.impl.sparql.parser.semantic.support.VariableScopeAnalyzer; @@ -8,6 +9,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -84,6 +86,7 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { return; } List allVars = new ArrayList<>(); + Set seenVars = new LinkedHashSet<>(); List expressionBoundVars = new ArrayList<>(); Map expressionTerms = new LinkedHashMap<>(); Map> expressionReferencedVariables = new LinkedHashMap<>(); @@ -91,13 +94,22 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { if (selectVar.expression() != null) { // (expr AS ?var) — introduces a new variable, not projected from WHERE String varName = selectVar.var_().getText(); + if (!seenVars.add(varName)) { + throw new QuerySyntaxException( + "Variable '" + varName + "' appears more than once in the SELECT clause."); + } allVars.add(varName); expressionBoundVars.add(varName); TermAst expressionAst = builder().termFromExpression(selectVar.expression()); expressionTerms.put(varName, expressionAst); expressionReferencedVariables.put(varName, variableScopeAnalyzer.collectReferencedVariables(expressionAst)); } else if (selectVar.var_() != null) { - allVars.add(selectVar.var_().getText()); + String varName = selectVar.var_().getText(); + if (!seenVars.add(varName)) { + throw new QuerySyntaxException( + "Variable '" + varName + "' appears more than once in the SELECT clause."); + } + allVars.add(varName); } } queryBuilder().setProjectionVariables(allVars, expressionBoundVars, expressionTerms, expressionReferencedVariables); From 290033d6eb31371d947282b47e76a120947f4d67 Mon Sep 17 00:00:00 2001 From: "AD\\aabdoun" Date: Tue, 22 Sep 2026 14:32:55 +0200 Subject: [PATCH 2/3] 607 [Query] Fix SPARQL blank node BGP scope validation and xsd:double MIN/MAX normalization --- .../core/next/data/api/term/SimpleDouble.java | 4 +- .../impl/sparql/parser/SparqlAstBuilder.java | 56 ++++++++++++++++++- .../listener/SelectQueryAstListener.java | 14 ++++- 3 files changed, 70 insertions(+), 4 deletions(-) diff --git a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java index dda5b02d8..5c2793da3 100644 --- a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java +++ b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java @@ -41,7 +41,7 @@ public SimpleDouble(String lexicalValue, IRI datatype) { public SimpleDouble(String lexicalValue, IRI datatype, CoreDatatype coreDatatype) { super(datatype == null ? XSDDatatype.DOUBLE.getIRI() : datatype); this.label = Objects.requireNonNull(lexicalValue, "lexicalValue"); - this.value = parseXsdDouble(lexicalValue); + this.value = parseXsdDouble(this.label); CoreDatatype resolved = coreDatatype != null ? coreDatatype : CoreDatatypes.from(this.datatype); this.coreDatatype = (resolved == XSDDatatype.FLOAT || resolved == XSDDatatype.DOUBLE) ? resolved : XSDDatatype.DOUBLE; } @@ -55,7 +55,7 @@ private static double parseXsdDouble(String s) { }; } - @Override +@Override public String getLabel() { return label; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java index 4a88ab9c2..eaf76949d 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java @@ -66,6 +66,28 @@ public abstract class SparqlAstBuilder { */ private final Set blankNodeLabels = new HashSet<>(); + /** + * Monotonically increasing counter used to assign unique BGP scope IDs. + * Incremented at each {@link #enterGroup()} and after each BGP-breaking + * pattern (OPTIONAL, UNION, GRAPH, SERVICE, MINUS, or nested plain group) closes. + */ + private int bgpScopeCounter = 0; + + /** + * Parallel to {@link #groupStack}: the current BGP scope ID for the group at the + * corresponding depth. Pushed in {@link #enterGroup()}, popped in {@link #exitGroup()}, + * and renewed (via {@link #renewBgpScope()}) whenever a BGP-breaking inner pattern + * closes inside the current group. + *

FILTER does NOT renew the scope; all other sub-patterns do. + */ + private final Deque bgpScopeIdStack = new ArrayDeque<>(); + + /** + * Maps each labeled blank node to the BGP scope ID in which it was first encountered. + * A blank node label must not appear in more than one basic graph pattern. + */ + private final Map blankNodeLabelToBgpScope = new HashMap<>(); + /** * Stack of currently open SELECT operations (top-level SELECT and nested SELECT subqueries). */ @@ -345,6 +367,7 @@ public void enterWhereClause() { */ public void enterGroup() { groupStack.push(new ArrayList<>()); + bgpScopeIdStack.push(++bgpScopeCounter); if (selectWhereGroupFollows && hasCurrentSelect()) { selectWhereGroupDepths.push(groupStack.size()); selectWhereGroupFollows = false; @@ -369,11 +392,13 @@ && hasCurrentSelect() && depthBeforePop == selectWhereGroupDepths.peek()) { selectWhereGroupDepths.pop(); List popped = groupStack.pop(); + bgpScopeIdStack.pop(); getCurrentSelectFrame().whereClause = new GroupGraphPatternAst(popped); return; } List popped = groupStack.pop(); + bgpScopeIdStack.pop(); GroupGraphPatternAst group = new GroupGraphPatternAst(popped); appendClosedGroup(group); } @@ -382,23 +407,41 @@ private void appendClosedGroup(GroupGraphPatternAst group) { if (!optionalGroupDepths.isEmpty() && groupStack.size() == optionalGroupDepths.peek()) { optionalGroupDepths.pop(); currentGroup().add(new OptionalAst(group)); + renewBgpScope(); } else if (!minusGroupDepths.isEmpty() && groupStack.size() == minusGroupDepths.peek()) { minusGroupDepths.pop(); currentGroup().add(new MinusAst(group)); + renewBgpScope(); } else if (!existsGroupDepths.isEmpty() && groupStack.size() == existsGroupDepths.peek()) { existsGroupDepths.pop(); capturedExistsStack.push(group); + renewBgpScope(); } else if (!serviceStack.isEmpty() && groupStack.size() == serviceStack.peek().groupDepth()) { ServiceEntry entry = serviceStack.pop(); currentGroup().add(new ServiceAst(entry.endpoint(), entry.silent(), group)); + renewBgpScope(); } else if (!graphStack.isEmpty() && groupStack.size() == graphStack.peek().groupDepth()) { GraphEntry entry = graphStack.pop(); currentGroup().add(new GraphAst(entry.name(), group)); + renewBgpScope(); } else if (groupStack.isEmpty()) { if (hasCurrentSelect()) getCurrentSelectFrame().whereClause = group; else whereClause = group; } else { currentGroup().add(group); + renewBgpScope(); + } + } + + /** + * Renews the BGP scope ID for the current (outer) group after a BGP-breaking pattern + * (OPTIONAL, UNION branch, GRAPH, SERVICE, MINUS, EXISTS, or plain nested group) has closed. + * Any subsequent triple patterns in the outer group belong to a fresh BGP. + */ + private void renewBgpScope() { + if (!bgpScopeIdStack.isEmpty()) { + bgpScopeIdStack.pop(); + bgpScopeIdStack.push(++bgpScopeCounter); } } @@ -655,7 +698,18 @@ public TermAst termFromBlankNode(SparqlParser.BlankNodeContext ctx) { return newAnonymousBlankNode(); } String label = ctx.getText(); - blankNodeLabels.add(label); + int currentScope = bgpScopeIdStack.isEmpty() ? 0 : bgpScopeIdStack.peek(); + Integer registeredScope = blankNodeLabelToBgpScope.get(label); + if (registeredScope != null) { + if (!registeredScope.equals(currentScope)) { + throw new QuerySyntaxException( + "Blank node label '" + label + "' is used in more than one basic graph pattern, " + + "which is not permitted by the SPARQL specification."); + } + } else { + blankNodeLabelToBgpScope.put(label, currentScope); + blankNodeLabels.add(label); + } return this.iri(label); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java index ed4f3a7ed..fcd84df76 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java @@ -1,6 +1,7 @@ package fr.inria.corese.core.next.query.impl.sparql.parser.listener; import fr.inria.corese.core.next.generated.antlr.SparqlParser; +import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlAstBuilder; import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlQueryAstBuilder; import fr.inria.corese.core.next.query.impl.sparql.parser.semantic.support.VariableScopeAnalyzer; @@ -8,6 +9,7 @@ import java.util.ArrayList; import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -84,6 +86,7 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { return; } List allVars = new ArrayList<>(); + Set seenVars = new LinkedHashSet<>(); List expressionBoundVars = new ArrayList<>(); Map expressionTerms = new LinkedHashMap<>(); Map> expressionReferencedVariables = new LinkedHashMap<>(); @@ -91,13 +94,22 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { if (selectVar.expression() != null) { // (expr AS ?var) — introduces a new variable, not projected from WHERE String varName = selectVar.var_().getText(); + if (!seenVars.add(varName)) { + throw new QuerySyntaxException( + "Variable '" + varName + "' appears more than once in the SELECT clause."); + } allVars.add(varName); expressionBoundVars.add(varName); TermAst expressionAst = builder().termFromExpression(selectVar.expression()); expressionTerms.put(varName, expressionAst); expressionReferencedVariables.put(varName, variableScopeAnalyzer.collectReferencedVariables(expressionAst)); } else if (selectVar.var_() != null) { - allVars.add(selectVar.var_().getText()); + String varName = selectVar.var_().getText(); + if (!seenVars.add(varName)) { + throw new QuerySyntaxException( + "Variable '" + varName + "' appears more than once in the SELECT clause."); + } + allVars.add(varName); } } queryBuilder().setProjectionVariables(allVars, expressionBoundVars, expressionTerms, expressionReferencedVariables); From dd8e830e9530f0b23dcf4e4c8c83a158463aa11a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= Date: Thu, 24 Sep 2026 11:34:39 +0200 Subject: [PATCH 3/3] fix(parser): enforce BGP blank node scoping and SELECT projection deduplication (#607) --- .../core/next/data/api/term/SimpleDouble.java | 4 +- .../impl/sparql/parser/SparqlAstBuilder.java | 57 +++++----- .../listener/SelectQueryAstListener.java | 23 ++-- .../NextSparqlPipelineScopeExecutorTest.java | 76 +++++++++++++ .../SparqlParserBlankNodeScopeTest.java | 101 ++++++++++++++++++ .../SparqlParserProjectionScopeTest.java | 64 +++++++++++ 6 files changed, 277 insertions(+), 48 deletions(-) create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserBlankNodeScopeTest.java create mode 100644 src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserProjectionScopeTest.java diff --git a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java index 5c2793da3..dda5b02d8 100644 --- a/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java +++ b/src/main/java/fr/inria/corese/core/next/data/api/term/SimpleDouble.java @@ -41,7 +41,7 @@ public SimpleDouble(String lexicalValue, IRI datatype) { public SimpleDouble(String lexicalValue, IRI datatype, CoreDatatype coreDatatype) { super(datatype == null ? XSDDatatype.DOUBLE.getIRI() : datatype); this.label = Objects.requireNonNull(lexicalValue, "lexicalValue"); - this.value = parseXsdDouble(this.label); + this.value = parseXsdDouble(lexicalValue); CoreDatatype resolved = coreDatatype != null ? coreDatatype : CoreDatatypes.from(this.datatype); this.coreDatatype = (resolved == XSDDatatype.FLOAT || resolved == XSDDatatype.DOUBLE) ? resolved : XSDDatatype.DOUBLE; } @@ -55,7 +55,7 @@ private static double parseXsdDouble(String s) { }; } -@Override + @Override public String getLabel() { return label; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java index eaf76949d..d555a0ad6 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlAstBuilder.java @@ -66,25 +66,19 @@ public abstract class SparqlAstBuilder { */ private final Set blankNodeLabels = new HashSet<>(); - /** - * Monotonically increasing counter used to assign unique BGP scope IDs. - * Incremented at each {@link #enterGroup()} and after each BGP-breaking - * pattern (OPTIONAL, UNION, GRAPH, SERVICE, MINUS, or nested plain group) closes. - */ - private int bgpScopeCounter = 0; + /** Unique IDs for basic graph patterns, including those in nested groups. */ + private int bgpScopeCounter; /** - * Parallel to {@link #groupStack}: the current BGP scope ID for the group at the - * corresponding depth. Pushed in {@link #enterGroup()}, popped in {@link #exitGroup()}, - * and renewed (via {@link #renewBgpScope()}) whenever a BGP-breaking inner pattern - * closes inside the current group. - *

FILTER does NOT renew the scope; all other sub-patterns do. + * Current BGP scope at each group depth. FILTER preserves the enclosing scope, + * including when its expression contains EXISTS or NOT EXISTS. Other graph + * patterns, BIND, and inline VALUES end the preceding BGP. */ private final Deque bgpScopeIdStack = new ArrayDeque<>(); /** - * Maps each labeled blank node to the BGP scope ID in which it was first encountered. - * A blank node label must not appear in more than one basic graph pattern. + * First BGP using each explicit label. Shared across subqueries and update + * WHERE clauses, but excludes CONSTRUCT and update templates. */ private final Map blankNodeLabelToBgpScope = new HashMap<>(); @@ -407,37 +401,31 @@ private void appendClosedGroup(GroupGraphPatternAst group) { if (!optionalGroupDepths.isEmpty() && groupStack.size() == optionalGroupDepths.peek()) { optionalGroupDepths.pop(); currentGroup().add(new OptionalAst(group)); - renewBgpScope(); } else if (!minusGroupDepths.isEmpty() && groupStack.size() == minusGroupDepths.peek()) { minusGroupDepths.pop(); currentGroup().add(new MinusAst(group)); - renewBgpScope(); } else if (!existsGroupDepths.isEmpty() && groupStack.size() == existsGroupDepths.peek()) { existsGroupDepths.pop(); capturedExistsStack.push(group); - renewBgpScope(); + // An expression's graph pattern does not end the enclosing BGP. + return; } else if (!serviceStack.isEmpty() && groupStack.size() == serviceStack.peek().groupDepth()) { ServiceEntry entry = serviceStack.pop(); currentGroup().add(new ServiceAst(entry.endpoint(), entry.silent(), group)); - renewBgpScope(); } else if (!graphStack.isEmpty() && groupStack.size() == graphStack.peek().groupDepth()) { GraphEntry entry = graphStack.pop(); currentGroup().add(new GraphAst(entry.name(), group)); - renewBgpScope(); } else if (groupStack.isEmpty()) { if (hasCurrentSelect()) getCurrentSelectFrame().whereClause = group; else whereClause = group; + return; } else { currentGroup().add(group); - renewBgpScope(); } + renewBgpScope(); } - /** - * Renews the BGP scope ID for the current (outer) group after a BGP-breaking pattern - * (OPTIONAL, UNION branch, GRAPH, SERVICE, MINUS, EXISTS, or plain nested group) has closed. - * Any subsequent triple patterns in the outer group belong to a fresh BGP. - */ + /** Starts a fresh BGP after a non-FILTER graph pattern in the current group. */ private void renewBgpScope() { if (!bgpScopeIdStack.isEmpty()) { bgpScopeIdStack.pop(); @@ -501,6 +489,7 @@ public void addInlineValues(ValuesAst values) { throw new IllegalStateException("addInlineValues() called outside of a group graph pattern"); } currentGroup().add(values); + renewBgpScope(); } /** @@ -526,6 +515,7 @@ public void addBind(BindAst bind) { "Variable ?" + name + " used in BIND is already declared in the same group graph pattern"); } group.add(bind); + renewBgpScope(); } // --- Optional --- @@ -698,17 +688,18 @@ public TermAst termFromBlankNode(SparqlParser.BlankNodeContext ctx) { return newAnonymousBlankNode(); } String label = ctx.getText(); - int currentScope = bgpScopeIdStack.isEmpty() ? 0 : bgpScopeIdStack.peek(); - Integer registeredScope = blankNodeLabelToBgpScope.get(label); - if (registeredScope != null) { - if (!registeredScope.equals(currentScope)) { + blankNodeLabels.add(label); + // Templates are built outside graph-pattern groups. Their labels are + // independent of WHERE labels; update-template restrictions are checked + // separately by UpdateTemplateValidator. CONSTRUCT WHERE opens a group + // explicitly, so its abbreviated WHERE still participates in this check. + if (!bgpScopeIdStack.isEmpty()) { + Integer currentScope = bgpScopeIdStack.peek(); + Integer registeredScope = blankNodeLabelToBgpScope.putIfAbsent(label, currentScope); + if (registeredScope != null && !registeredScope.equals(currentScope)) { throw new QuerySyntaxException( - "Blank node label '" + label + "' is used in more than one basic graph pattern, " + - "which is not permitted by the SPARQL specification."); + "Blank node label '" + label + "' is used in more than one basic graph pattern"); } - } else { - blankNodeLabelToBgpScope.put(label, currentScope); - blankNodeLabels.add(label); } return this.iri(label); } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java index fcd84df76..f4476a619 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/SelectQueryAstListener.java @@ -1,5 +1,6 @@ package fr.inria.corese.core.next.query.impl.sparql.parser.listener; +import fr.inria.corese.core.next.common.text.RdfText; import fr.inria.corese.core.next.generated.antlr.SparqlParser; import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlAstBuilder; @@ -85,33 +86,29 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { queryBuilder().setProjectionAll(); return; } - List allVars = new ArrayList<>(); - Set seenVars = new LinkedHashSet<>(); + Set projectedVariables = new LinkedHashSet<>(); List expressionBoundVars = new ArrayList<>(); Map expressionTerms = new LinkedHashMap<>(); Map> expressionReferencedVariables = new LinkedHashMap<>(); for (SparqlParser.SelectVarContext selectVar : ctx.selectVar()) { if (selectVar.expression() != null) { // (expr AS ?var) — introduces a new variable, not projected from WHERE - String varName = selectVar.var_().getText(); - if (!seenVars.add(varName)) { + String varName = RdfText.stripVariableMarker(selectVar.var_().getText()); + if (!projectedVariables.add(varName)) { throw new QuerySyntaxException( - "Variable '" + varName + "' appears more than once in the SELECT clause."); + "Variable ?" + varName + " introduced by SELECT expression is already projected"); } - allVars.add(varName); expressionBoundVars.add(varName); TermAst expressionAst = builder().termFromExpression(selectVar.expression()); expressionTerms.put(varName, expressionAst); expressionReferencedVariables.put(varName, variableScopeAnalyzer.collectReferencedVariables(expressionAst)); } else if (selectVar.var_() != null) { - String varName = selectVar.var_().getText(); - if (!seenVars.add(varName)) { - throw new QuerySyntaxException( - "Variable '" + varName + "' appears more than once in the SELECT clause."); - } - allVars.add(varName); + String varName = RdfText.stripVariableMarker(selectVar.var_().getText()); + // SPARQL 1.1 section 18.2.4.4 accumulates plain variables in a set. + // A later plain reference to an earlier alias is also permitted. + projectedVariables.add(varName); } } - queryBuilder().setProjectionVariables(allVars, expressionBoundVars, expressionTerms, expressionReferencedVariables); + queryBuilder().setProjectionVariables(new ArrayList<>(projectedVariables), expressionBoundVars, expressionTerms, expressionReferencedVariables); } } 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 new file mode 100644 index 000000000..02b88c1df --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/execution/NextSparqlPipelineScopeExecutorTest.java @@ -0,0 +1,76 @@ +package fr.inria.corese.core.next.query.impl.sparql.execution; + +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.assertTrue; + +import java.util.List; + +import fr.inria.corese.core.next.data.api.term.BNode; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** Result-level regressions for blank-node scopes and SELECT projection sets. */ +class NextSparqlPipelineScopeExecutorTest extends PipelineTestSupport { + + @ParameterizedTest + @ValueSource(strings = { "?x ?x", "?x $x", "$x ?x" }) + void repeatedProjectionProducesOneColumn(String projection) { + try (var result = executor.evaluateTuple("SELECT " + projection + " WHERE { VALUES ?x { 1 } }")) { + assertEquals(List.of("x"), result.getBindingNames()); + assertTrue(result.hasNext()); + assertEquals("1", result.next().getValue("x").stringValue()); + assertFalse(result.hasNext()); + } + } + + @Test + void projectedAliasMayBeReferencedAgain() { + try (var result = executor.evaluateTuple("SELECT (1 AS ?x) $x (?x + 1 AS ?y) WHERE {}")) { + assertEquals(List.of("x", "y"), result.getBindingNames()); + assertTrue(result.hasNext()); + var row = result.next(); + assertEquals("1", row.getValue("x").stringValue()); + assertEquals("2", row.getValue("y").stringValue()); + assertFalse(result.hasNext()); + } + } + + @ParameterizedTest + @ValueSource(strings = { + "FILTER EXISTS { ?s ?o }", + "FILTER NOT EXISTS { ?s ?o }" + }) + void blankNodeStillJoinsTriplesAcrossExistsFilter(String filter) { + insert(iri(ALICE), iri(NAME), valueFactory.createLiteral("Alice")); + insert(iri(BOB), iri(NAME), valueFactory.createLiteral("Bob")); + try (var result = executor.evaluateTuple(""" + SELECT ?name WHERE { + _:a ?friend . + %s + _:a ?name + } + """.formatted(filter))) { + assertTrue(result.hasNext()); + assertEquals("Alice", result.next().getValue("name").stringValue()); + assertFalse(result.hasNext(), "The same blank-node label must join on the same subject"); + } + } + + @Test + void constructLabelCreatesAFreshNodeInsteadOfReusingTheWhereBinding() { + try (var result = executor.evaluateGraph(""" + CONSTRUCT { _:a ?friend } + WHERE { _:a ?friend } + """)) { + assertTrue(result.hasNext()); + var triple = result.next(); + assertInstanceOf(BNode.class, triple.getSubject()); + assertEquals(iri("urn:friend"), triple.getPredicate()); + assertEquals(iri(BOB), triple.getObject()); + assertFalse(result.hasNext()); + } + } +} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserBlankNodeScopeTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserBlankNodeScopeTest.java new file mode 100644 index 000000000..0ac8db50d --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserBlankNodeScopeTest.java @@ -0,0 +1,101 @@ +package fr.inria.corese.core.next.query.impl.sparql.parser; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** SPARQL 1.1 sections 5.1.1, 16.2.1, 18.2.2, and 19.6. */ +class SparqlParserBlankNodeScopeTest { + + private final SparqlParser parser = new SparqlParser(); + + @ParameterizedTest + @ValueSource(strings = { + "", + "FILTER (?x != ?y)", + "FILTER EXISTS { ?s ?p ?o }", + "FILTER NOT EXISTS { ?s ?p ?o }", + "FILTER (EXISTS { ?s ?p ?o } && NOT EXISTS { ?t ?q ?r })", + "FILTER EXISTS { _:inner ?v . OPTIONAL { ?s ?p ?o } }", + "FILTER EXISTS { _:inner ?v . FILTER EXISTS { ?s ?p ?o } _:inner ?w }" + }) + void filtersPreserveTheEnclosingBasicGraphPattern(String filter) { + String query = "SELECT * WHERE { _:a ?x . " + filter + " _:a ?y }"; + assertDoesNotThrow(() -> parser.parse(query)); + assertTrue(parser.validate(query).isValid()); + } + + @ParameterizedTest + @ValueSource(strings = { + "{}", + "{ ?s ?p ?o }", + "OPTIONAL { ?s ?p ?o }", + "MINUS { ?s ?p ?o }", + "GRAPH { ?s ?p ?o }", + "SERVICE { ?s ?p ?o }", + "{ ?s ?p ?o } UNION { ?t ?q ?r }", + "{ SELECT ?s WHERE { ?s ?p ?o } }", + "BIND (1 AS ?z)", + "BIND (EXISTS { ?s ?p ?o } AS ?z)", + "VALUES ?z { 1 }" + }) + void nonFilterPatternsEndThePrecedingBasicGraphPattern(String pattern) { + assertInvalidLabelReuse("SELECT * WHERE { _:a ?x . " + pattern + " _:a ?y }"); + } + + @ParameterizedTest + @ValueSource(strings = { + "SELECT * WHERE { _:a ?x . OPTIONAL { _:a ?y } }", + "SELECT * WHERE { _:a ?x . FILTER EXISTS { _:a ?y } }", + "SELECT * WHERE { _:a ?x . FILTER NOT EXISTS { _:a ?y } }", + "SELECT * WHERE { FILTER EXISTS { _:a ?x } _:a ?y }", + "SELECT * WHERE { { _:a ?x } UNION { _:a ?y } }", + "SELECT * WHERE { { SELECT ?x WHERE { _:a ?x } } _:a ?y }", + "SELECT * WHERE { GRAPH { _:a ?x } GRAPH { _:a ?y } }", + "CONSTRUCT { _:a ?x } WHERE { _:a ?x . OPTIONAL { _:a ?y } }", + "INSERT {} WHERE { _:a ?x }; INSERT {} WHERE { _:a ?y }" + }) + void labelsCannotBeSharedByDistinctBasicGraphPatterns(String query) { + assertInvalidLabelReuse(query); + } + + @ParameterizedTest + @ValueSource(strings = { + "CONSTRUCT { _:a ?x . _:a ?x } WHERE { _:a ?x }", + "CONSTRUCT WHERE { _:a ?x . _:a ?y }", + "INSERT { _:a ?x } WHERE { _:a ?x }", + "INSERT { _:a ?x } WHERE { ?s ?x }; INSERT { _:a ?x } WHERE { ?s ?x }", + "INSERT DATA { _:a 1 . GRAPH { _:a 2 } }", + "SELECT * WHERE { [] ?x . OPTIONAL { [] ?y } }", + "SELECT * WHERE { _:a ?x . OPTIONAL { _:b ?y } _:c ?z }" + }) + void templatesAndAnonymousNodesHaveIndependentScopes(String query) { + assertDoesNotThrow(() -> parser.parse(query)); + } + + @Test + void insertDataStillRejectsLabelsReusedAcrossOperations() { + assertThrows(QuerySyntaxException.class, () -> parser.parse( + "INSERT DATA { _:a 1 }; INSERT DATA { _:a 2 }")); + } + + @Test + void scopeStateDoesNotLeakBetweenParserInvocations() { + String query = "SELECT * WHERE { _:a ?x }"; + assertDoesNotThrow(() -> parser.parse(query)); + assertInvalidLabelReuse("SELECT * WHERE { _:a ?x . OPTIONAL { _:a ?y } }"); + assertDoesNotThrow(() -> parser.parse(query)); + } + + private void assertInvalidLabelReuse(String query) { + QuerySyntaxException error = assertThrows(QuerySyntaxException.class, () -> parser.parse(query)); + assertTrue(error.getMessage().contains("Blank node label '_:a'"), error.getMessage()); + assertFalse(parser.validate(query).isValid()); + } +} diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserProjectionScopeTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserProjectionScopeTest.java new file mode 100644 index 000000000..41932a1ee --- /dev/null +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserProjectionScopeTest.java @@ -0,0 +1,64 @@ +package fr.inria.corese.core.next.query.impl.sparql.parser; + +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.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; + +import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; +import fr.inria.corese.core.next.query.impl.sparql.ast.SelectQueryAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.VarAst; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** SPARQL 1.1 sections 4.1.3 and 18.2.4.4: projection is a set, aliases must be new. */ +class SparqlParserProjectionScopeTest { + + private final SparqlParser parser = new SparqlParser(); + + @ParameterizedTest + @ValueSource(strings = { "?x ?y ?x ?y", "?x $x ?y $y", "$x ?y ?x $y" }) + void plainVariablesAreDeduplicatedInFirstOccurrenceOrder(String projection) { + SelectQueryAst ast = assertInstanceOf(SelectQueryAst.class, + parser.parse("SELECT " + projection + " WHERE { ?x ?y }")); + assertEquals(List.of(new VarAst("x"), new VarAst("y")), ast.projection().variables()); + } + + @ParameterizedTest + @ValueSource(strings = { "(1 AS ?x) ?x", "(1 AS $x) ?x $x", "(1 AS ?x) $x" }) + void plainReferencesMayFollowAnAlias(String projection) { + SelectQueryAst ast = assertInstanceOf(SelectQueryAst.class, + parser.parse("SELECT " + projection + " WHERE {}")); + assertEquals(List.of(new VarAst("x")), ast.projection().variables()); + assertTrue(ast.projection().expressionBoundVariables().contains("x")); + assertEquals(1, ast.projection().expressionTerms().size()); + } + + @ParameterizedTest + @ValueSource(strings = { + "?x (1 AS ?x)", "?x (1 AS $x)", "$x (1 AS ?x)", + "(1 AS ?x) (2 AS ?x)", "(1 AS ?x) (2 AS $x)", "(1 AS $x) (2 AS ?x)" + }) + void aliasesCannotRedefineAnEarlierProjectedVariable(String projection) { + String query = "SELECT " + projection + " WHERE {}"; + assertThrows(QuerySyntaxException.class, () -> parser.parse(query)); + assertFalse(parser.validate(query).isValid()); + assertThrows(QuerySyntaxException.class, () -> parser.parse("SELECT * WHERE { { " + query + " } }")); + } + + @Test + void projectionScopesAreIndependentAcrossSubqueries() { + String query = "SELECT ?x $x WHERE { { SELECT ?x $x WHERE { ?x ?p ?o } } }"; + SelectQueryAst ast = assertInstanceOf(SelectQueryAst.class, parser.parse(query)); + assertEquals(List.of(new VarAst("x")), ast.projection().variables()); + } + + @Test + void aliasesStillCannotRedefineVariablesFromWhere() { + assertFalse(parser.validate("SELECT (1 AS $x) WHERE { ?x ?p ?o }").isValid()); + } +}