fix: Change the maximum range of setbit's offset to fit redis v3.5 (#…

…3007) * Change the maximum range of setbit's offset to fit redis --------- Co-authored-by: wuxianrong <[email protected]>
OpenAtomFoundation · Feb 11, 2025 · 3b316bc · 3b316bc
1 parent 80cceb3
commit 3b316bc
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 14 deletions.
diff --git a/.github/workflows/pika.yml b/.github/workflows/pika.yml
@@ -50,7 +50,7 @@ jobs:
           rm -rf  ./deps
           rm -rf ./buildtrees
 
-      - uses: actions/upload-artifact@v3
+      - uses: actions/upload-artifact@v4
         with:
           name: ${{ env.ARTIFACT_PIKA_NAME }}
           path: ${{ github.workspace }}/build/pika
@@ -242,7 +242,7 @@ jobs:
       - name: Install Deps
         run: |
           brew update
-          brew install --overwrite python@3.12 autoconf protobuf llvm wget git
+          brew install --overwrite python@3.13 autoconf protobuf llvm wget git
           brew install gcc@10 automake cmake make binutils
 
       - name: Configure CMake
@@ -303,7 +303,7 @@ jobs:
         with:
           images: pikadb/pika
 
-      - uses: actions/download-artifact@v3
+      - uses: actions/download-artifact@v4
         with:
           name: ${{ env.ARTIFACT_PIKA_NAME }}
           path: artifact/

diff --git a/include/pika_define.h b/include/pika_define.h
@@ -366,7 +366,7 @@ const std::string kInnerReplOk = "ok";
 const std::string kInnerReplWait = "wait";
 
 const unsigned int kMaxBitOpInputKey = 12800;
-const int kMaxBitOpInputBit = 21;
+const int kMaxBitOpInputBit = 32;
 /*
  * db sync
  */

diff --git a/src/pika_bit.cc b/src/pika_bit.cc
@@ -31,7 +31,7 @@ void BitSetCmd::DoInitial() {
     res_.SetRes(CmdRes::kInvalidBitOffsetInt);
     return;
   }
-  // value no bigger than 2^18
+  // value no bigger than 2^32
   if ((bit_offset_ >> kMaxBitOpInputBit) > 0) {
     res_.SetRes(CmdRes::kInvalidBitOffsetInt);
     return;
@@ -237,7 +237,7 @@ void BitPosCmd::Do() {
     s_ = db_->storage()->BitPos(key_, static_cast<int32_t>(bit_val_), start_offset_, end_offset_, &pos);
   }
   if (s_.ok()) {
-    res_.AppendInteger(static_cast<int>(pos));
+    res_.AppendInteger(pos);
   } else {
     res_.SetRes(CmdRes::kErrOther, s_.ToString());
   }

diff --git a/src/storage/src/redis_strings.cc b/src/storage/src/redis_strings.cc
@@ -973,12 +973,12 @@ Status RedisStrings::Strlen(const Slice& key, int32_t* len) {
   return s;
 }
 
-int32_t GetBitPos(const unsigned char* s, unsigned int bytes, int bit) {
+int64_t GetBitPos(const unsigned char* s, unsigned int bytes, int bit) {
   uint64_t word = 0;
   uint64_t skip_val = 0;
   auto value = const_cast<unsigned char*>(s);
   auto l = reinterpret_cast<uint64_t*>(value);
-  int pos = 0;
+  int64_t pos = 0;
   if (bit == 0) {
     skip_val = std::numeric_limits<uint64_t>::max();
   } else {
@@ -1044,7 +1044,7 @@ Status RedisStrings::BitPos(const Slice& key, int32_t bit, int64_t* ret) {
         pos = -1;
       }
       if (pos != -1) {
-        pos = pos + 8 * start_offset;
+        pos += 8 * start_offset;
       }
       *ret = pos;
     }

diff --git a/tests/unit/type/bitops.tcl b/tests/unit/type/bitops.tcl
@@ -146,6 +146,46 @@ start_server {tags {"bitops"}} {
         r bitop xor res3 no-such-key a
         list [r get res1] [r get res2] [r get res3]
     } [list "\x00\x00\x00" "\x01\x02\xff" "\x01\x02\xff"]
+
+    test {SetBit and GetBit with large offset} {
+        set max_offset [expr {2**32 - 1}]
+        set invalid_offset [expr {2**32}]
+
+        r setbit large_key $max_offset 1
+        set result [r getbit large_key $max_offset]
+        set invalid_result [catch {r setbit large_key $invalid_offset 1} err]
+
+        list $result $invalid_result $err
+    } {1 1 {ERR bit offset is not an integer or out of range}}
+
+    test {BITCOUNT with large offset} {
+        r setbit count_key 0 1
+        r setbit count_key 100 1
+        r setbit count_key [expr {2**32 - 1}] 1
+
+        set total_count [r bitcount count_key]
+        set range_count [r bitcount count_key 0 12]
+
+        list $total_count $range_count
+    } {3 2}
+
+    test {BITPOS with large offset} {
+        r setbit pos_key [expr {2**32 - 1}] 1
+        set first_one [r bitpos pos_key 1]
+        set first_zero [r bitpos pos_key 0]
+        list $first_one $first_zero
+    } {4294967295 0}
+
+    test {BITOP operations} {
+        r setbit key1 0 1
+        r setbit key2 [expr {2**32 - 1}] 1
+        r bitop or result_key key1 key2
+
+        set result_bit1 [r getbit result_key 0]
+        set result_bit2 [r getbit result_key [expr {2**32 - 1}]]
+
+        list $result_bit1 $result_bit2
+    } {1 1}
 
     test {BITOP shorter keys are zero-padded to the key with max length} {
         r set a "\x01\x02\xff\xff"

diff --git a/tools/pika-port/pika_port_3/pika_define.h b/tools/pika-port/pika_port_3/pika_define.h
@@ -117,7 +117,7 @@ const std::string kInnerReplOk = "ok";
 const std::string kInnerReplWait = "wait";
 
 const unsigned int kMaxBitOpInputKey = 12800;
-const int kMaxBitOpInputBit = 21;
+const int kMaxBitOpInputBit = 32;
 /*
  * db sync
  */

diff --git a/tools/pika_migrate/include/pika_define.h b/tools/pika_migrate/include/pika_define.h
@@ -413,7 +413,7 @@ const std::string kInnerReplOk = "ok";
 const std::string kInnerReplWait = "wait";
 
 const unsigned int kMaxBitOpInputKey = 12800;
-const int kMaxBitOpInputBit = 21;
+const int kMaxBitOpInputBit = 32;
 /*
  * db sync
  */

diff --git a/tools/pika_migrate/src/pika_bit.cc b/tools/pika_migrate/src/pika_bit.cc
@@ -27,7 +27,7 @@ void BitSetCmd::DoInitial() {
     res_.SetRes(CmdRes::kInvalidBitOffsetInt);
     return;
   }
-  // value no bigger than 2^18
+  // value no bigger than 2^32
   if ( (bit_offset_ >> kMaxBitOpInputBit) > 0) {
     res_.SetRes(CmdRes::kInvalidBitOffsetInt);
     return;
@@ -168,7 +168,7 @@ void BitPosCmd::Do(std::shared_ptr<Partition> partition) {
     s = partition->db()->BitPos(key_, bit_val_, start_offset_, end_offset_, &pos);
   }
   if (s.ok()) {
-    res_.AppendInteger((int)pos);
+    res_.AppendInteger(pos);
   } else {
     res_.SetRes(CmdRes::kErrOther, s.ToString());
   }

diff --git a/tools/pika_migrate/src/pika_client_conn.cc b/tools/pika_migrate/src/pika_client_conn.cc
@@ -61,7 +61,6 @@ std::string PikaClientConn::DoCmd(const PikaCmdArgsType& argv,
   }
 
   g_pika_server->UpdateQueryNumAndExecCountTable(opt);
-
   // PubSub connection
   // (P)SubscribeCmd will set is_pubsub_
   if (this->IsPubSub()) {