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..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,6 +66,22 @@ public abstract class SparqlAstBuilder { */ private final Set blankNodeLabels = new HashSet<>(); + /** Unique IDs for basic graph patterns, including those in nested groups. */ + private int bgpScopeCounter; + + /** + * 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<>(); + + /** + * 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<>(); + /** * Stack of currently open SELECT operations (top-level SELECT and nested SELECT subqueries). */ @@ -345,6 +361,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 +386,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); } @@ -388,6 +407,8 @@ private void appendClosedGroup(GroupGraphPatternAst group) { } else if (!existsGroupDepths.isEmpty() && groupStack.size() == existsGroupDepths.peek()) { existsGroupDepths.pop(); capturedExistsStack.push(group); + // 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)); @@ -397,9 +418,19 @@ private void appendClosedGroup(GroupGraphPatternAst group) { } else if (groupStack.isEmpty()) { if (hasCurrentSelect()) getCurrentSelectFrame().whereClause = group; else whereClause = group; + return; } else { currentGroup().add(group); } + renewBgpScope(); + } + + /** Starts a fresh BGP after a non-FILTER graph pattern in the current group. */ + private void renewBgpScope() { + if (!bgpScopeIdStack.isEmpty()) { + bgpScopeIdStack.pop(); + bgpScopeIdStack.push(++bgpScopeCounter); + } } /** @@ -458,6 +489,7 @@ public void addInlineValues(ValuesAst values) { throw new IllegalStateException("addInlineValues() called outside of a group graph pattern"); } currentGroup().add(values); + renewBgpScope(); } /** @@ -483,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 --- @@ -656,6 +689,18 @@ public TermAst termFromBlankNode(SparqlParser.BlankNodeContext ctx) { } String label = ctx.getText(); 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"); + } + } 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..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,6 +1,8 @@ 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; 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 +10,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; @@ -83,23 +86,29 @@ private void extractProjection(SparqlParser.SelectClauseContext ctx) { queryBuilder().setProjectionAll(); return; } - List allVars = new ArrayList<>(); + 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(); - allVars.add(varName); + String varName = RdfText.stripVariableMarker(selectVar.var_().getText()); + if (!projectedVariables.add(varName)) { + throw new QuerySyntaxException( + "Variable ?" + varName + " introduced by SELECT expression is already projected"); + } 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 = 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()); + } +}