fix: Change the maximum range of setbit's offset to fit redis (#2995)

* Change the maximum range of setbit's offset to fit redis --------- Co-authored-by: wuxianrong <[email protected]>
OpenAtomFoundation · Feb 11, 2025 · a3c1666 · a3c1666
1 parent 95d4be6
commit a3c1666
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 13 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -367,9 +367,9 @@ set(ZSTD_INCLUDE_DIR ${INSTALL_INCLUDEDIR})
 ExternalProject_Add(fmt
   DEPENDS
   URL
-  https://github.com/fmtlib/fmt/archive/refs/tags/7.1.0.tar.gz
+  https://github.com/fmtlib/fmt/archive/refs/tags/10.2.1.tar.gz 
   URL_HASH
-  MD5=32af902636d373641f4ef9032fc65b3a
+  MD5=dc09168c94f90ea890257995f2c497a5
   DOWNLOAD_NO_PROGRESS
   1
   UPDATE_COMMAND

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
@@ -241,7 +241,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 if (s_.IsInvalidArgument()) {
     res_.SetRes(CmdRes::kMultiKey);
   } else {

diff --git a/src/storage/src/redis_strings.cc b/src/storage/src/redis_strings.cc
@@ -1066,12 +1066,12 @@ Status Redis::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 {
@@ -1149,7 +1149,7 @@ Status Redis::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/integration/string_test.go b/tests/integration/string_test.go
@@ -277,7 +277,7 @@ var _ = Describe("String Commands", func() {
 			Expect(getBit.Err()).NotTo(HaveOccurred())
 			Expect(getBit.Val()).To(Equal(int64(0)))
 		})
-
+				
 		It("should GetRange", func() {
 			set := client.Set(ctx, "key", "This is a string", 0)
 			Expect(set.Err()).NotTo(HaveOccurred())

diff --git a/tests/unit/type/bitops.tcl b/tests/unit/type/bitops.tcl
@@ -131,6 +131,46 @@ start_server {tags {"bitops"}} {
         r get s
     } "\x55\xff\x00\xaa"
 
+    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 AND|OR|XOR don't change the string with single input key} {
         r set a "\x01\x02\xff"
         r bitop and res1 a

diff --git a/tools/manifest_generator/include/pika_define.h b/tools/manifest_generator/include/pika_define.h
@@ -292,7 +292,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-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());
   }