fix: delete with same ts (#3780)

4paradigm · Feb 27, 2024 · ad4eace · ad4eace
1 parent 76af71c
commit ad4eace
Show file tree

Hide file tree

Showing 5 changed files with 38 additions and 34 deletions.
diff --git a/.github/workflows/hybridse-ci.yml b/.github/workflows/hybridse-ci.yml
@@ -61,7 +61,7 @@ jobs:
 
       - name: Install dependencies
         run: |
-          brew install coreutils
+          brew install coreutils z3
 
       - name: Cache thirdparty
         uses: actions/cache@v3

diff --git a/cases/integration_test/dml/test_delete.yaml b/cases/integration_test/dml/test_delete.yaml
@@ -423,9 +423,9 @@ cases:
         rows:
           - [1,"aa",1,2,3,1.1,2.1,1590738989000,"2020-05-01",true]
           - [2,"bb",1,2,3,1.1,2.1,1590738989000,"2020-05-01",true]
-          - [3,"aa",1,2,3,1.1,2.1,1590738990000,"2020-05-01",true]
+          - [3,"aa",1,2,4,1.1,2.1,1590738990001,"2020-05-01",true]
           - [4,"cc",2,2,3,1.1,2.1,1590738990000,"2020-05-01",true]
-          - [5,"cc",2,2,3,1.1,2.1,1590738991000,"2020-05-01",true]
+          - [5,"cc",2,2,4,1.1,2.1,1590738991000,"2020-05-01",true]
     sqls:
       - delete from {0} where c1='aa';
       - select * from {0};
@@ -435,7 +435,7 @@ cases:
       rows:
         - [2,"bb",1,2,3,1.1,2.1,1590738989000,"2020-05-01",true]
         - [4,"cc",2,2,3,1.1,2.1,1590738990000,"2020-05-01",true]
-        - [5,"cc",2,2,3,1.1,2.1,1590738991000,"2020-05-01",true]
+        - [5,"cc",2,2,4,1.1,2.1,1590738991000,"2020-05-01",true]
   -
     id: 23
     desc: delete全部数据

diff --git a/src/cmd/sql_cmd_test.cc b/src/cmd/sql_cmd_test.cc
@@ -1248,6 +1248,40 @@ TEST_P(DBSDKTest, DeletetSameColIndex) {
                     });
 }
 
+TEST_P(DBSDKTest, DeletetSameTsCol) {
+    auto cli = GetParam();
+    sr = cli->sr;
+    std::string db_name = "test2";
+    std::string table_name = "test1";
+    std::string ddl =
+        "create table test1 (id int, c1 string, c2 smallint, c3 int, c4 bigint, c5 float,c6 double,"
+        "c7 timestamp,c8 date, c9 bool, INDEX(KEY=c7, ts=c7));";
+    ProcessSQLs(sr, {
+                        "set @@execute_mode = 'online'",
+                        absl::StrCat("create database ", db_name, ";"),
+                        absl::StrCat("use ", db_name, ";"),
+                        ddl,
+                    });
+    hybridse::sdk::Status status;
+    sr->ExecuteSQL(absl::StrCat("insert into ", table_name,
+                " values (1,\"aa\",1,2,3,1.1,2.1,1590738989000,\"2020-05-01\",true);"), &status);
+    sr->ExecuteSQL(absl::StrCat("insert into ", table_name,
+                " values (2,\"bb\",1,2,3,1.1,2.1,1590738989000,\"2020-05-01\",true);"), &status);
+    sr->ExecuteSQL(absl::StrCat("insert into ", table_name,
+                " values (3,\"aa\",1,2,3,1.1,2.1,1590738990000,\"2020-05-01\",true);"), &status);
+
+    auto res = sr->ExecuteSQL(absl::StrCat("select * from ", table_name, ";"), &status);
+    ASSERT_EQ(res->Size(), 3);
+    sr->ExecuteSQL(absl::StrCat("delete from ", table_name, " where c7=1590738989000;"), &status);
+    ASSERT_TRUE(status.IsOK()) << status.msg;
+    res = sr->ExecuteSQL(absl::StrCat("select * from ", table_name, ";"), &status);
+    ASSERT_EQ(res->Size(), 1);
+    ProcessSQLs(sr, {
+                        absl::StrCat("drop table ", table_name),
+                        absl::StrCat("drop database ", db_name),
+                    });
+}
+
 TEST_P(DBSDKTest, TestDelete) {
     auto cli = GetParam();
     sr = cli->sr;

diff --git a/src/storage/segment.cc b/src/storage/segment.cc
@@ -313,37 +313,8 @@ bool Segment::GetTsIdx(const std::optional<uint32_t>& idx, uint32_t* ts_idx) {
     return true;
 }
 
-bool Segment::Delete(const std::optional<uint32_t>& idx, const Slice& key, uint64_t ts) {
-    uint32_t ts_idx = 0;
-    if (!GetTsIdx(idx, &ts_idx)) {
-        return false;
-    }
-    void* entry = nullptr;
-    if (entries_->Get(key, entry) < 0 || entry == nullptr) {
-        return true;
-    }
-    KeyEntry* key_entry = nullptr;
-    if (ts_cnt_ == 1) {
-        key_entry = reinterpret_cast<KeyEntry*>(entry);
-    } else {
-        key_entry = reinterpret_cast<KeyEntry**>(entry)[ts_idx];
-    }
-    base::Node<uint64_t, DataBlock*>* data_node = nullptr;
-    {
-        std::lock_guard<std::mutex> lock(mu_);
-        data_node = key_entry->entries.Remove(ts);
-    }
-    if (data_node) {
-        node_cache_.AddSingleValueNode(ts_idx, gc_version_.load(std::memory_order_relaxed), data_node);
-    }
-    return true;
-}
-
 bool Segment::Delete(const std::optional<uint32_t>& idx, const Slice& key,
             uint64_t ts, const std::optional<uint64_t>& end_ts) {
-    if (end_ts.has_value() && end_ts.value() + 1 == ts) {
-        return Delete(idx, key, ts);
-    }
     uint32_t ts_idx = 0;
     if (!GetTsIdx(idx, &ts_idx)) {
         return false;

diff --git a/src/storage/segment.h b/src/storage/segment.h
@@ -81,7 +81,6 @@ class Segment {
     bool Put(const Slice& key, const std::map<int32_t, uint64_t>& ts_map, DataBlock* row, bool put_if_absent = false);
 
     bool Delete(const std::optional<uint32_t>& idx, const Slice& key);
-    bool Delete(const std::optional<uint32_t>& idx, const Slice& key, uint64_t ts);
     bool Delete(const std::optional<uint32_t>& idx, const Slice& key,
             uint64_t ts, const std::optional<uint64_t>& end_ts);