diff --git a/dbms/src/Storages/Page/V3/PageDirectory.cpp b/dbms/src/Storages/Page/V3/PageDirectory.cpp index e940b4155e1..e10b8dda2eb 100644 --- a/dbms/src/Storages/Page/V3/PageDirectory.cpp +++ b/dbms/src/Storages/Page/V3/PageDirectory.cpp @@ -47,7 +47,7 @@ namespace PS::V3 * VersionedPageEntries methods * ********************************/ -std::optional> VersionedPageEntries::getEntryNotSafe(UInt64 seq) const +std::shared_ptr VersionedPageEntries::getEntryNotSafe(UInt64 seq) const { // entries are sorted by , find the first one less than if (auto iter = MapUtils::findLess(entries, PageVersionType(seq + 1)); @@ -56,10 +56,10 @@ std::optional> VersionedPageEntries::getEntryNotSaf if (!iter->second.isDelete()) return iter->second.entryPtr(); } - return std::nullopt; + return nullptr; } -std::optional> VersionedPageEntries::getEntry(UInt64 seq) const +std::shared_ptr VersionedPageEntries::getEntry(UInt64 seq) const { auto page_lock = acquireLock(); return getEntryNotSafe(seq); @@ -405,9 +405,9 @@ PageIDAndEntryV3 PageDirectory::get(PageId page_id, const PageDirectorySnapshotP throw Exception(fmt::format("Entry [id={}] not exist!", page_id), ErrorCodes::PS_ENTRY_NOT_EXISTS); } - if (auto entry = iter->second->getEntry(snap->sequence); entry) + if (auto entry = iter->second->getEntry(snap->sequence); entry != nullptr) { - return PageIDAndEntryV3(page_id, *entry.value()); + return PageIDAndEntryV3(page_id, *entry); } throw Exception(fmt::format("Entry [id={}] [seq={}] not exist!", page_id, snap->sequence), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); @@ -442,9 +442,9 @@ PageIDAndEntriesV3 PageDirectory::get(const PageIds & page_ids, const PageDirect for (size_t idx = 0; idx < page_ids.size(); ++idx) { const auto & iter = iters[idx]; - if (auto entry = iter->second->getEntry(snap->sequence); entry) + if (auto entry = iter->second->getEntry(snap->sequence); entry != nullptr) { - id_entries.emplace_back(page_ids[idx], *entry.value()); + id_entries.emplace_back(page_ids[idx], *entry); } else throw Exception(fmt::format("Entry [id={}] [seq={}] at [idx={}] not exist!", page_ids[idx], snap->sequence, idx), ErrorCodes::PS_ENTRY_NO_VALID_VERSION); @@ -549,10 +549,10 @@ void PageDirectory::apply(PageEntriesEdit && edit) if (auto entry = (updating_locks.find(r.ori_page_id) != updating_locks.end() ? iter->second->getEntryNotSafe(last_sequence + 1) : iter->second->getEntry(last_sequence + 1)); - entry) + entry != nullptr) { // copy the entry shared ptr to be ref - std::shared_ptr entry_ptr_copy = entry.value(); + std::shared_ptr entry_ptr_copy = entry; updating_pages[idx]->createNewVersion(last_sequence + 1, std::move(entry_ptr_copy)); } else diff --git a/dbms/src/Storages/Page/V3/PageDirectory.h b/dbms/src/Storages/Page/V3/PageDirectory.h index a88c529127b..d14a8278801 100644 --- a/dbms/src/Storages/Page/V3/PageDirectory.h +++ b/dbms/src/Storages/Page/V3/PageDirectory.h @@ -68,26 +68,26 @@ struct EntryOrDelete { public: explicit EntryOrDelete(bool del) - : entry_or_delete(std::nullopt) + : entry_or_delete(nullptr) { assert(del == true); } explicit EntryOrDelete(std::shared_ptr && entry) : entry_or_delete(std::move(entry)) { - assert(entry_or_delete.value() != nullptr); + assert(entry_or_delete != nullptr); } - inline bool isDelete() const { return !entry_or_delete.has_value(); } + inline bool isDelete() const { return entry_or_delete == nullptr; } - const std::shared_ptr & entryPtr() const + inline const std::shared_ptr & entryPtr() const { - assert(!isDelete()); - return entry_or_delete.value(); + assert(entry_or_delete != nullptr); + return entry_or_delete; } private: - std::optional> entry_or_delete; + std::shared_ptr entry_or_delete; }; using PageLock = std::unique_ptr>; @@ -116,10 +116,10 @@ class VersionedPageEntries // Return the shared_ptr to the entry that is visible by `seq`. // If the entry is not visible (deleted or not exist for `seq`), then - // return `std::nullopt`. - std::optional> getEntry(UInt64 seq) const; + // return `nullptr`. + std::shared_ptr getEntry(UInt64 seq) const; - std::optional> getEntryNotSafe(UInt64 seq) const; + std::shared_ptr getEntryNotSafe(UInt64 seq) const; std::shared_ptr getLatestEntryNotSafe() const; diff --git a/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp b/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp index 68b6742c9b4..f9ebc017dbe 100644 --- a/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp +++ b/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp @@ -499,54 +499,54 @@ TEST_F(VersionedEntriesTest, InsertGet) INSERT_ENTRY(10); // Insert some entries with version - ASSERT_FALSE(entries.getEntry(1).has_value()); - ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_v2)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_v2)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_v2)); + ASSERT_FALSE(entries.getEntry(1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_v2)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_v2)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_v2)); for (UInt64 seq = 5; seq < 10; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v5)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v5)); } for (UInt64 seq = 10; seq < 20; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10)); } // Insert some entries with version && gc epoch INSERT_GC_ENTRY(2, 1); INSERT_GC_ENTRY(5, 1); INSERT_GC_ENTRY(5, 2); - ASSERT_FALSE(entries.getEntry(1).has_value()); - ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_gc_v2_1)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_gc_v2_1)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_gc_v2_1)); + ASSERT_FALSE(entries.getEntry(1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_gc_v2_1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_gc_v2_1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_gc_v2_1)); for (UInt64 seq = 5; seq < 10; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_gc_v5_2)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_gc_v5_2)); } for (UInt64 seq = 10; seq < 20; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10)); } // Insert delete. Can not get entry with seq >= delete_version. // But it won't affect reading with old seq. entries.createDelete(15); - ASSERT_FALSE(entries.getEntry(1).has_value()); - ASSERT_TRUE(isSameEntry(**entries.getEntry(2), entry_gc_v2_1)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(3), entry_gc_v2_1)); - ASSERT_TRUE(isSameEntry(**entries.getEntry(4), entry_gc_v2_1)); + ASSERT_FALSE(entries.getEntry(1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(2), entry_gc_v2_1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(3), entry_gc_v2_1)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(4), entry_gc_v2_1)); for (UInt64 seq = 5; seq < 10; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_gc_v5_2)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_gc_v5_2)); } for (UInt64 seq = 10; seq < 15; ++seq) { - ASSERT_TRUE(isSameEntry(**entries.getEntry(seq), entry_v10)); + ASSERT_TRUE(isSameEntry(*entries.getEntry(seq), entry_v10)); } for (UInt64 seq = 15; seq < 20; ++seq) { - ASSERT_FALSE(entries.getEntry(seq).has_value()); + ASSERT_FALSE(entries.getEntry(seq)); } } @@ -609,13 +609,13 @@ try INSERT_ENTRY(5); // Read with snapshot seq=2 - ASSERT_TRUE(isSameEntry(entry_v2, **entries.getEntry(2))); + ASSERT_TRUE(isSameEntry(entry_v2, *entries.getEntry(2))); // Mock that gc applied and insert <2, 1> INSERT_GC_ENTRY(2, 1); // Now we should read the entry <2, 1> with seq=2 - ASSERT_TRUE(isSameEntry(entry_gc_v2_1, **entries.getEntry(2))); + ASSERT_TRUE(isSameEntry(entry_gc_v2_1, *entries.getEntry(2))); // <2,0> get removed auto removed_entries = gcAndGetRemovedEntries(2);