Code Review

src/engine/idTable/IdTableRow.h

@@ -95,8 +95,8 @@ class Row {
     return result;
   }
 
-  template <std::enable_if_t<std::is_same_v<T, Id>, bool> = true>
-  friend std::ostream& operator<<(std::ostream& os, const Row& idTableRow) {
+  friend std::ostream& operator<<(std::ostream& os, const Row& idTableRow)
+      requires(std::is_same_v<T, Id>) {
     os << "(";
     for (size_t i = 0; i < idTableRow.numColumns(); ++i) {
       os << idTableRow[i] << (i < idTableRow.numColumns() - 1 ? " " : ")");

src/global/IdTriple.h

@@ -14,8 +14,7 @@
 struct IdTriple {
   std::array<Id, 3> ids_;
 
-  explicit IdTriple()
-      : ids_({Id::makeUndefined(), Id::makeUndefined(), Id::makeUndefined()}){};
+  explicit IdTriple() : ids_(){};
   explicit IdTriple(
       const CompressedBlockMetadata::PermutedTriple& permutedTriple)
       : ids_({permutedTriple.col0Id_, permutedTriple.col1Id_,

src/index/LocatedTriples.cpp

@@ -107,60 +107,49 @@ IdTable LocatedTriplesPerBlock::mergeTriples(size_t blockIndex,
       return std::tie(row[0]) == std::tie(lt->triple_.ids_[2]);
     }
   };
-  auto writeTripleToResult = [&numIndexColumns, &result](auto resultEntry,
-                                                         auto& locatedTriple) {
+
+  auto row = block.begin();
+  auto locatedTriple = locatedTriples.begin();
+  auto resultEntry = result.begin();
+
+  auto writeTripleToResult = [&numIndexColumns, &result,
+                              &resultEntry](auto& locatedTriple) {
     for (size_t i = 0; i < numIndexColumns; i++) {
-      (*resultEntry)[i] = locatedTriple->triple_.ids_[3 - numIndexColumns + i];
+      (*resultEntry)[i] = locatedTriple.triple_.ids_[3 - numIndexColumns + i];
     }
     // Write UNDEF to any additional columns.
     for (size_t i = numIndexColumns; i < result.numColumns(); i++) {
       (*resultEntry)[i] = ValueId::makeUndefined();
     }
+    resultEntry++;
   };
 
-  auto row = block.begin();
-  auto locatedTriple = locatedTriples.begin();
-  auto resultEntry = result.begin();
-
   while (row != block.end() && locatedTriple != locatedTriples.end()) {
     if (cmpLt(locatedTriple, *row)) {
       // locateTriple < row
       if (locatedTriple->shouldTripleExist_) {
         // Insertion of a non-existent triple.
-        writeTripleToResult(resultEntry, locatedTriple);
-        resultEntry++;
-        locatedTriple++;
-      } else {
-        // Deletion of a non-existent triple.
-        locatedTriple++;
+        writeTripleToResult(*locatedTriple);
       }
+      locatedTriple++;
     } else if (cmpEq(locatedTriple, *row)) {
       // locateTriple == row
-      if (locatedTriple->shouldTripleExist_) {
-        // Insertion of an already existing triple.
-        locatedTriple++;
-      } else {
+      if (!locatedTriple->shouldTripleExist_) {
         // Deletion of an existing triple.
-        locatedTriple++;
         row++;
       }
+      locatedTriple++;
     } else {
       // locateTriple > row
       // The row is not deleted - copy it
       *resultEntry++ = *row++;
     }
   }
 
-  if (locatedTriple != locatedTriples.end()) {
-    AD_CORRECTNESS_CHECK(row == block.end());
-    while (locatedTriple != locatedTriples.end()) {
-      if (locatedTriple->shouldTripleExist_ == true) {
-        writeTripleToResult(resultEntry, locatedTriple);
-        ++resultEntry;
-      }
-      ++locatedTriple;
-    }
-  }
+  std::ranges::for_each(
+      std::ranges::subrange(locatedTriple, locatedTriples.end()) |
+          std::views::filter(&LocatedTriple::shouldTripleExist_),
+      writeTripleToResult);
   if (row != block.end()) {
     AD_CORRECTNESS_CHECK(locatedTriple == locatedTriples.end());
     while (row != block.end()) {

src/index/LocatedTriples.h

@@ -136,15 +136,16 @@ std::ostream& operator<<(std::ostream& os, const std::vector<IdTriple>& v);
 //
 // 1. The position is defined by the index of a block.
 //
-// 2. If there is a block, where the first triple is smaller and the last triple
-// is larger or equal, then that is the block.
+// 2. A triple belongs to the first block (with the smallest index) that also
+// contains at least one triple that is larger than or equal to the input
+// triple.
 //
-// 3. If the triple falls "between two blocks" (the last triple of the previous
-// block is smaller and the first triple of the next block is larger), then the
-// block is the next block.
+// 2.1. In particular, if the triple falls "between two blocks" (the last triple
+// of the previous block is smaller and the first triple of the next block is
+// larger), then the block is the next block.
 //
-// 4. As a special case of 3, if the triple is smaller than all triples in the
+// 2.2. In particular, if the triple is smaller than all triples in the
 // permutation, the position is the first position of the first block.
 //
-// 5. As a special case of 3, if the triple is larger than all triples in the
-// permutation, the block index is one after the largest block index.
+// 3. If the triple is larger than all triples in the permutation, the block
+// index is one after the largest block index.