Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 9 commits into from
Feb 11, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion include/pika_define.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion src/pika_bit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/storage/src/redis_strings.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/string_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
40 changes: 40 additions & 0 deletions tests/unit/type/bitops.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tools/manifest_generator/include/pika_define.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion tools/pika-port/pika_port_3/pika_define.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
2 changes: 1 addition & 1 deletion tools/pika_migrate/include/pika_define.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
4 changes: 2 additions & 2 deletions tools/pika_migrate/src/pika_bit.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down
Loading