Added quite some unit tests, the next pass will be for the comments.

Signed-off-by: Johannes Kalmbach <[email protected]>

src/engine/Describe.cpp

@@ -88,7 +88,7 @@ static IdTable getNewBlankNodes(
   return result;
 }
 
-// TODO<joka921> Recursively follow the blank nodes etc.
+// TODO<joka921> Comment.
 void Describe::recursivelyAddBlankNodes(
     IdTable& finalResult, ad_utility::HashSetWithMemoryLimit<Id>& alreadySeen,
     IdTable blankNodes) {
@@ -150,6 +150,9 @@ ProtoResult Describe::computeResult([[maybe_unused]] bool requestLaziness) {
 
   // TODO<joka921> It might be, that the Column index is definitely not 0, we
   // have to store this separately.
+  // TODO<joka921> We have to (here, as well as in the recursion) also respect
+  // the GRAPHS by which the result is filtered ( + add unit tests for that
+  // case).
   auto join = ad_utility::makeExecutionTree<Join>(
       getExecutionContext(), valuesOp, std::move(indexScan), 0, 0);
 

src/engine/Describe.h

@@ -18,7 +18,7 @@ class Describe : public Operation {
            parsedQuery::Describe describe);
 
   // Getter for testing.
-  const auto& getDescribe() const {return describe_;}
+  const auto& getDescribe() const { return describe_; }
 
   std::vector<QueryExecutionTree*> getChildren() override;
 

src/parser/CMakeLists.txt

@@ -25,6 +25,8 @@ add_library(parser
         GraphPattern.cpp data/Variable.cpp
         Iri.cpp
         Literal.cpp
-        LiteralOrIri.cpp)
+        LiteralOrIri.cpp
+        DatasetClauses.cpp
+)
 qlever_target_link_libraries(parser sparqlParser parserData sparqlExpressions rdfEscaping re2::re2 util engine index)
 

src/parser/DatasetClauses.cpp

@@ -0,0 +1,19 @@
+// Copyright 2024, University of Freiburg
+// Chair of Algorithms and Data Structures
+// Author: Johannes Kalmbach <[email protected]>
+
+#include "DatasetClauses.h"
+
+// _____________________________________________________________________________
+parsedQuery::DatasetClauses parsedQuery::DatasetClauses::fromClauses(
+    const std::vector<DatasetClause>& clauses) {
+  DatasetClauses result;
+  for (auto& [dataset, isNamed] : clauses) {
+    auto& graphs = isNamed ? result.namedGraphs_ : result.defaultGraphs_;
+    if (!graphs.has_value()) {
+      graphs.emplace();
+    }
+    graphs.value().insert(dataset);
+  }
+  return result;
+}

src/parser/DatasetClauses.h

@@ -0,0 +1,23 @@
+// Copyright 2024, University of Freiburg
+// Chair of Algorithms and Data Structures
+// Author: Johannes Kalmbach <[email protected]>
+
+#pragma once
+#include <vector>
+
+#include "index/ScanSpecification.h"
+#include "parser/sparqlParser/DatasetClause.h"
+
+namespace parsedQuery {
+// A struct for the FROM and FROM NAMED clauses;
+struct DatasetClauses {
+  // FROM clauses.
+  ScanSpecificationAsTripleComponent::Graphs defaultGraphs_{};
+  // FROM NAMED clauses.
+  ScanSpecificationAsTripleComponent::Graphs namedGraphs_{};
+
+  static DatasetClauses fromClauses(const std::vector<DatasetClause>& clauses);
+
+  bool operator==(const DatasetClauses& other) const = default;
+};
+}  // namespace parsedQuery

src/parser/GraphPatternOperation.h

@@ -10,6 +10,7 @@
 
 #include "engine/PathSearch.h"
 #include "engine/sparqlExpressions/SparqlExpressionPimpl.h"
+#include "parser/DatasetClauses.h"
 #include "parser/GraphPattern.h"
 #include "parser/TripleComponent.h"
 #include "parser/data/Variable.h"
@@ -135,7 +136,7 @@ class Subquery {
 struct Describe {
   using VarOrIri = std::variant<TripleComponent::Iri, Variable>;
   std::vector<VarOrIri> resources_;
-  Dataset
+  DatasetClauses datasetClauses_;
   Subquery whereClause_;
 };
 

src/parser/ParsedQuery.cpp

@@ -23,20 +23,6 @@
 using std::string;
 using std::vector;
 
-// _____________________________________________________________________________
-parsedQuery::DatasetClauses parsedQuery::DatasetClauses::fromClauses(
-    const std::vector<DatasetClause>& clauses) {
-  DatasetClauses result;
-  for (auto& [dataset, isNamed] : clauses) {
-    auto& graphs = isNamed ? result.namedGraphs_ : result.defaultGraphs_;
-    if (!graphs.has_value()) {
-      graphs.emplace();
-    }
-    graphs.value().insert(dataset);
-  }
-  return result;
-}
-
 // _____________________________________________________________________________
 string SparqlPrefix::asString() const {
   std::ostringstream os;

src/parser/ParsedQuery.h

@@ -14,6 +14,7 @@
 #include "index/ScanSpecification.h"
 #include "parser/Alias.h"
 #include "parser/ConstructClause.h"
+#include "parser/DatasetClauses.h"
 #include "parser/GraphPattern.h"
 #include "parser/GraphPatternOperation.h"
 #include "parser/PropertyPath.h"
@@ -37,21 +38,6 @@
 using std::string;
 using std::vector;
 
-// Forward declaration
-struct DatasetClause;
-
-namespace parsedQuery {
-// A struct for the FROM and FROM NAMED clauses;
-struct DatasetClauses {
-  // FROM clauses.
-  ScanSpecificationAsTripleComponent::Graphs defaultGraphs_{};
-  // FROM NAMED clauses.
-  ScanSpecificationAsTripleComponent::Graphs namedGraphs_{};
-
-  static DatasetClauses fromClauses(const std::vector<DatasetClause>& clauses);
-};
-}  // namespace parsedQuery
-
 // Data container for prefixes
 class SparqlPrefix {
  public:

src/parser/sparqlParser/SparqlQleverVisitor.cpp

@@ -285,28 +285,36 @@ ParsedQuery Visitor::visit(Parser::DescribeQueryContext* ctx) {
     reportNotSupported(ctx, "DESCRIBE * is");
   }
 
+  std::vector<Variable> describedVariables;
   for (GraphTerm& spec : specs) {
     if (std::holds_alternative<Variable>(spec)) {
-      clause.resources_.push_back(std::get<Variable>(spec));
+      const auto& variable = std::get<Variable>(spec);
+      clause.resources_.push_back(variable);
+      describedVariables.push_back(variable);
     } else if (std::holds_alternative<Iri>(spec)) {
       auto iri =
           TripleComponent::Iri::fromIriref(std::get<Iri>(spec).toSparql());
       clause.resources_.push_back(std::move(iri));
     }
   }
 
-  parsedQuery_.datasetClauses_ = parsedQuery::DatasetClauses::fromClauses(
+  auto datasetClauses = parsedQuery::DatasetClauses::fromClauses(
       visitVector(ctx->datasetClause()));
+  clause.datasetClauses_ = datasetClauses;
   if (ctx->whereClause()) {
     auto [pattern, visibleVariables] = visit(ctx->whereClause());
     parsedQuery_._rootGraphPattern = std::move(pattern);
     parsedQuery_.registerVariablesVisibleInQueryBody(visibleVariables);
   }
 
   parsedQuery_.addSolutionModifiers(visit(ctx->solutionModifier()));
+
+  auto& selectClause = parsedQuery_.selectClause();
+  selectClause.setSelected(std::move(describedVariables));
   clause.whereClause_ = std::move(parsedQuery_);
   parsedQuery_ = ParsedQuery{};
   parsedQuery_._rootGraphPattern._graphPatterns.push_back(std::move(clause));
+  parsedQuery_.datasetClauses_ = datasetClauses;
   auto constructClause = ParsedQuery::ConstructClause{};
   using G = GraphTerm;
   using V = Variable;

test/QueryPlannerTest.cpp

@@ -1,7 +1,7 @@
 // Copyright 2015 - 2024, University of Freiburg
 // Chair of Algorithms and Data Structures
 // Authors: Björn Buchhold <[email protected]> [2015 - 2017]
-//          Johannes Kalmbach <kalmbachqcs.uni-freiburg.de> [2018 - 2024]
+//          Johannes Kalmbach <kalmbach@cs.uni-freiburg.de> [2018 - 2024]
 
 #include <gtest/gtest.h>
 
@@ -2151,10 +2151,18 @@ TEST(QueryPlanner, WarningsOnUnboundVariables) {
 
 // ___________________________________________________________________________
 TEST(QueryPlanner, Describe) {
-  // TODO<joka921> Properly match the describe clause.
+  // Note: We deliberately don't test the contents of the actual DESCRIBE
+  // clause, because they have been extensively tested already in
+  // `SparqlAntlrParserTest.cpp` where we have access to proper matchers for
+  // them.
   h::expect("DESCRIBE <x>", h::Describe(::testing::_, h::NeutralElement()));
   h::expect("DESCRIBE ?x", h::Describe(::testing::_, h::NeutralElement()));
   h::expect(
       "Describe ?y { ?y <p> <o>}",
       h::Describe(::testing::_, h::IndexScanFromStrings("?y", "<p>", "<o>")));
+  h::expect(
+      "Describe ?y FROM <g> { ?y <p> <o>}",
+      h::Describe(::testing::_, h::IndexScanFromStrings(
+                                    "?y", "<p>", "<o>", {},
+                                    ad_utility::HashSet<std::string>{"<g>"})));
 }

test/QueryPlannerTestHelpers.h

@@ -14,6 +14,7 @@
 #include "engine/Bind.h"
 #include "engine/CartesianProductJoin.h"
 #include "engine/CountAvailablePredicates.h"
+#include "engine/Describe.h"
 #include "engine/Filter.h"
 #include "engine/GroupBy.h"
 #include "engine/IndexScan.h"
@@ -22,7 +23,6 @@
 #include "engine/MultiColumnJoin.h"
 #include "engine/NeutralElementOperation.h"
 #include "engine/OptionalJoin.h"
-#include "engine/Describe.h"
 #include "engine/OrderBy.h"
 #include "engine/PathSearch.h"
 #include "engine/QueryExecutionTree.h"
@@ -194,8 +194,7 @@ inline auto CountAvailablePredicates =
     [](size_t subjectColumnIdx, const Variable& predicateVar,
        const Variable& countVar,
        const std::same_as<QetMatcher> auto&... childMatchers)
-        requires(sizeof...(childMatchers) <= 1)
-{
+        requires(sizeof...(childMatchers) <= 1) {
   return RootOperation<::CountAvailablePredicates>(AllOf(
       AD_PROPERTY(::CountAvailablePredicates, subjectColumnIndex,
                   Eq(subjectColumnIdx)),
@@ -388,7 +387,7 @@ constexpr auto OrderBy = [](const ::OrderBy::SortedVariables& sortedVariables,
 // Match a `UNION` operation.
 constexpr auto Union = MatchTypeAndOrderedChildren<::Union>;
 
-// Match a `DESCRIBE` operationl
+// Match a `DESCRIBE` operation
 inline QetMatcher Describe(
     const Matcher<const parsedQuery::Describe&> describeMatcher,
     const QetMatcher& childMatcher) {
@@ -397,6 +396,8 @@ inline QetMatcher Describe(
             AD_PROPERTY(::Describe, getDescribe, describeMatcher)));
 }
 
+// Match a `DISTINCT` operation
+
 //
 inline QetMatcher QetWithWarnings(
     const std::vector<std::string>& warningSubstrings,

test/SparqlAntlrParserTest.cpp

@@ -1367,19 +1367,46 @@ TEST(SparqlParser, Query) {
                          {Var{"?s"}, Var{"?p"}, Var{"?o"}}, "{ ?s ?p ?o }",
                          "PREFIX doof: <http://doof.org/>"))));
 
-  // DESCRIBE * queries are not yet supported.
-  expectQueryFails("DESCRIBE *");
-
-  expectQuery(
-      "DESCRIBE <x>",
-      m::ConstructQuery({{Var{"?subject"}, Var{"?predicate"}, Var{"?object"}}}),
-      m::GraphPattern(m::));
-
-      // Test the various places where warnings are added in a query.
-      expectQuery(
-          "SELECT ?x {} GROUP BY ?x ORDER BY ?y",
-          m::WarningsOfParsedQuery({"?x was used by GROUP BY",
-                                    "?y was used in an ORDER BY clause"}));
+  {
+    // DESCRIBE * queries are not yet supported.
+    expectQueryFails("DESCRIBE *");
+    using R = std::vector<parsedQuery::Describe::VarOrIri>;
+    auto i = [](const auto& x) { return TripleComponent::Iri::fromIriref(x); };
+    // The tested describe queries will always DESCRIBE <x> ?y <z>
+    R xyz{i("<x>"), Var{"?y"}, i("<z>")};
+
+    // A matcher for the subquery `SELECT ?y { ?y <is-a> ?v}`
+    // This subquery computes the values for `?y` that are to
+    // be described.
+    auto yIsAVMatcher = m::SelectQuery(
+        m::Select({Var{"?y"}}),
+        m::GraphPattern(m::Triples({{Var{"?y"}, "<is-a>", Var{"?v"}}})));
+
+    // A test with empty dataset clauses (no FROM and FROM NAMED).
+    expectQuery("DESCRIBE <x> ?y <z> { ?y <is-a> ?v }",
+                m::DescribeQuery(m::Describe(xyz, {}, yIsAVMatcher)));
+
+    // A test with nonempty dataset clauses.
+    // Note that the clauses are relevant for the retrieval of the values for
+    // `?y`, as well as for the actual descriptions, so they have to be part of
+    // the outer query as well as of the internals of the DESCRIBE operation.
+    using Graphs = ScanSpecificationAsTripleComponent::Graphs;
+    Graphs defaultGraphs;
+    Graphs namedGraphs;
+    defaultGraphs.emplace({i("<default-graph>")});
+    namedGraphs.emplace({i("<named-graph>")});
+    parsedQuery::DatasetClauses clauses{defaultGraphs, namedGraphs};
+    clauses.defaultGraphs_ = defaultGraphs;
+    clauses.namedGraphs_ = namedGraphs;
+    expectQuery(
+        "DESCRIBE <x> ?y <z> FROM <default-graph> FROM NAMED <named-graph>",
+        m::DescribeQuery(m::Describe(xyz, clauses, ::testing::_), defaultGraphs,
+                         namedGraphs));
+  }
+  // Test the various places where warnings are added in a query.
+  expectQuery("SELECT ?x {} GROUP BY ?x ORDER BY ?y",
+              m::WarningsOfParsedQuery({"?x was used by GROUP BY",
+                                        "?y was used in an ORDER BY clause"}));
   expectQuery("SELECT * { BIND (?a as ?b) }",
               m::WarningsOfParsedQuery(
                   {"?a was used in the expression of a BIND clause"}));

test/SparqlAntlrParserTestHelpers.h

@@ -14,6 +14,7 @@
 
 #include "engine/sparqlExpressions/SparqlExpressionPimpl.h"
 #include "parser/Alias.h"
+#include "parser/DatasetClauses.h"
 #include "parser/ParsedQuery.h"
 #include "parser/SparqlParserHelpers.h"
 #include "parser/TripleComponent.h"
@@ -676,11 +677,19 @@ inline auto Triples = [](const vector<SparqlTriple>& triples)
                testing::UnorderedElementsAreArray(triples)));
 };
 
-inline auto Describe = []()
+// Match a `Describe` clause.
+inline auto Describe = [](const std::vector<p::Describe::VarOrIri>& resources,
+                          const p::DatasetClauses& datasetClauses,
+                          const Matcher<const ParsedQuery&>& subquery)
     -> Matcher<const p::GraphPatternOperation&> {
-  return detail::GraphPatternOperation<p::Describe>(
-      AD_FIELD(p::BasicGraphPattern, _triples,
-               testing::UnorderedElementsAreArray(triples)));
+  using namespace ::testing;
+  auto getSubquery = [](const p::Subquery& subquery) -> const ParsedQuery& {
+    return subquery.get();
+  };
+  return detail::GraphPatternOperation<p::Describe>(AllOf(
+      AD_FIELD(p::Describe, resources_, Eq(resources)),
+      AD_FIELD(p::Describe, datasetClauses_, Eq(datasetClauses)),
+      AD_FIELD(p::Describe, whereClause_, ResultOf(getSubquery, subquery))));
 };
 
 namespace detail {
@@ -899,6 +908,19 @@ inline auto ConstructQuery(
       RootGraphPattern(m));
 }
 
+// A matcher for a `DescribeQuery`
+inline auto DescribeQuery(
+    const Matcher<const p::GraphPatternOperation&>& describeMatcher,
+    const ScanSpecificationAsTripleComponent::Graphs& defaultGraphs =
+        std::nullopt,
+    const ScanSpecificationAsTripleComponent::Graphs& namedGraphs =
+        std::nullopt) {
+  using Var = ::Variable;
+  return ConstructQuery({{Var{"?subject"}, Var{"?predicate"}, Var{"?object"}}},
+                        GraphPattern(describeMatcher), defaultGraphs,
+                        namedGraphs);
+}
+
 // _____________________________________________________________________________
 inline auto VisibleVariables =
     [](const std::vector<::Variable>& elems) -> Matcher<const ParsedQuery&> {