diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/query/CoreseUpdate.java b/src/main/java/fr/inria/corese/core/next/query/impl/query/CoreseUpdate.java index 411572caa..39705a50a 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/query/CoreseUpdate.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/query/CoreseUpdate.java @@ -1,6 +1,8 @@ package fr.inria.corese.core.next.query.impl.query; +import fr.inria.corese.core.next.data.spi.io.IOConstants; import fr.inria.corese.core.next.data.api.term.IRI; +import fr.inria.corese.core.next.data.api.term.BNode; import fr.inria.corese.core.next.data.api.term.Resource; import fr.inria.corese.core.next.data.api.model.Statement; import fr.inria.corese.core.next.data.api.term.Value; @@ -15,15 +17,18 @@ import fr.inria.corese.core.next.query.impl.sparql.ast.LiteralAst; import fr.inria.corese.core.next.query.impl.sparql.ast.NamedGraphQuadsAst; import fr.inria.corese.core.next.query.impl.sparql.ast.QuadsAst; +import fr.inria.corese.core.next.query.impl.sparql.ast.QueryPrologueAst; import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst; import fr.inria.corese.core.next.query.impl.sparql.ast.TriplePatternAst; import fr.inria.corese.core.next.query.impl.sparql.ast.UpdateRequestAst; import fr.inria.corese.core.next.query.impl.sparql.ast.UpdateRequestUnitAst; import fr.inria.corese.core.next.query.impl.sparql.ast.path.PredicatePathAst; +import fr.inria.corese.core.next.query.impl.sparql.bridge.SparqlAstToExpression; import fr.inria.corese.core.next.storage.api.StorageManager; import fr.inria.corese.core.next.storage.api.operations.MutationOperations; -import fr.inria.corese.core.next.common.text.RdfText; +import java.util.HashMap; +import java.util.Map; import java.util.Objects; /** @@ -60,8 +65,8 @@ public void execute() throws QueryEvaluationException { for (UpdateRequestUnitAst operation : request.operations()) { switch (operation) { - case InsertDataRequestAst(QuadsAst data) -> applyQuads(data, mutations, factory, true); - case DeleteDataRequestAst(QuadsAst data) -> applyQuads(data, mutations, factory, false); + case InsertDataRequestAst(QuadsAst data) -> applyQuads(data, mutations, factory, request.prologue(), true); + case DeleteDataRequestAst(QuadsAst data) -> applyQuads(data, mutations, factory, request.prologue(), false); default -> throw new UnsupportedQueryFeatureException( "SPARQL UPDATE operation not yet supported: " + operation.getClass().getSimpleName()); @@ -74,9 +79,10 @@ public void execute() throws QueryEvaluationException { // ------------------------------------------------------------------------- private void applyQuads(QuadsAst quads, MutationOperations mutations, - CoreseValueFactory factory, boolean insert) { + CoreseValueFactory factory, QueryPrologueAst prologue, boolean insert) { + Map blankNodes = new HashMap<>(); for (TriplePatternAst triple : quads.defaultTriples()) { - Statement stmt = toStatement(triple, null, factory); + Statement stmt = toStatement(triple, null, factory, prologue, blankNodes, insert); if (insert) { mutations.add(stmt); } else { @@ -84,9 +90,9 @@ private void applyQuads(QuadsAst quads, MutationOperations mutations, } } for (NamedGraphQuadsAst block : quads.namedGraphBlocks()) { - Resource context = (Resource) termToValue(block.graph(), factory); + Resource context = (Resource) termToValue(block.graph(), factory, prologue, blankNodes, insert); for (TriplePatternAst triple : block.triples()) { - Statement stmt = toStatement(triple, context, factory); + Statement stmt = toStatement(triple, context, factory, prologue, blankNodes, insert); if (insert) { mutations.add(stmt); } else { @@ -96,16 +102,18 @@ private void applyQuads(QuadsAst quads, MutationOperations mutations, } } - private Statement toStatement(TriplePatternAst triple, Resource context, CoreseValueFactory factory) { - Value subject = termToValue(triple.subject(), factory); - Value object = termToValue(triple.object(), factory); + private Statement toStatement(TriplePatternAst triple, Resource context, + CoreseValueFactory factory, QueryPrologueAst prologue, + Map blankNodes, boolean insert) { + Value subject = termToValue(triple.subject(), factory, prologue, blankNodes, insert); + Value object = termToValue(triple.object(), factory, prologue, blankNodes, insert); // Resolve predicate — INSERT/DELETE DATA only allows simple predicate IRIs if (!(triple.predicate() instanceof PredicatePathAst(TermAst pp))) { throw new UnsupportedQueryFeatureException( "Property paths are not allowed in INSERT/DELETE DATA"); } - Value predicate = termToValue(pp, factory); + Value predicate = termToValue(pp, factory, prologue, blankNodes, insert); if (!(subject instanceof Resource s)) { throw new QueryEvaluationException("UPDATE subject must be a Resource, got: " + subject); @@ -119,17 +127,33 @@ private Statement toStatement(TriplePatternAst triple, Resource context, CoreseV return factory.createStatement(s, p, object); } - private Value termToValue(TermAst term, CoreseValueFactory factory) { + private Value termToValue( + TermAst term, + CoreseValueFactory factory, + QueryPrologueAst prologue, + Map blankNodes, + boolean insert) { return switch (term) { - case IriAst(String raw) -> factory.createIRI(RdfText.stripAngleBrackets(raw)); - case LiteralAst(String lexical, String datatype, String lang) -> { + case IriAst(String raw) -> { + String resolved = SparqlAstToExpression.resolveIri(raw, prologue); + if (resolved != null && resolved.startsWith(IOConstants.BLANK_NODE_PREFIX)) { + if (!insert) { + throw new QueryEvaluationException("Blank nodes are not allowed in DELETE DATA"); + } + yield blankNodes.computeIfAbsent(resolved, ignored -> factory.createBNode()); + } + yield factory.createIRI(resolved); + } + case LiteralAst(String lexical, String lang, String datatype) -> { + String clean = SparqlAstToExpression.unquoteLexical(lexical); if (lang != null && !lang.isBlank()) { - yield factory.createLiteral(lexical, lang); + yield factory.createLiteral(clean, lang); } if (datatype != null && !datatype.isBlank()) { - yield factory.createLiteral(lexical, factory.createIRI(datatype)); + String resolvedDatatype = SparqlAstToExpression.resolveIri(datatype, prologue); + yield factory.createLiteral(clean, factory.createIRI(resolvedDatatype)); } - yield factory.createLiteral(lexical); + yield factory.createLiteral(clean); } default -> throw new UnsupportedQueryFeatureException( "Variables are not allowed in INSERT/DELETE DATA: " + term); diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java index b5fe29d01..4c3b7a631 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/bridge/SparqlAstToExpression.java @@ -131,16 +131,40 @@ private static Constant literalToConstant(String lexical, String lang, String da return Constant.createString(unquoteLexical(lexical)); } - private static String unquoteLexical(String lexical) { - if (lexical.length() >= 2 && lexical.startsWith("\"")) { - if (lexical.endsWith("\"")) { - return lexical.substring(1, lexical.length() - 1); + public static String unquoteLexical(String lexical) { + if (lexical == null || lexical.length() < 2) { + return lexical; + } + String unquotedTriple = stripTripleQuotes(lexical); + if (unquotedTriple != null) { + return unquotedTriple; + } + return stripSingleQuotes(lexical); + } + + private static String stripTripleQuotes(String lexical) { + if (lexical.length() >= 6) { + if (lexical.startsWith("\"\"\"") && lexical.endsWith("\"\"\"")) { + return lexical.substring(3, lexical.length() - 3); } - int langIdx = lexical.lastIndexOf('"'); - if (langIdx > 0) { - return lexical.substring(1, langIdx); + if (lexical.startsWith("'''") && lexical.endsWith("'''")) { + return lexical.substring(3, lexical.length() - 3); } } + return null; + } + + private static String stripSingleQuotes(String lexical) { + char quote = lexical.charAt(0); + if (quote != '"' && quote != '\'') { + return lexical; + } + int endIdx = lexical.endsWith(String.valueOf(quote)) + ? lexical.length() - 1 + : lexical.lastIndexOf(quote); + if (endIdx > 0) { + return lexical.substring(1, endIdx); + } return lexical; } @@ -189,7 +213,7 @@ private static Constant iriToConstant(String rawIri) { *
  • Prefixed names (prefix:local) are expanded using declared prefixes or Corese default namespaces.
    • * */ - static String resolveIri(String raw, QueryPrologueAst prologue) { + public static String resolveIri(String raw, QueryPrologueAst prologue) { if (raw == null) { return null; } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/ConstructQueryAstListener.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/ConstructQueryAstListener.java index e64403434..1b3dddb88 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/ConstructQueryAstListener.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/listener/ConstructQueryAstListener.java @@ -4,6 +4,7 @@ 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.ast.TermAst; +import org.antlr.v4.runtime.RuleContext; import java.util.List; @@ -29,10 +30,20 @@ public SparqlQueryAstBuilder queryBuilder() { @Override public void enterConstructQuery(SparqlParser.ConstructQueryContext ctx) { queryBuilder().enterConstructQuery(); + if (ctx.constructTemplate() == null) { + queryBuilder().enterConstructTemplate(); + queryBuilder().enterGroup(); + queryBuilder().enterBgp(); + } } @Override public void exitConstructQuery(SparqlParser.ConstructQueryContext ctx) { + if (ctx.constructTemplate() == null) { + queryBuilder().exitBgp(); + queryBuilder().exitGroup(); + queryBuilder().exitConstructTemplate(); + } queryBuilder().exitConstructQuery(); } @@ -47,11 +58,14 @@ public void exitConstructTemplate(SparqlParser.ConstructTemplateContext ctx) { } /** - * Only handles {@code triplesSameSubject} nodes inside the CONSTRUCT template (not the WHERE BGP). + * Handles {@code triplesSameSubject} nodes inside the CONSTRUCT template + * or inside the short-form {@code CONSTRUCT WHERE { triplesTemplate }}. */ @Override public void exitTriplesSameSubject(SparqlParser.TriplesSameSubjectContext ctx) { - if (!(ctx.getParent() instanceof SparqlParser.ConstructTriplesContext)) { + boolean inConstructTriples = ctx.getParent() instanceof SparqlParser.ConstructTriplesContext; + boolean inConstructWhere = isConstructWhere(ctx); + if (!inConstructTriples && !inConstructWhere) { return; } if (ctx.varOrTerm() == null || ctx.propertyListNotEmpty() == null) { @@ -64,7 +78,18 @@ public void exitTriplesSameSubject(SparqlParser.TriplesSameSubjectContext ctx) { List objects = queryBuilder().termListFromObjectList(propertyList.objectList(verbIndex)); for (TermAst object : objects) { queryBuilder().addConstructTriple(subject, predicate, object); + if (inConstructWhere) { + queryBuilder().addTriple(subject, predicate, object); + } } } } + + private boolean isConstructWhere(SparqlParser.TriplesSameSubjectContext ctx) { + RuleContext parent = ctx.getParent(); + while (parent instanceof SparqlParser.TriplesTemplateContext) { + parent = parent.getParent(); + } + return parent instanceof SparqlParser.ConstructQueryContext; + } } diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/semantic/rule/SelectProjectionScopeValidationRule.java b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/semantic/rule/SelectProjectionScopeValidationRule.java index b150a2a26..32b55bf6d 100644 --- a/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/semantic/rule/SelectProjectionScopeValidationRule.java +++ b/src/main/java/fr/inria/corese/core/next/query/impl/sparql/parser/semantic/rule/SelectProjectionScopeValidationRule.java @@ -1,19 +1,20 @@ package fr.inria.corese.core.next.query.impl.sparql.parser.semantic.rule; import fr.inria.corese.core.next.query.api.validation.QueryDiagnostic; -import fr.inria.corese.core.next.query.impl.sparql.ast.ProjectionAst; import fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst; import fr.inria.corese.core.next.query.impl.sparql.ast.SelectQueryAst; -import fr.inria.corese.core.next.query.impl.sparql.ast.VarAst; -import java.util.ArrayList; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; /** - * Validates that explicitly projected SELECT variables are visible from the - * WHERE clause scope. + * Validates aliases introduced by explicit SELECT expressions. + * + *

    A projected variable, or a variable referenced by a SELECT expression, may be + * unbound. The expression then evaluates to an error for that solution. In contrast, + * the target variable of {@code (expr AS ?var)} must be new at the point where it is + * introduced.

    */ public final class SelectProjectionScopeValidationRule extends AbstractSemanticValidationRule { @@ -33,14 +34,15 @@ public List validate(QueryAst queryAst) { return List.of(); } - Set visibleVariables = collectSelectAvailableVariables(selectQueryAst); - List diagnostics = new ArrayList<>(); - validateProjectionVariables(selectQueryAst.projection(), visibleVariables, diagnostics); - return List.copyOf(diagnostics); + Set inScopeVariables = collectSelectAvailableVariables(selectQueryAst); + return selectQueryAst.projection().expressionBoundVariables().stream() + .filter(inScopeVariables::contains) + .map(this::buildAlreadyInScopeDiagnostic) + .toList(); } /** - * SELECT projections can reuse aliases introduced by {@code GROUP BY (expr AS ?var)}. + * GROUP BY aliases are already in scope and cannot be reused as SELECT expression targets. */ private Set collectSelectAvailableVariables(SelectQueryAst selectQueryAst) { Set visibleVariables = new LinkedHashSet<>(collectVisibleVariables(selectQueryAst)); @@ -48,38 +50,15 @@ private Set collectSelectAvailableVariables(SelectQueryAst selectQueryAs return visibleVariables; } - private void validateProjectionVariables( - ProjectionAst projection, - Set visibleVariables, - List diagnostics - ) { - Set availableVariables = new LinkedHashSet<>(visibleVariables); - for (VarAst projectedVar : projection.variables()) { - if (projection.expressionBoundVariables().contains(projectedVar.name())) { - validateProjectionExpression(projectedVar.name(), projection, availableVariables, diagnostics); - availableVariables.add(projectedVar.name()); - continue; - } - if (!availableVariables.contains(projectedVar.name())) { - diagnostics.add(buildOutOfScopeDiagnostic(projectedVar.name(), ScopeClause.SELECT_PROJECTION)); - } - } + private QueryDiagnostic buildAlreadyInScopeDiagnostic(String variableName) { + return new QueryDiagnostic( + QueryDiagnostic.Kind.SEMANTIC_ERROR, + QueryDiagnostic.Severity.ERROR, + "Variable ?" + variableName + " introduced by SELECT expression is already in scope", + -1, + -1, + "?" + variableName, + getDiagnosticSource()); } - /** - * SELECT expressions introduce the projected variable themselves, but the variables they reference - * must still be visible from the query scope. - */ - private void validateProjectionExpression( - String projectionVariableName, - ProjectionAst projection, - Set visibleVariables, - List diagnostics - ) { - addOutOfScopeDiagnostics( - projection.expressionReferencedVariables().getOrDefault(projectionVariableName, Set.of()), - visibleVariables, - ScopeClause.SELECT_PROJECTION, - diagnostics); - } } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/repository/CoreseRepositoryConnectionTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/repository/CoreseRepositoryConnectionTest.java index 761915d3d..d1750868e 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/repository/CoreseRepositoryConnectionTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/repository/CoreseRepositoryConnectionTest.java @@ -10,6 +10,7 @@ import fr.inria.corese.core.next.query.api.Update; import fr.inria.corese.core.next.query.api.dataset.Dataset; import fr.inria.corese.core.next.query.api.exception.QuerySyntaxException; +import fr.inria.corese.core.next.query.api.exception.QueryEvaluationException; import fr.inria.corese.core.next.query.api.exception.RepositoryException; import fr.inria.corese.core.next.query.api.result.GraphQueryResult; import fr.inria.corese.core.next.data.api.model.Statement; @@ -206,6 +207,23 @@ void constructReturnsEmptyWhenNoMatch() { } } + @Test + @DisplayName("CONSTRUCT WHERE { ?s ?p ?o } short form returns the stored triple") + void constructWhereReturnsStoredTriple() { + try (RepositoryConnection conn = repository.getConnection()) { + GraphQuery q = conn.prepareGraphQuery( + "CONSTRUCT WHERE { ?s ?p ?o }"); + try (GraphQueryResult result = q.evaluate()) { + assertTrue(result.hasNext(), "Expected at least one constructed statement"); + Statement stmt = result.next(); + assertEquals(ALICE, stmt.getSubject().stringValue()); + assertEquals(KNOWS, stmt.getPredicate().stringValue()); + assertEquals(BOB, stmt.getObject().stringValue()); + assertFalse(result.hasNext(), "Expected exactly one statement"); + } + } + } + @Test @DisplayName("Throws QuerySyntaxException for a non-CONSTRUCT/DESCRIBE query string") void throwsSyntaxExceptionForSelect() { @@ -260,11 +278,13 @@ void insertDataInsertsTriple() { } @Test - @DisplayName("DELETE DATA removes a triple from the store") - void deleteDataRemovesTriple() { + @DisplayName("DELETE DATA resolves prefixed IRIs and removes a triple from the store") + void deleteDataResolvesPrefixedIris() { try (RepositoryConnection conn = repository.getConnection()) { - Update u = conn.prepareUpdate( - "DELETE DATA { <" + ALICE + "> <" + KNOWS + "> <" + BOB + "> }"); + Update u = conn.prepareUpdate(""" + PREFIX ex: + DELETE DATA { ex:alice ex:knows ex:bob } + """); u.execute(); } try (RepositoryConnection conn = repository.getConnection()) { @@ -273,6 +293,71 @@ void deleteDataRemovesTriple() { assertFalse(q.evaluate(), "DELETE DATA should have removed alice→knows→bob"); } } + + @Test + @DisplayName("DELETE DATA rejects blank nodes") + void deleteDataRejectsBlankNodes() { + try (RepositoryConnection conn = repository.getConnection()) { + Update u = conn.prepareUpdate(""" + PREFIX ex: + DELETE DATA { _:b ex:knows ex:bob } + """); + assertThrows(QueryEvaluationException.class, u::execute); + } + } + + @Test + @DisplayName("INSERT DATA supports prefixed IRIs, blank nodes and literals") + void insertDataSupportsPrefixedIrisAndBlankNodes() { + try (RepositoryConnection conn = repository.getConnection()) { + Update u = conn.prepareUpdate(""" + PREFIX ex: + PREFIX xsd: + INSERT DATA { + ex:dave ex:knows _:b1 . + _:b1 ex:name "Dave Friend"@en ; + ex:age "30"^^xsd:integer . + } + """); + u.execute(); + } + try (RepositoryConnection conn = repository.getConnection()) { + BooleanQuery q = conn.prepareBooleanQuery(""" + PREFIX ex: + PREFIX xsd: + ASK WHERE { + ex:dave ex:knows ?b . + ?b ex:name "Dave Friend"@en . + ?b ex:age "30"^^xsd:integer . + } + """); + assertTrue(q.evaluate(), "Prefixed IRIs, blank nodes, and literals should be inserted"); + } + } + + @Test + @DisplayName("INSERT DATA blank node labels are fresh for each update request") + void insertDataCreatesFreshBlankNodesForEachRequest() { + try (RepositoryConnection conn = repository.getConnection()) { + conn.prepareUpdate(""" + PREFIX ex: + INSERT DATA { ex:dave ex:knows _:b1 } + """).execute(); + conn.prepareUpdate(""" + PREFIX ex: + INSERT DATA { ex:erin ex:knows _:b1 } + """).execute(); + + BooleanQuery q = conn.prepareBooleanQuery(""" + PREFIX ex: + ASK WHERE { + ex:dave ex:knows ?b . + ex:erin ex:knows ?b + } + """); + assertFalse(q.evaluate(), "Blank node labels must not leak across update requests"); + } + } } // ------------------------------------------------------------------------- diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserConstructQueryTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserConstructQueryTest.java index c4bc79e9a..a92ef0c50 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserConstructQueryTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserConstructQueryTest.java @@ -101,6 +101,47 @@ ORDER BY DESC(?o) assertEquals(5L, solutionModifier.offset()); } + @Test + @DisplayName("Should parse short-form CONSTRUCT WHERE query") + void shouldParseShortFormConstructWhereQuery() { + SparqlParser parser = newParserDefault(); + + QueryAst ast = parser.parse(""" + CONSTRUCT WHERE { + ?s ?p ?o . + ?o ?x + } + """); + + assertInstanceOf(ConstructQueryAst.class, ast); + ConstructQueryAst construct = (ConstructQueryAst) ast; + + assertNotNull(construct.constructTemplate()); + assertNotNull(construct.whereClause()); + assertEquals(2, construct.constructTemplate().triplePatternAsts().size()); + + GroupGraphPatternAst where = construct.whereClause(); + assertEquals(1, where.patterns().size()); + assertInstanceOf(BgpAst.class, where.patterns().getFirst()); + BgpAst bgp = (BgpAst) where.patterns().getFirst(); + assertEquals(2, bgp.triples().size()); + } + + @Test + @DisplayName("Should expand the W3C CONSTRUCT WHERE object-list shorthand in both clauses") + void shouldParseW3cConstructWhereObjectList() { + SparqlParser parser = newParserDefault(); + + ConstructQueryAst construct = assertInstanceOf(ConstructQueryAst.class, parser.parse(""" + PREFIX : + CONSTRUCT WHERE { :s2 :p ?o1, ?o2 } + """)); + + assertEquals(2, construct.constructTemplate().triplePatternAsts().size()); + BgpAst bgp = assertInstanceOf(BgpAst.class, construct.whereClause().patterns().getFirst()); + assertEquals(2, bgp.triples().size()); + } + private void assertTemplateWithBlankNodes(ConstructTemplateAst template) { assertEquals(3, template.triplePatternAsts().size()); diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserMinusTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserMinusTest.java index e7414bfb5..04ad799a1 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserMinusTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserMinusTest.java @@ -1,6 +1,5 @@ package fr.inria.corese.core.next.query.impl.sparql.parser; -import fr.inria.corese.core.next.query.api.exception.QueryValidationException; import fr.inria.corese.core.next.query.impl.sparql.parser.semantic.support.VariableScopeAnalyzer; import fr.inria.corese.core.next.query.impl.sparql.ast.*; import org.junit.jupiter.api.DisplayName; @@ -11,7 +10,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; class SparqlParserMinusTest extends AbstractSparqlParserFeatureTest { @@ -84,19 +83,15 @@ void shouldExcludeMinusVariablesFromSelectAllScope() { } @Test - @DisplayName("SELECT projection should reject a variable declared only inside MINUS") - void shouldRejectProjectionOfMinusOnlyVariable() { + @DisplayName("SELECT projection may contain a variable declared only inside MINUS") + void shouldAcceptProjectionOfMinusOnlyVariable() { SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" + assertDoesNotThrow(() -> parser.parse(""" SELECT ?hidden WHERE { ?s ?p ?o . MINUS { ?s ?q ?hidden . } } """)); - - assertEquals( - "Variable ?hidden used in SELECT projection is not visible in WHERE clause", - exception.getMessage()); } } diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserTest.java index 9ce77bc34..b5578d52e 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserTest.java @@ -14,8 +14,13 @@ import java.io.StringReader; import java.nio.charset.StandardCharsets; +import java.util.stream.Stream; + import fr.inria.corese.core.next.query.impl.sparql.ast.SparqlQueryAst; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.antlr.v4.runtime.misc.ParseCancellationException; import fr.inria.corese.core.next.query.api.exception.QueryEvaluationException; @@ -219,7 +224,7 @@ void validateReturnsMultipleSemanticDiagnostics() { """); assertFalse(result.isValid()); - assertEquals(4, result.diagnostics().size()); + assertEquals(2, result.diagnostics().size()); assertTrue(result.diagnostics().stream() .allMatch(diagnostic -> diagnostic.kind() == QueryDiagnostic.Kind.SEMANTIC_ERROR)); } @@ -247,87 +252,130 @@ void validateReturnsConstructSemanticDiagnostic() { assertEquals("OrderByScopeValidationRule", result.diagnostics().getFirst().source()); } - @Test - void validateAcceptsConstructOrderByVariableVisibleInValues() { + @ParameterizedTest(name = "{0}") + @MethodSource("validQueryValidationCases") + void validateAcceptsValidQueries(String testName, String query) { SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - CONSTRUCT { - ?s ?p ?o - } - WHERE { - ?s ?p ?o - } - ORDER BY ?rank - VALUES ?rank { 1 } - """); - + QueryValidationResult result = parser.validate(query); assertTrue(result.isValid()); assertTrue(result.diagnostics().isEmpty()); } - @Test - void validateReturnsAskSemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - ASK - WHERE { - ?s ?p ?o - } - ORDER BY ?z - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("OrderByScopeValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateReturnsGroupBySemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s WHERE { - ?s ?p ?o - } - GROUP BY ?z - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("GroupByScopeValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateReturnsGroupedSelectProjectionSemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s ?o WHERE { - ?s ?p ?o - } - GROUP BY ?s - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("GroupedSelectProjectionValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateReturnsImplicitAggregateProjectionSemanticDiagnostic() { + private static Stream validQueryValidationCases() { + return Stream.of( + Arguments.of("Construct ORDER BY variable visible in VALUES", """ + CONSTRUCT { + ?s ?p ?o + } + WHERE { + ?s ?p ?o + } + ORDER BY ?rank + VALUES ?rank { 1 } + """), + Arguments.of("SELECT expression referencing an unbound variable", """ + SELECT (STR(?x) AS ?label) WHERE { + ?s ?p ?o + } + """), + Arguments.of("SELECT expression using earlier alias", """ + SELECT (?p AS ?price) (STR(?price) AS ?label) WHERE { + ?s ?p ?o + } + """), + Arguments.of("Implicit aggregate projection", """ + SELECT (COUNT(?o) AS ?count) WHERE { + ?s ?p ?o + } + """), + Arguments.of("GROUP BY expression alias projection", """ + SELECT ?key WHERE { + ?s ?p ?o + } + GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) + """), + Arguments.of("HAVING using GROUP BY expression alias", """ + SELECT ?key WHERE { + ?s ?p ?o + } + GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) + HAVING (BOUND(?key)) + """), + Arguments.of("ORDER BY using GROUP BY expression alias", """ + SELECT ?key WHERE { + ?s ?p ?o + } + GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) + ORDER BY ?key + """), + Arguments.of("ORDER BY using projected aggregate alias", """ + SELECT ?s (COUNT(?o) AS ?count) WHERE { + ?s ?p ?o + } + GROUP BY ?s + ORDER BY ?count + """) + ); + } + + @ParameterizedTest(name = "{0}") + @MethodSource("semanticDiagnosticQueryCases") + void validateReturnsSemanticDiagnostic(String testName, String query, String expectedSource) { SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s (COUNT(?o) AS ?count) WHERE { - ?s ?p ?o - } - """); - + QueryValidationResult result = parser.validate(query); assertFalse(result.isValid()); assertEquals(1, result.diagnostics().size()); - assertEquals("GroupedSelectProjectionValidationRule", result.diagnostics().getFirst().source()); + assertEquals(expectedSource, result.diagnostics().getFirst().source()); + } + + private static Stream semanticDiagnosticQueryCases() { + return Stream.of( + Arguments.of("ASK query with out-of-scope ORDER BY", """ + ASK + WHERE { + ?s ?p ?o + } + ORDER BY ?z + """, "OrderByScopeValidationRule"), + Arguments.of("GROUP BY with out-of-scope variable", """ + SELECT ?s WHERE { + ?s ?p ?o + } + GROUP BY ?z + """, "GroupByScopeValidationRule"), + Arguments.of("Grouped SELECT projection with ungrouped variable", """ + SELECT ?s ?o WHERE { + ?s ?p ?o + } + GROUP BY ?s + """, "GroupedSelectProjectionValidationRule"), + Arguments.of("Implicit aggregate projection with ungrouped variable", """ + SELECT ?s (COUNT(?o) AS ?count) WHERE { + ?s ?p ?o + } + """, "GroupedSelectProjectionValidationRule"), + Arguments.of("HAVING with out-of-scope variable", """ + SELECT ?s WHERE { + ?s ?p ?o + } + GROUP BY ?s + HAVING (BOUND(?z)) + """, "HavingScopeValidationRule"), + Arguments.of("Grouped HAVING with ungrouped variable", """ + SELECT ?s WHERE { + ?s ?p ?o + } + GROUP BY ?s + HAVING (BOUND(?o)) + """, "GroupedHavingValidationRule"), + Arguments.of("Grouped ORDER BY with ungrouped variable", """ + SELECT ?s WHERE { + ?s ?p ?o + } + GROUP BY ?s + ORDER BY ?o + """, "GroupedOrderByValidationRule") + ); } @Test @@ -347,35 +395,6 @@ void validateReturnsGroupedSelectAllSemanticDiagnostic() { assertEquals("SELECT * is not permitted with GROUP BY", result.diagnostics().getFirst().message()); } - @Test - void validateReturnsSelectExpressionSemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT (STR(?x) AS ?label) WHERE { - ?s ?p ?o - } - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("SelectProjectionScopeValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateAcceptsSelectExpressionUsingEarlierAlias() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT (?p AS ?price) (STR(?price) AS ?label) WHERE { - ?s ?p ?o - } - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - @Test void validateRejectsGroupedSelectExpressionMatchingGroupExpression() { SparqlParser parser = new SparqlParser(); @@ -393,134 +412,6 @@ GROUP BY CONCAT(?s, ?o) .allMatch(diagnostic -> "GroupedSelectProjectionValidationRule".equals(diagnostic.source()))); } - @Test - void validateAcceptsImplicitAggregateProjection() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT (COUNT(?o) AS ?count) WHERE { - ?s ?p ?o - } - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - - @Test - void validateAcceptsGroupByExpressionAliasProjection() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?key WHERE { - ?s ?p ?o - } - GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - - @Test - void validateReturnsHavingSemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s WHERE { - ?s ?p ?o - } - GROUP BY ?s - HAVING (BOUND(?z)) - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("HavingScopeValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateAcceptsHavingUsingGroupByExpressionAlias() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?key WHERE { - ?s ?p ?o - } - GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) - HAVING (BOUND(?key)) - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - - @Test - void validateReturnsGroupedHavingSemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s WHERE { - ?s ?p ?o - } - GROUP BY ?s - HAVING (BOUND(?o)) - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("GroupedHavingValidationRule", result.diagnostics().getFirst().source()); - } - - @Test - void validateAcceptsOrderByUsingGroupByExpressionAlias() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?key WHERE { - ?s ?p ?o - } - GROUP BY (CONCAT(STR(?s), STR(?o)) AS ?key) - ORDER BY ?key - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - - @Test - void validateAcceptsOrderByUsingProjectedAggregateAlias() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s (COUNT(?o) AS ?count) WHERE { - ?s ?p ?o - } - GROUP BY ?s - ORDER BY ?count - """); - - assertTrue(result.isValid()); - assertTrue(result.diagnostics().isEmpty()); - } - - @Test - void validateReturnsGroupedOrderBySemanticDiagnostic() { - SparqlParser parser = new SparqlParser(); - - QueryValidationResult result = parser.validate(""" - SELECT ?s WHERE { - ?s ?p ?o - } - GROUP BY ?s - ORDER BY ?o - """); - - assertFalse(result.isValid()); - assertEquals(1, result.diagnostics().size()); - assertEquals("GroupedOrderByValidationRule", result.diagnostics().getFirst().source()); - } - @Test void validateReturnsDescribeSemanticDiagnostic() { SparqlParser parser = new SparqlParser(); diff --git a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserValidationTest.java b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserValidationTest.java index b6e044414..fec7211b7 100644 --- a/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserValidationTest.java +++ b/src/test/java/fr/inria/corese/core/next/query/impl/sparql/parser/SparqlParserValidationTest.java @@ -16,12 +16,6 @@ class SparqlParserValidationTest extends AbstractSparqlParserFeatureTest { private static final String ORDER_BY_SCOPE_MESSAGE = "Variable ?z used in ORDER BY is not visible in WHERE clause"; - private static final String SELECT_PROJECTION_SCOPE_MESSAGE = - "Variable ?x used in SELECT projection is not visible in WHERE clause"; - private static final String CITY_LABEL_SCOPE_MESSAGE = - "Variable ?cityLabel used in SELECT projection is not visible in WHERE clause"; - private static final String LABEL_SCOPE_MESSAGE = - "Variable ?label used in SELECT projection is not visible in WHERE clause"; private static final String GROUP_BY_SCOPE_MESSAGE = "Variable ?z used in GROUP BY is not visible in WHERE clause"; private static final String HAVING_SCOPE_MESSAGE = @@ -47,6 +41,28 @@ void shouldRejectInvalidSelectQueries(String testName, String query, String expe assertEquals(expectedMessage, exception.getMessage()); } + @ParameterizedTest(name = "{0}") + @MethodSource("validUnboundProjectionQueries") + void shouldAcceptUnboundProjectionQueries(String testName, String query) { + assertDoesNotThrow(() -> newParserDefault().parse(query)); + } + + @Test + @DisplayName("Should reject a SELECT expression alias already introduced by the graph pattern") + void shouldRejectSelectExpressionAliasAlreadyInScope() { + SparqlParser parser = newParserDefault(); + String query = """ + SELECT (?p AS ?s) WHERE { + ?s ?p ?o + } + """; + QueryValidationException exception = assertThrows( + QueryValidationException.class, + () -> parser.parse(query)); + + assertEquals("Variable ?s introduced by SELECT expression is already in scope", exception.getMessage()); + } + @Nested class AskValidationTest { @@ -146,484 +162,349 @@ void shouldRejectBindVariableAlreadyVisibleFromPreviousBind() { @Nested class FilterValidationTest { - @Test - void shouldRejectConcatUsedAsNumericOperand() { - SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(CONCAT("1", "2") + 1 = ?o) - } - """)); - - assertEquals("CONCAT used in + should be resolvable to a numeric", exception.getMessage()); - } - @Test - @DisplayName("Should accept FILTER with numeric operator") - void shouldAcceptFilterWithNumericOperator() { + @ParameterizedTest(name = "{0}") + @MethodSource("invalidFilterQueries") + void shouldRejectInvalidFilterQueries(String testName, String query, String expectedMessage) { SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(RAND()) - } - """)); + QueryValidationException exception = assertThrows( + QueryValidationException.class, + () -> parser.parse(query)); + assertEquals(expectedMessage, exception.getMessage()); } - @Test - @DisplayName("Should reject FILTER with IRI-returning operator") - void shouldRejectFilterWithIRIOperator() { - SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(DATATYPE("test"^^)) - } - """)); - - assertEquals("DATATYPE used in FILTER should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should reject FILTER with datetime operator") - void shouldRejectFilterWithDatetime() { - SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(NOW()) - } - """)); - - assertEquals("NOW used in FILTER should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should accept FILTER with numeric expression derived from datetime") - void shouldAcceptFilterWithDuration() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(DAY(NOW())) - } - """)); - } - - @Test - @DisplayName("Should accept FILTER with plain literal") - void shouldAcceptFilterWithPlainLiteral() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER("test") - } - """)); - } - - @Test - @DisplayName("Should accept FILTER with CONCAT result") - void shouldAcceptFilterWithConcat() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(CONCAT("te", "st")) - } - """)); - } - - @Test - @DisplayName("Should reject FILTER when IF condition is not EBV-compatible") - void shouldRejectFilterContainingIfWithIncorrectConditionType() { - SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IF(NOW(), true, false)) - } - """)); - - assertEquals("IF used in FILTER should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should let pass FILTER with simple literal operator") - void shouldAcceptFilterWithStringOperator() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(STR("test"^^)) - } - """)); - } - - @Test - @DisplayName("Should let pass FILTER with variable") - void shouldAcceptFilterWithVariable() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(?o) - } - """)); - } - - @Test - @DisplayName("Should let pass FILTER with boolean") - void shouldAcceptFilterWithBoolean() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(true) - } - """)); - } - - @Test - @DisplayName("Should let pass FILTER containing a IF that returns boolean or any acceptable AST type.") - void shouldAcceptFilterContainingIfWithCorrectType() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IF(?s, true, ?o)) - } - """); - }); - } - - @Test - @DisplayName("Should reject FILTER containing a IF that does not returns boolean or any acceptable AST type.") - void shouldRejectFilterContainingIfWithIncorrectType() { - SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IF(?s, 4, )) - } - """)); - - assertEquals("IF used in FILTER should be resolvable to a boolean", exception.getMessage()); + private static Stream invalidFilterQueries() { + return Stream.of( + Arguments.of( + "Should reject CONCAT used as numeric operand", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(CONCAT("1", "2") + 1 = ?o) + } + """, + "CONCAT used in + should be resolvable to a numeric"), + Arguments.of( + "Should reject FILTER with IRI-returning operator", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(DATATYPE("test"^^)) + } + """, + "DATATYPE used in FILTER should be resolvable to a boolean"), + Arguments.of( + "Should reject FILTER with datetime operator", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(NOW()) + } + """, + "NOW used in FILTER should be resolvable to a boolean"), + Arguments.of( + "Should reject FILTER when IF condition is not EBV-compatible", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IF(NOW(), true, false)) + } + """, + "IF used in FILTER should be resolvable to a boolean"), + Arguments.of( + "Should reject FILTER containing an IF that does not return boolean or acceptable AST type", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IF(?s, 4, )) + } + """, + "IF used in FILTER should be resolvable to a boolean"), + Arguments.of( + "Should reject FILTER containing an IF that does not return only boolean or acceptable AST type", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IF(?s, true, )) + } + """, + "IF used in FILTER should be resolvable to a boolean") + ); } - @Test - @DisplayName("Should reject FILTER containing a IF that does not returns only boolean or any acceptable AST type.") - void shouldRejectFilterContainingIfWithIncorrectTypeMix() { + @ParameterizedTest(name = "{0}") + @MethodSource("validFilterQueries") + void shouldAcceptValidFilterQueries(String testName, String query) { SparqlParser parser = newParserDefault(); - - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IF(?s, true, )) - } - """)); - - assertEquals("IF used in FILTER should be resolvable to a boolean", exception.getMessage()); + assertDoesNotThrow(() -> parser.parse(query)); } - @Test - @DisplayName("Should accept FILTER with numeric operator in a nested BGP") - void shouldAcceptFilterWithNumericOperatorInNestedBGP() { - SparqlParser parser = newParserDefault(); - - assertDoesNotThrow(() -> parser.parse(""" - SELECT * WHERE { - { + private static Stream validFilterQueries() { + return Stream.of( + Arguments.of( + "Should accept FILTER with numeric operator", + """ + SELECT * WHERE { ?x ?p ?o . FILTER(RAND()) - } UNION { - ?s ?p ?o . - } - } - """)); + } + """), + Arguments.of( + "Should accept FILTER with numeric expression derived from datetime", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(DAY(NOW())) + } + """), + Arguments.of( + "Should accept FILTER with plain literal", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER("test") + } + """), + Arguments.of( + "Should accept FILTER with CONCAT result", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(CONCAT("te", "st")) + } + """), + Arguments.of( + "Should let pass FILTER with simple literal operator", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(STR("test"^^)) + } + """), + Arguments.of( + "Should let pass FILTER with variable", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(?o) + } + """), + Arguments.of( + "Should let pass FILTER with boolean", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(true) + } + """), + Arguments.of( + "Should let pass FILTER containing an IF that returns boolean or acceptable AST type", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IF(?s, true, ?o)) + } + """), + Arguments.of( + "Should accept FILTER with numeric operator in a nested BGP", + """ + SELECT * WHERE { + { + ?x ?p ?o . + FILTER(RAND()) + } UNION { + ?s ?p ?o . + } + } + """) + ); } } @Nested class OperandTypeTest { - @Test - @DisplayName("Should accept + operator with numerics") - void shouldAcceptPlusWithNumerics() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(RAND() + 1 = ?o) - } - """); - }); - } - - @Test - @DisplayName("Should accept - operator with numerics") - void shouldAcceptMinusWithNumerics() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(STRLEN("test") - 1 = ?o) - } - """); - }); - } - - @Test - @DisplayName("Should accept * operator with numerics") - void shouldAcceptMultiplyWithNumerics() { + @ParameterizedTest(name = "{0}") + @MethodSource("validOperandQueries") + void shouldAcceptValidOperandQueries(String testName, String query) { SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(STRLEN("test") * RAND() = ?o) - } - """); - }); + assertDoesNotThrow(() -> parser.parse(query)); } - @Test - @DisplayName("Should accept / operator with numerics") - void shouldAcceptDivideWithNumerics() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(DAY(NOW()) / 2 = ?o) - } - """); - }); - } - - @Test - @DisplayName("Should reject + operator with non numerics") - void shouldRefusePlusWithNonNumerics() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(RAND() + "one" = ?o) - } - """); - }); - assertEquals("\"one\" used in + should be resolvable to a numeric", exception.getMessage()); - } - - @Test - @DisplayName("Should reject - operator with non numerics") - void shouldRejectMinusWithNonNumerics() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER((3 - STRENDS("test", "")) = ?o) - } - """); - }); - assertEquals("STRENDS used in - should be resolvable to a numeric", exception.getMessage()); - } - - @Test - @DisplayName("Should reject * operator with non numerics") - void shouldRejectDivideWithNonNumerics() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER( / 2 = ?o) - } - """); - }); - assertEquals(" used in / should be resolvable to a numeric", exception.getMessage()); - } - - @Test - @DisplayName("Should reject / operator with non numerics") - void shouldRejectMultiplyWithNonNumerics() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(STRLEN("test") * NOW() = ?o) - } - """); - }); - assertEquals("NOW used in * should be resolvable to a numeric", exception.getMessage()); - } - - @Test - @DisplayName("Should accept || operator with booleans") - void shouldAcceptOrWithBoolean() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IsIri(?s) || false) - } - """); - }); - } - - @Test - @DisplayName("Should accept && operator with booleans") - void shouldAcceptAndWithBoolean() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(NOT EXISTS { ?s ?p false } && STRSTARTS("test", "t")) - } - """); - }); - } - - @Test - @DisplayName("Should reject || operator with non booleans") - void shouldRejectOrWithNonBoolean() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(IsIri(?s) || "potato"^^) - } - """); - }); - assertEquals("\"potato\"^^ used in || should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should reject && operator with non booleans") - void shouldRejectAndWithNonBoolean() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER( && langMatches("potato", "fr")) - } - """); - }); - assertEquals(" used in && should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should accept ! operator with booleans") - void shouldAcceptNotWithBoolean() { - SparqlParser parser = newParserDefault(); - assertDoesNotThrow(() -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(! NOT EXISTS { ?s ?p false } && STRSTARTS("test", "t")) - } - """); - }); - } - - @Test - @DisplayName("Should reject ! operator with non booleans") - void shouldRejectNotWithNonBoolean() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(! ) - } - """); - }); - assertEquals(" used in ! should be resolvable to a boolean", exception.getMessage()); - } - - @Test - @DisplayName("Should reject < operator with IRIs") - void shouldRejectLTWithIRIs() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(2 < ) - } - """); - }); - assertEquals(" used in < should be resolvable to a not an IRI", exception.getMessage()); + private static Stream validOperandQueries() { + return Stream.of( + Arguments.of( + "Should accept + operator with numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(RAND() + 1 = ?o) + } + """), + Arguments.of( + "Should accept - operator with numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(STRLEN("test") - 1 = ?o) + } + """), + Arguments.of( + "Should accept * operator with numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(STRLEN("test") * RAND() = ?o) + } + """), + Arguments.of( + "Should accept / operator with numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(DAY(NOW()) / 2 = ?o) + } + """), + Arguments.of( + "Should accept || operator with booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IsIri(?s) || false) + } + """), + Arguments.of( + "Should accept && operator with booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(NOT EXISTS { ?s ?p false } && STRSTARTS("test", "t")) + } + """), + Arguments.of( + "Should accept ! operator with booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(! NOT EXISTS { ?s ?p false } && STRSTARTS("test", "t")) + } + """) + ); } - @Test - @DisplayName("Should reject <= operator with IRIs") - void shouldRejectLTEWithIRIs() { + @ParameterizedTest(name = "{0}") + @MethodSource("invalidOperandQueries") + void shouldRejectInvalidOperandQueries(String testName, String query, String expectedMessage) { SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(STRLEN("test") <= ) - } - """); - }); - assertEquals(" used in <= should be resolvable to a not an IRI", exception.getMessage()); + QueryValidationException exception = assertThrows( + QueryValidationException.class, + () -> parser.parse(query)); + assertEquals(expectedMessage, exception.getMessage()); } - @Test - @DisplayName("Should reject > operator with IRIs") - void shouldRejectGTWithIRIs() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER("2"^^ > ) - } - """); - }); - assertEquals(" used in > should be resolvable to a not an IRI", exception.getMessage()); - } - - @Test - @DisplayName("Should reject >= operator with IRIs") - void shouldRejectGTEWithIRIs() { - SparqlParser parser = newParserDefault(); - QueryValidationException exception = assertThrows(QueryValidationException.class, () -> { - parser.parse(""" - SELECT * WHERE { - ?x ?p ?o . - FILTER(datatype("2"^^) >= 4) - } - """); - }); - assertEquals("DATATYPE used in >= should be resolvable to a not an IRI", exception.getMessage()); + private static Stream invalidOperandQueries() { + return Stream.of( + Arguments.of( + "Should reject + operator with non numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(RAND() + "one" = ?o) + } + """, + "\"one\" used in + should be resolvable to a numeric"), + Arguments.of( + "Should reject - operator with non numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER((3 - STRENDS("test", "")) = ?o) + } + """, + "STRENDS used in - should be resolvable to a numeric"), + Arguments.of( + "Should reject / operator with non numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER( / 2 = ?o) + } + """, + " used in / should be resolvable to a numeric"), + Arguments.of( + "Should reject * operator with non numerics", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(STRLEN("test") * NOW() = ?o) + } + """, + "NOW used in * should be resolvable to a numeric"), + Arguments.of( + "Should reject || operator with non booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(IsIri(?s) || "potato"^^) + } + """, + "\"potato\"^^ used in || should be resolvable to a boolean"), + Arguments.of( + "Should reject && operator with non booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER( && langMatches("potato", "fr")) + } + """, + " used in && should be resolvable to a boolean"), + Arguments.of( + "Should reject ! operator with non booleans", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(! ) + } + """, + " used in ! should be resolvable to a boolean"), + Arguments.of( + "Should reject < operator with IRIs", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(2 < ) + } + """, + " used in < should be resolvable to a not an IRI"), + Arguments.of( + "Should reject <= operator with IRIs", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(STRLEN("test") <= ) + } + """, + " used in <= should be resolvable to a not an IRI"), + Arguments.of( + "Should reject > operator with IRIs", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER("2"^^ > ) + } + """, + " used in > should be resolvable to a not an IRI"), + Arguments.of( + "Should reject >= operator with IRIs", + """ + SELECT * WHERE { + ?x ?p ?o . + FILTER(datatype("2"^^) >= 4) + } + """, + "DATATYPE used in >= should be resolvable to a not an IRI") + ); } - } private static Stream invalidSelectQueries() { @@ -637,26 +518,6 @@ private static Stream invalidSelectQueries() { ORDER BY ?z """, ORDER_BY_SCOPE_MESSAGE), - Arguments.of( - "Should reject projection variable only referenced in FILTER", - """ - SELECT ?x WHERE { - ?s ?p ?o . - FILTER(BOUND(?x)) - } - """, - SELECT_PROJECTION_SCOPE_MESSAGE), - Arguments.of( - "Should reject projection variable not visible through UNION", - """ - SELECT ?cityLabel - WHERE { - { ?country wdt:P36 ?city. } - UNION - { ?city wdt:P17 ?country. } - } - """, - CITY_LABEL_SCOPE_MESSAGE), Arguments.of( "Should reject ORDER BY variable not visible in WHERE", """ @@ -693,22 +554,6 @@ ORDER BY STR(?z) ORDER BY IF(BOUND(?o), ?o, ?z) """, ORDER_BY_SCOPE_MESSAGE), - Arguments.of( - "Should reject SELECT expression projection using a variable not visible in WHERE", - """ - SELECT (STR(?x) AS ?label) WHERE { - ?s ?p ?o - } - """, - SELECT_PROJECTION_SCOPE_MESSAGE), - Arguments.of( - "Should reject SELECT expression projection using a later alias not yet visible in SELECT", - """ - SELECT (STR(?label) AS ?copy) (STR(?p) AS ?label) WHERE { - ?s ?p ?o - } - """, - LABEL_SCOPE_MESSAGE), Arguments.of( "Should reject GROUP BY variable not visible in WHERE", """ @@ -826,14 +671,54 @@ GROUP BY CONCAT(?s, ?o) } ORDER BY ?s """, - "Variable ?s used in ORDER BY must be grouped or aggregated"), + "Variable ?s used in ORDER BY must be grouped or aggregated")); + } + + private static Stream validUnboundProjectionQueries() { + return Stream.of( Arguments.of( - "Should reject SELECT projection variables not visible in WHERE", + "Should accept projection variable only referenced in FILTER", """ SELECT ?x WHERE { - ?s ?p ?o + ?s ?p ?o . + FILTER(BOUND(?x)) } - """, - SELECT_PROJECTION_SCOPE_MESSAGE)); + """), + Arguments.of( + "Should accept projection variable not visible through UNION", + """ + SELECT ?cityLabel + WHERE { + { ?country wdt:P36 ?city. } + UNION + { ?city wdt:P17 ?country. } + } + """), + Arguments.of( + "Should accept SELECT expression projection using an unbound variable", + """ + SELECT (STR(?x) AS ?label) WHERE { + ?s ?p ?o + } + """), + Arguments.of( + "Should accept aggregate projection over an empty group", + """ + SELECT (SUM(?x) AS ?y) {} + """), + Arguments.of( + "Should accept SELECT expression projection referring to a later alias", + """ + SELECT (STR(?label) AS ?copy) (STR(?p) AS ?label) WHERE { + ?s ?p ?o + } + """), + Arguments.of( + "Should accept a projected variable not visible in WHERE", + """ + SELECT ?x WHERE { + ?s ?p ?o + } + """)); } }