Address review comments

facebook · Feb 6, 2024 · 2c3943d · 2c3943d
1 parent 0d92d96
commit 2c3943d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/db/db_compaction_test.cc b/db/db_compaction_test.cc
@@ -2210,7 +2210,7 @@ TEST_P(DBDeleteFileRangeTest, DeleteFileRangeFileEndpointsOverlapBug) {
 }
 
 INSTANTIATE_TEST_CASE_P(DBDeleteFileRangeTest, DBDeleteFileRangeTest,
-                        ::testing::Values(true, false));
+                        ::testing::Boolean());
 
 TEST_P(DBCompactionTestWithParam, TrivialMoveToLastLevelWithFiles) {
   int32_t trivial_move = 0;

diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc
@@ -4918,11 +4918,9 @@ Status DBImpl::DeleteFilesInRanges(ColumnFamilyHandle* column_family,
     auto [start, limit] = MaybeAddTimestampsToRange(
         ranges[i].start, ranges[i].limit, ts_sz, &keys.emplace_back(),
         &keys.emplace_back(), !include_end);
-    assert(ranges[i].start == nullptr || start.has_value());
-    assert(ranges[i].limit == nullptr || limit.has_value());
-    ukey_ranges.emplace_back(
-        start.has_value() ? &key_slices.emplace_back(start.value()) : nullptr,
-        limit.has_value() ? &key_slices.emplace_back(limit.value()) : nullptr);
+    assert((ranges[i].start != nullptr) == start.has_value());
+    assert((ranges[i].limit != nullptr) == limit.has_value());
+    ukey_ranges.emplace_back(start, limit);
   }
 
   VersionEdit edit;
@@ -4934,7 +4932,8 @@ Status DBImpl::DeleteFilesInRanges(ColumnFamilyHandle* column_family,
 
     auto* vstorage = input_version->storage_info();
     for (const auto& range : ukey_ranges) {
-      auto begin = range.start, end = range.limit;
+      auto begin = range.start.has_value() ? &range.start.value() : nullptr;
+      auto end = range.limit.has_value() ? &range.limit.value() : nullptr;
       for (int i = 1; i < cfd->NumberLevels(); i++) {
         if (vstorage->LevelFiles(i).empty() ||
             !vstorage->OverlapInLevel(i, begin, end)) {

diff --git a/db/dbformat.h b/db/dbformat.h
@@ -11,6 +11,7 @@
 #include <stdio.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 
@@ -85,7 +86,7 @@ struct UserKeyRange {
   Slice start;
   Slice limit;
 
-  UserKeyRange() {}
+  UserKeyRange() = default;
   UserKeyRange(const Slice& s, const Slice& l) : start(s), limit(l) {}
 };
 
@@ -94,11 +95,13 @@ struct UserKeyRange {
 struct UserKeyRangePtr {
   // In case of user_defined timestamp, if enabled, `start` and `limit` should
   // point to key with timestamp part.
-  const Slice* start;
-  const Slice* limit;
+  // An optional range start, if missing, indicating a start before all keys.
+  std::optional<Slice> start;
+  // An optional range end, if missing, indicating an end after all keys.
+  std::optional<Slice> limit;
 
-  UserKeyRangePtr() : start(nullptr), limit(nullptr) {}
-  UserKeyRangePtr(const Slice* s, const Slice* l) : start(s), limit(l) {}
+  UserKeyRangePtr(const std::optional<Slice> s, const std::optional<Slice> l)
+      : start(s), limit(l) {}
 };
 
 // Checks whether a type is an inline value type