range-check-before-yakushima-scan

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;

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

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