Correct handling of Datasets

src/parser/SparqlParser.cpp

@@ -9,15 +9,17 @@
 using AntlrParser = SparqlAutomaticParser;
 
 // _____________________________________________________________________________
-ParsedQuery SparqlParser::parseQuery(std::string query) {
+ParsedQuery SparqlParser::parseQuery(
+    std::string query, const std::vector<DatasetClause>& datasets) {
   // The second argument is the `PrefixMap` for QLever's internal IRIs.
   using S = std::string;
   sparqlParserHelpers::ParserAndVisitor p{
       std::move(query),
-      {{S{QLEVER_INTERNAL_PREFIX_NAME}, S{QLEVER_INTERNAL_PREFIX_IRI}}}};
-  // Note: `AntlrParser::query` is a method of `AntlrParser` (which is an alias
-  // for `SparqlAutomaticParser`) that returns the `QueryContext*` for the whole
-  // query.
+      {{S{QLEVER_INTERNAL_PREFIX_NAME}, S{QLEVER_INTERNAL_PREFIX_IRI}}},
+      parsedQuery::DatasetClauses::fromClauses(datasets)};
+  // Note: `AntlrParser::queryOrUpdate` is a method of `AntlrParser` (which is
+  // an alias for `SparqlAutomaticParser`) that returns the
+  // `QueryOrUpdateContext*` for the whole query or update.
   auto resultOfParseAndRemainingText =
       p.parseTypesafe(&AntlrParser::queryOrUpdate);
   // The query rule ends with <EOF> so the parse always has to consume the whole
@@ -26,16 +28,3 @@ ParsedQuery SparqlParser::parseQuery(std::string query) {
   AD_CONTRACT_CHECK(resultOfParseAndRemainingText.remainingText_.empty());
   return std::move(resultOfParseAndRemainingText.resultOfParse_);
 }
-
-// _____________________________________________________________________________
-ParsedQuery SparqlParser::parseQuery(
-    std::string operation, const std::vector<DatasetClause>& datasets) {
-  auto parsedOperation = parseQuery(std::move(operation));
-  // SPARQL Protocol 2.1.4 specifies that the dataset from the query
-  // parameters overrides the dataset from the query itself.
-  if (!datasets.empty()) {
-    parsedOperation.datasetClauses_ =
-        parsedQuery::DatasetClauses::fromClauses(datasets);
-  }
-  return parsedOperation;
-}

src/parser/SparqlParser.h

@@ -13,8 +13,6 @@
 // message is given.
 class SparqlParser {
  public:
-  static ParsedQuery parseQuery(std::string query);
-  // A convenience function for parsing the query and setting the datasets.
-  static ParsedQuery parseQuery(std::string operation,
-                                const std::vector<DatasetClause>& datasets);
+  static ParsedQuery parseQuery(
+      std::string operation, const std::vector<DatasetClause>& datasets = {});
 };

src/parser/SparqlParserHelpers.cpp

@@ -16,10 +16,10 @@ using std::string;
 
 // _____________________________________________________________________________
 ParserAndVisitor::ParserAndVisitor(
-    std::string input,
+    std::string input, ParsedQuery::DatasetClauses datasetClauses,
     SparqlQleverVisitor::DisableSomeChecksOnlyForTesting disableSomeChecks)
     : input_{unescapeUnicodeSequences(std::move(input))},
-      visitor_{{}, disableSomeChecks} {
+      visitor_{{}, std::move(datasetClauses), disableSomeChecks} {
   // The default in ANTLR is to log all errors to the console and to continue
   // the parsing. We need to turn parse errors into exceptions instead to
   // propagate them to the user.
@@ -32,8 +32,10 @@ ParserAndVisitor::ParserAndVisitor(
 // _____________________________________________________________________________
 ParserAndVisitor::ParserAndVisitor(
     std::string input, SparqlQleverVisitor::PrefixMap prefixes,
+    ParsedQuery::DatasetClauses datasetClauses,
     SparqlQleverVisitor::DisableSomeChecksOnlyForTesting disableSomeChecks)
-    : ParserAndVisitor{std::move(input), disableSomeChecks} {
+    : ParserAndVisitor{std::move(input), std::move(datasetClauses),
+                       disableSomeChecks} {
   visitor_.setPrefixMapManually(std::move(prefixes));
 }
 

src/parser/SparqlParserHelpers.h

@@ -43,11 +43,12 @@ struct ParserAndVisitor {
   SparqlAutomaticParser parser_{&tokens_};
   SparqlQleverVisitor visitor_;
   explicit ParserAndVisitor(
-      string input,
+      string input, ParsedQuery::DatasetClauses datasetClauses = {},
       SparqlQleverVisitor::DisableSomeChecksOnlyForTesting disableSomeChecks =
           SparqlQleverVisitor::DisableSomeChecksOnlyForTesting::False);
   ParserAndVisitor(
       string input, SparqlQleverVisitor::PrefixMap prefixes,
+      ParsedQuery::DatasetClauses datasetClauses = {},
       SparqlQleverVisitor::DisableSomeChecksOnlyForTesting disableSomeChecks =
           SparqlQleverVisitor::DisableSomeChecksOnlyForTesting::False);
 

src/parser/sparqlParser/SparqlQleverVisitor.cpp

@@ -318,12 +318,23 @@ parsedQuery::BasicGraphPattern Visitor::toGraphPattern(
   return pattern;
 }
 
+// ____________________________________________________________________________________
+parsedQuery::DatasetClauses SparqlQleverVisitor::setAndGetDatasetClauses(
+    const std::vector<DatasetClause>& clauses) {
+  // TODO: is it a good idea to do this implicitly (the fields are nullopt) or
+  // should this be done explicitly (with a bool flag)?
+  if (!activeDatasetClauses_.defaultGraphs_.has_value() &&
+      !activeDatasetClauses_.namedGraphs_.has_value()) {
+    activeDatasetClauses_ = parsedQuery::DatasetClauses::fromClauses(clauses);
+  }
+  return activeDatasetClauses_;
+}
+
 // ____________________________________________________________________________________
 ParsedQuery Visitor::visit(Parser::ConstructQueryContext* ctx) {
   ParsedQuery query;
-  query.datasetClauses_ = parsedQuery::DatasetClauses::fromClauses(
-      visitVector(ctx->datasetClause()));
-  activeDatasetClauses_ = query.datasetClauses_;
+  query.datasetClauses_ =
+      setAndGetDatasetClauses(visitVector(ctx->datasetClause()));
   if (ctx->constructTemplate()) {
     query._clause = visit(ctx->constructTemplate())
                         .value_or(parsedQuery::ConstructClause{});
@@ -367,9 +378,8 @@ ParsedQuery Visitor::visit(Parser::DescribeQueryContext* ctx) {
   }
 
   // Parse the FROM and FROM NAMED clauses.
-  activeDatasetClauses_ = parsedQuery::DatasetClauses::fromClauses(
-      visitVector(ctx->datasetClause()));
-  describeClause.datasetClauses_ = activeDatasetClauses_;
+  describeClause.datasetClauses_ =
+      setAndGetDatasetClauses(visitVector(ctx->datasetClause()));
 
   // Parse the WHERE clause and construct a SELECT query from it. For `DESCRIBE
   // *`, add each visible variable as a resource to describe.
@@ -414,9 +424,8 @@ ParsedQuery Visitor::visit(Parser::DescribeQueryContext* ctx) {
 // ____________________________________________________________________________________
 ParsedQuery Visitor::visit(Parser::AskQueryContext* ctx) {
   parsedQuery_._clause = ParsedQuery::AskClause{};
-  parsedQuery_.datasetClauses_ = parsedQuery::DatasetClauses::fromClauses(
-      visitVector(ctx->datasetClause()));
-  activeDatasetClauses_ = parsedQuery_.datasetClauses_;
+  parsedQuery_.datasetClauses_ =
+      setAndGetDatasetClauses(visitVector(ctx->datasetClause()));
   visitWhereClause(ctx->whereClause(), parsedQuery_);
   // NOTE: It can make sense to have solution modifiers with an ASK query, for
   // example, a GROUP BY with a HAVING.
@@ -661,9 +670,9 @@ ParsedQuery Visitor::visit(Parser::ModifyContext* ctx) {
         }
       };
   AD_CORRECTNESS_CHECK(visibleVariables_.empty());
-  auto graphPattern = visit(ctx->groupGraphPattern());
   parsedQuery_.datasetClauses_ =
-      parsedQuery::DatasetClauses::fromClauses(visitVector(ctx->usingClause()));
+      setAndGetDatasetClauses(visitVector(ctx->usingClause()));
+  auto graphPattern = visit(ctx->groupGraphPattern());
   parsedQuery_._rootGraphPattern = std::move(graphPattern);
   parsedQuery_.registerVariablesVisibleInQueryBody(visibleVariables_);
   visibleVariables_.clear();
@@ -1263,9 +1272,8 @@ void Visitor::visit(Parser::PrefixDeclContext* ctx) {
 // ____________________________________________________________________________________
 ParsedQuery Visitor::visit(Parser::SelectQueryContext* ctx) {
   parsedQuery_._clause = visit(ctx->selectClause());
-  parsedQuery_.datasetClauses_ = parsedQuery::DatasetClauses::fromClauses(
-      visitVector(ctx->datasetClause()));
-  activeDatasetClauses_ = parsedQuery_.datasetClauses_;
+  parsedQuery_.datasetClauses_ =
+      setAndGetDatasetClauses(visitVector(ctx->datasetClause()));
   visitWhereClause(ctx->whereClause(), parsedQuery_);
   parsedQuery_.addSolutionModifiers(visit(ctx->solutionModifier()));
   return parsedQuery_;

src/parser/sparqlParser/SparqlQleverVisitor.h

@@ -109,11 +109,15 @@ class SparqlQleverVisitor {
 
  public:
   SparqlQleverVisitor() = default;
+  // If `datasetOverride` contains datasets, then the datasets in
+  // the operation itself are ignored. This is used for the datasets from the
+  // url parameters which override those in the operation.
   explicit SparqlQleverVisitor(
-      PrefixMap prefixMap,
+      PrefixMap prefixMap, ParsedQuery::DatasetClauses datasetOverride,
       DisableSomeChecksOnlyForTesting disableSomeChecksOnlyForTesting =
           DisableSomeChecksOnlyForTesting::False)
-      : prefixMap_{std::move(prefixMap)},
+      : activeDatasetClauses_{std::move(datasetOverride)},
+        prefixMap_{std::move(prefixMap)},
         disableSomeChecksOnlyForTesting_{disableSomeChecksOnlyForTesting} {}
 
   const PrefixMap& prefixMap() const { return prefixMap_; }
@@ -629,5 +633,10 @@ class SparqlQleverVisitor {
   static parsedQuery::BasicGraphPattern toGraphPattern(
       const ad_utility::sparql_types::Triples& triples);
 
+  // Set the datasets state of the visitor if no override is defined. Returns
+  // the currently active datasets.
+  parsedQuery::DatasetClauses setAndGetDatasetClauses(
+      const std::vector<DatasetClause>& clauses);
+
   FRIEND_TEST(SparqlParser, ensureExceptionOnInvalidGraphTerm);
 };

test/QueryPlannerTest.cpp

@@ -2968,6 +2968,10 @@ TEST(QueryPlanner, Exists) {
       filter);
   h::expect("Describe ?x FROM <g> { ?x ?y ?z FILTER EXISTS {?a ?b ?c}}",
             h::Describe(::testing::_, filter));
+  h::expect(
+      "DELETE { ?x <b> <c> } USING <g> WHERE { ?x ?y ?z FILTER EXISTS {?a ?b "
+      "?c}}",
+      filter);
 
   // Test the interaction of FROM NAMES with EXISTS
   auto varG = std::vector{Variable{"?g"}};