diff --git a/src/concurrency_control/interface/scan/open_scan.cpp b/src/concurrency_control/interface/scan/open_scan.cpp index 8b96bf1f0..8d6a2be3a 100644 --- a/src/concurrency_control/interface/scan/open_scan.cpp +++ b/src/concurrency_control/interface/scan/open_scan.cpp @@ -34,6 +34,27 @@ inline Status find_open_scan_slot(session* const ti, // NOLINT return Status::WARN_MAX_OPEN_SCAN; } +constexpr Status check_empty_scan_range(const std::string_view l_key, const scan_endpoint l_end, + const std::string_view r_key, const scan_endpoint r_end) { + if (r_end == scan_endpoint::INF) { + return Status::OK; // if right end is inf, not empty + } + if (l_end == scan_endpoint::INF) { + // if left end is inf, not empty in most cases + if (r_end == scan_endpoint::EXCLUSIVE && r_key.empty()) { + return Status::WARN_NOT_FOUND; // the only exception + } + return Status::OK; + } + int cmp_key = l_key.compare(r_key); + if (cmp_key < 0) { return Status::OK; } // if left is less, not empty + if (cmp_key > 0) { return Status::WARN_NOT_FOUND; } // if left is greater, invalid range + // l_key == r_key + return (l_end == scan_endpoint::INCLUSIVE && r_end == scan_endpoint::INCLUSIVE) + ? Status::OK // single point, not empty + : Status::WARN_NOT_FOUND; // empty +} + /** * This is for some creating for this tx and consider other concurrent strand * thread. If that failed, this cleanup local effect, respect the result and @@ -292,8 +313,11 @@ Status open_scan_body(Token const token, Storage storage, // NOLINT nvec; constexpr std::size_t index_nvec_body{0}; constexpr std::size_t index_nvec_ptr{1}; - auto rc = scan(storage, l_key, l_end, r_key, r_end, max_size, scan_res, - &nvec, right_to_left); + auto rc = check_empty_scan_range(l_key, l_end, r_key, r_end); + + if (rc == Status::OK) { + rc = scan(storage, l_key, l_end, r_key, r_end, max_size, scan_res, &nvec, right_to_left); + } if (rc != Status::OK) { update_local_read_range_if_ltx(); return rc; diff --git a/test/concurrency_control/short_tx/scan/open_scan/short_open_scan_test.cpp b/test/concurrency_control/short_tx/scan/open_scan/short_open_scan_test.cpp index 82a321fff..636606b2d 100644 --- a/test/concurrency_control/short_tx/scan/open_scan/short_open_scan_test.cpp +++ b/test/concurrency_control/short_tx/scan/open_scan/short_open_scan_test.cpp @@ -9,6 +9,8 @@ #include "gtest/gtest.h" +#include "test_tool.h" + namespace shirakami::testing { using namespace shirakami; @@ -164,4 +166,32 @@ TEST_F(open_scan_test, multi_open_reading_values) { // NOLINT ASSERT_EQ(Status::OK, leave(s)); } +TEST_F(open_scan_test, empty_range) { + Storage storage{}; + create_storage("", storage); + Token s{}; + ASSERT_OK(enter(s)); + ASSERT_OK(tx_begin({s, transaction_options::transaction_type::SHORT})); + ASSERT_OK(insert(s, storage, "a", "val")); + ScanHandle handle{}; + ASSERT_EQ(Status::WARN_NOT_FOUND, + open_scan(s, storage, + "b", scan_endpoint::INCLUSIVE, + "a", scan_endpoint::INCLUSIVE, handle)); + ASSERT_EQ(Status::WARN_NOT_FOUND, + open_scan(s, storage, + "a", scan_endpoint::INCLUSIVE, + "a", scan_endpoint::EXCLUSIVE, handle)); + ASSERT_EQ(Status::WARN_NOT_FOUND, + open_scan(s, storage, + "a", scan_endpoint::EXCLUSIVE, + "a", scan_endpoint::EXCLUSIVE, handle)); + ASSERT_EQ(Status::WARN_NOT_FOUND, + open_scan(s, storage, + "a", scan_endpoint::EXCLUSIVE, + "a", scan_endpoint::INCLUSIVE, handle)); + ASSERT_OK(commit(s)); + ASSERT_OK(leave(s)); +} + } // namespace shirakami::testing diff --git a/test/misc_ut/scan_range_check_test.cpp b/test/misc_ut/scan_range_check_test.cpp new file mode 100644 index 000000000..4efdfa453 --- /dev/null +++ b/test/misc_ut/scan_range_check_test.cpp @@ -0,0 +1,84 @@ + +#include "shirakami/interface.h" + +#include "glog/logging.h" +#include "gtest/gtest.h" + +namespace shirakami { + extern Status check_empty_scan_range(const std::string_view l_key, const scan_endpoint l_end, + const std::string_view r_key, const scan_endpoint r_end); +} + +namespace shirakami::testing { + +using namespace shirakami; + +class scan_range_check_test : public ::testing::Test { +public: + static void call_once_f() { + google::InitGoogleLogging("shirakami-test-misc_ut-scan_range_check_test"); + // FLAGS_stderrthreshold = 0; + } + + void SetUp() override { std::call_once(init_google, call_once_f); } + + void TearDown() override {} + +private: + static inline std::once_flag init_google; +}; + +TEST_F(scan_range_check_test, regular) { + // regular non-empty pattern + // ["a", "a"]; X = "a" + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::INCLUSIVE, "a", scan_endpoint::INCLUSIVE)); + // ["a", "b"]; + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::INCLUSIVE, "b", scan_endpoint::INCLUSIVE)); + // ("a", "b") + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::EXCLUSIVE, "b", scan_endpoint::EXCLUSIVE)); + // ["a", "b") + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::INCLUSIVE, "b", scan_endpoint::EXCLUSIVE)); + // ("a", "b"] + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::EXCLUSIVE, "b", scan_endpoint::INCLUSIVE)); + // ["a", inf) + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::INCLUSIVE, "", scan_endpoint::INF)); + // ("a", inf) + EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::EXCLUSIVE, "", scan_endpoint::INF)); + // (-inf, "b"] + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::INF, "b", scan_endpoint::INCLUSIVE)); + // (-inf, "b") + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::INF, "b", scan_endpoint::EXCLUSIVE)); + // (-inf, inf); full scan + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::INF, "", scan_endpoint::INF)); + + // regular empty pattern + // ["a", "a") + EXPECT_EQ(Status::WARN_NOT_FOUND, check_empty_scan_range("a", scan_endpoint::INCLUSIVE, "a", scan_endpoint::EXCLUSIVE)); + + // valid but rare pattern + // ["", inf); X >= ""; ie. full scan + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::INCLUSIVE, "a", scan_endpoint::INF)); + // ("", inf); X > "" + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::EXCLUSIVE, "a", scan_endpoint::INF)); + // (-inf, ""); X < "" + EXPECT_EQ(Status::WARN_NOT_FOUND, check_empty_scan_range("", scan_endpoint::INF, "", scan_endpoint::EXCLUSIVE)); + // (-inf, ""]; X <= ""; ie. X = "" + EXPECT_EQ(Status::OK, check_empty_scan_range("", scan_endpoint::INF, "", scan_endpoint::INCLUSIVE)); +} + +TEST_F(scan_range_check_test, irregular) { + // invalid range + // ["b", "a"]; left > right + EXPECT_EQ(Status::WARN_NOT_FOUND, check_empty_scan_range("b", scan_endpoint::INCLUSIVE, "a", scan_endpoint::INCLUSIVE)); + // ("a", "a"); empty with two EXCLUSIVEs + EXPECT_EQ(Status::WARN_NOT_FOUND, check_empty_scan_range("a", scan_endpoint::EXCLUSIVE, "a", scan_endpoint::EXCLUSIVE)); + + // invalid use + // inf with non empty key; this implementation treats these as normal inf + // EXPECT_EQ(Status::OK, check_empty_scan_range("b", scan_endpoint::INF, "a", scan_endpoint::INCLUSIVE)); + // EXPECT_EQ(Status::OK, check_empty_scan_range("b", scan_endpoint::INCLUSIVE, "a", scan_endpoint::INF)); + // EXPECT_EQ(Status::OK, check_empty_scan_range("b", scan_endpoint::INF, "a", scan_endpoint::INF)); + // EXPECT_EQ(Status::OK, check_empty_scan_range("a", scan_endpoint::INF, "a", scan_endpoint::INF)); +} + +} // namespace shirakami::testing