Fix the bug, and add a first unit test.

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

src/engine/Describe.cpp

@@ -65,8 +65,9 @@ VariableToColumnMap Describe::computeVariableToColumnMap() const {
           {V("?object"), col(2)}};
 }
 
-static IdTable getNewBlankNodes(const auto& allocator, ad_utility::HashSetWithMemoryLimit<Id>& alreadySeen,
-std::span<Id>  input) {
+static IdTable getNewBlankNodes(
+    const auto& allocator, ad_utility::HashSetWithMemoryLimit<Id>& alreadySeen,
+    std::span<Id> input) {
   IdTable result{1, allocator};
   result.resize(input.size());
   decltype(auto) col = result.getColumn(0);
@@ -75,8 +76,8 @@ std::span<Id>  input) {
     if (id.getDatatype() != Datatype::BlankNodeIndex) {
       continue;
     }
-    auto [it, wasContained] = alreadySeen.emplace(id);
-    if (wasContained) {
+    auto [it, isNew] = alreadySeen.emplace(id);
+    if (!isNew) {
       continue;
     }
     col[i] = id;
@@ -108,7 +109,8 @@ void Describe::recursivelyAddBlankNodes(
       getExecutionContext(), valuesOp, std::move(indexScan), 0, 0);
 
   auto result = join->getResult();
-  // TODO<joka921> A lot of those things are inefficient, lets get it working first.
+  // TODO<joka921> A lot of those things are inefficient, lets get it working
+  // first.
   auto table = result->idTable().clone();
   finalResult.reserve(finalResult.size() + table.size());
   auto s = join->getVariableColumn(V{"?subject"});
@@ -117,7 +119,8 @@ void Describe::recursivelyAddBlankNodes(
   table.setColumnSubset(std::vector{s, p, o});
   finalResult.insertAtEnd(table);
 
-  auto newBlankNodes = getNewBlankNodes(allocator(), alreadySeen, table.getColumn(2));
+  auto newBlankNodes =
+      getNewBlankNodes(allocator(), alreadySeen, table.getColumn(2));
   // recurse
   recursivelyAddBlankNodes(finalResult, alreadySeen, std::move(newBlankNodes));
 }
@@ -160,7 +163,8 @@ ProtoResult Describe::computeResult([[maybe_unused]] bool requestLaziness) {
 
   // Recursively follow blank nodes
   ad_utility::HashSetWithMemoryLimit<Id> alreadySeen{allocator()};
-  auto blankNodes = getNewBlankNodes(allocator(), alreadySeen, resultTable.getColumn(2));
+  auto blankNodes =
+      getNewBlankNodes(allocator(), alreadySeen, resultTable.getColumn(2));
   recursivelyAddBlankNodes(resultTable, alreadySeen, std::move(blankNodes));
   return {std::move(resultTable), resultSortedOn(),
           result->getSharedLocalVocab()};

src/engine/Describe.h

@@ -5,6 +5,7 @@
 #pragma once
 
 #include "engine/Operation.h"
+#include "parser/GraphPatternOperation.h"
 
 class Describe : public Operation {
  private:

test/engine/CMakeLists.txt

@@ -11,3 +11,4 @@ addLinkAndDiscoverTest(CountConnectedSubgraphsTest)
 addLinkAndDiscoverTest(BindTest engine)
 addLinkAndDiscoverTest(SpatialJoinAlgorithmsTest engine)
 addLinkAndDiscoverTestSerial(QueryExecutionTreeTest engine)
+addLinkAndDiscoverTestSerial(DescribeTest engine)

test/engine/DescribeTest.cpp

@@ -0,0 +1,47 @@
+//  Copyright 2024, University of Freiburg,
+//                  Chair of Algorithms and Data Structures.
+//  Author: Johannes Kalmbach <[email protected]>
+
+#include <gmock/gmock.h>
+
+#include "../util/IndexTestHelpers.h"
+#include "engine/Describe.h"
+#include "engine/NeutralElementOperation.h"
+#include "engine/QueryExecutionTree.h"
+
+using namespace ad_utility::testing;
+
+// _____________________________________________________________________________
+TEST(Describe, recursiveBlankNodes) {
+  auto qec = getQec(
+      "<s> <p> <o> . <s> <p> _:g1 . _:g1 <p2> <o2> . _:g1 <p2> _:g1 . _:g1 "
+      "<p2> _:g2 . _:g2 <p> <o4>. <s2> <p> <o> . _:g4 <p> _:g5");
+  parsedQuery::Describe parsedDescribe;
+  parsedDescribe.resources_.push_back(TripleComponent::Iri::fromIriref("<s>"));
+  Describe describe{qec,
+                    ad_utility::makeExecutionTree<NeutralElementOperation>(qec),
+                    parsedDescribe};
+  auto res = describe.computeResultOnlyForTesting();
+  const auto& table = res.idTable();
+  /* The expected result is:
+     <s> <p> <o>
+     <s> <p> _:g1
+     _:g1 <p2> <o2>
+     _:g1 <p2> _:g1
+     _:g1 <p2> _:g2
+     _:g2 <p> <o4>
+  */
+  // As the blank nodes are renamed, we cannot easily assert the exact result,
+  // but we can at least check the statisticts.
+  EXPECT_EQ(table.size(), 6);
+  auto numUnique = [](size_t size) {
+    using namespace ::testing;
+    auto getNumUnique = [](const auto& col) {
+      return ad_utility::HashSet<Id>(col.begin(), col.end()).size();
+    };
+    return ResultOf(getNumUnique, size);
+  };
+  EXPECT_THAT(table.getColumn(0), numUnique(3));
+  EXPECT_THAT(table.getColumn(1), numUnique(2));
+  EXPECT_THAT(table.getColumn(2), numUnique(5));
+}