This is an automated email from the ASF dual-hosted git repository.
jihuayu pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new cbafb7a91 feat(bit): add BYTE/BIT option support for BITPOS command
(#3460)
cbafb7a91 is described below
commit cbafb7a9142d234759fbdd9db1938757cb28f95f
Author: gongna-au <[email protected]>
AuthorDate: Wed May 6 11:45:15 2026 +0800
feat(bit): add BYTE/BIT option support for BITPOS command (#3460)
## Summary
This PR adds support for the `BYTE`/`BIT` unit option to the `BITPOS`
command, aligning it with the Redis 7.0 specification.
The previous implementation only checked for `BIT` at a fixed argument
position (`args[5]`) and ignored `BYTE` entirely. This PR refactors the
parser to correctly handle the full Redis 7.0 syntax:
```
BITPOS key bit [start [end [BYTE | BIT]]]
```
The `BYTE|BIT` unit is strictly nested: it can only appear after both
`start` and `end` are provided, matching Redis behavior exactly.
## Changes
### 1. Command Parser Refactor (`src/commands/cmd_bit.cc`)
- Refactored `CommandBitPos::Parse` to cleanly separate argument parsing
by count.
- `args[4]` (the 5th argument) is always parsed as the `end` integer —
never as a unit keyword. This matches Redis, which rejects `BITPOS key 1
0 BIT` with a "not an integer" error.
- `args[5]` (the 6th argument) is checked for `BIT`/`BYTE` keyword, with
`errInvalidSyntax` on anything else.
- Added explicit `BYTE` recognition (previously only `BIT` was partially
handled).
- Moved `bit` argument validation to the top for clarity.
- Replaced verbose `ParseInt` + manual error check with `GET_OR_RET`
macro for consistency.
### 2. Storage Layer — No Changes Needed
- The `CHECK(stop_given)` assertion in `Bitmap::BitPos` is preserved as
a defensive guard. Since the parser guarantees `stop_given=true`
whenever `is_bit_index=true` (unit keyword requires `end` to be
present), this assertion is correct and protects against future
regressions.
## Compatibility
| Command Form | Redis 7.0 | Kvrocks (after PR) |
| --- | --- | --- |
| `BITPOS key bit` | OK | OK |
| `BITPOS key bit start` | OK | OK |
| `BITPOS key bit start end` | OK | OK |
| `BITPOS key bit start end BYTE` | OK (since 7.0) | **OK (new)** |
| `BITPOS key bit start end BIT` | OK (since 7.0) | **OK (new)** |
| `BITPOS key bit start BIT` | ERR not integer | ERR not integer |
## Verification
- Argument parsing logic verified against Redis 7.0 syntax definition:
`BITPOS key bit [start [end [BYTE | BIT]]]`.
- `is_bit_index` is only set when `args.size() == 6`, guaranteeing
`stop_given_ == true`, so the `CHECK(stop_given)` assertion in the
storage layer is always satisfied.
- `BITCOUNT` already has correct `BYTE/BIT` support and is not affected
by this PR.
Co-authored-by: gongna-au <[email protected]>
Co-authored-by: Sisyphus <[email protected]>
Co-authored-by: 纪华裕 <[email protected]>
---
src/commands/cmd_bit.cc | 44 +++---
tests/gocase/unit/type/bitmap/bitmap_test.go | 217 +++++++++++++++++++++++++++
2 files changed, 235 insertions(+), 26 deletions(-)
diff --git a/src/commands/cmd_bit.cc b/src/commands/cmd_bit.cc
index eb829d31c..ba9f721e9 100644
--- a/src/commands/cmd_bit.cc
+++ b/src/commands/cmd_bit.cc
@@ -158,39 +158,31 @@ class CommandBitPos : public Commander {
using Commander::Parse;
Status Parse(const std::vector<std::string> &args) override {
- if (args.size() >= 4) {
- auto parse_start = ParseInt<int64_t>(args[3], 10);
- if (!parse_start) {
- return {Status::RedisParseErr, errValueNotInteger};
- }
+ auto parse_bit = ParseInt<int>(args[2], 10);
+ if (!parse_bit || (*parse_bit != 0 && *parse_bit != 1)) {
+ return {Status::RedisParseErr, "The bit argument must be 1 or 0."};
+ }
+ bit_ = (*parse_bit == 1);
- start_ = *parse_start;
+ if (args.size() >= 4) {
+ start_ = GET_OR_RET(ParseInt<int64_t>(args[3], 10));
}
if (args.size() >= 5) {
- auto parse_stop = ParseInt<int64_t>(args[4], 10);
- if (!parse_stop) {
- return {Status::RedisParseErr, errValueNotInteger};
- }
-
+ stop_ = GET_OR_RET(ParseInt<int64_t>(args[4], 10));
stop_given_ = true;
- stop_ = *parse_stop;
}
- if (args.size() >= 6 && util::EqualICase(args[5], "BIT")) {
- is_bit_index_ = true;
- }
-
- auto parse_arg = ParseInt<int64_t>(args[2], 10);
- if (!parse_arg) {
- return {Status::RedisParseErr, errValueNotInteger};
- }
- if (*parse_arg == 0) {
- bit_ = false;
- } else if (*parse_arg == 1) {
- bit_ = true;
- } else {
- return {Status::RedisParseErr, "The bit argument must be 1 or 0."};
+ if (args.size() == 6) {
+ if (util::EqualICase(args[5], "BIT")) {
+ is_bit_index_ = true;
+ } else if (util::EqualICase(args[5], "BYTE")) {
+ is_bit_index_ = false;
+ } else {
+ return {Status::RedisParseErr, errInvalidSyntax};
+ }
+ } else if (args.size() > 6) {
+ return {Status::RedisParseErr, errInvalidSyntax};
}
return Commander::Parse(args);
diff --git a/tests/gocase/unit/type/bitmap/bitmap_test.go
b/tests/gocase/unit/type/bitmap/bitmap_test.go
index ede1a7d43..0f3399316 100644
--- a/tests/gocase/unit/type/bitmap/bitmap_test.go
+++ b/tests/gocase/unit/type/bitmap/bitmap_test.go
@@ -541,6 +541,223 @@ func TestBitmap(t *testing.T) {
require.EqualValues(t, 2, cmd.Val())
})
+ t.Run("BITPOS BYTE option produces same result as default byte-indexed
mode", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\xff\xf0",
0).Err())
+ byteResult := rdb.BitPos(ctx, "mykey", 1, 1)
+ require.NoError(t, byteResult.Err())
+ explicitByte := rdb.BitPosSpan(ctx, "mykey", 1, 1, -1, "byte")
+ require.NoError(t, explicitByte.Err())
+ require.EqualValues(t, byteResult.Val(), explicitByte.Val())
+ })
+
+ t.Run("BITPOS BYTE option is case-insensitive", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\xff\xf0",
0).Err())
+ lower, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, -1,
"byte").Int64()
+ require.NoError(t, err)
+ upper, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, -1,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, lower, upper)
+ })
+
+ t.Run("BITPOS BIT vs BYTE gives different results for same range",
func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\xff\xf0",
0).Err())
+ bitResult := rdb.BitPosSpan(ctx, "mykey", 1, 0, 15, "bit")
+ require.NoError(t, bitResult.Err())
+ require.EqualValues(t, 8, bitResult.Val())
+ byteResult := rdb.BitPosSpan(ctx, "mykey", 1, 0, 15, "byte")
+ require.NoError(t, byteResult.Err())
+ require.EqualValues(t, 8, byteResult.Val())
+ bitResult2 := rdb.BitPosSpan(ctx, "mykey", 1, 0, 7, "bit")
+ require.NoError(t, bitResult2.Err())
+ require.EqualValues(t, -1, bitResult2.Val())
+ byteResult2 := rdb.BitPosSpan(ctx, "mykey", 1, 0, 7, "byte")
+ require.NoError(t, byteResult2.Err())
+ require.EqualValues(t, 8, byteResult2.Val())
+ })
+
+ t.Run("BITPOS rejects invalid option", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, -1, "INVALID").Err()
+ require.Error(t, err)
+ })
+
+ t.Run("BITPOS rejects extra arguments after BYTE/BIT", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, -1, "BIT",
"extra").Err()
+ require.Error(t, err)
+ })
+
+ t.Run("BITPOS rejects BIT unit without end offset", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, "BIT").Err()
+ require.ErrorContains(t, err, "not started as an integer")
+ })
+
+ t.Run("BITPOS rejects BYTE unit without end offset", func(t *testing.T)
{
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, "BYTE").Err()
+ require.ErrorContains(t, err, "not started as an integer")
+ })
+
+ t.Run("BITPOS rejects non-integer bit argument", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", "x").Err()
+ require.ErrorContains(t, err, "The bit argument must be 1 or 0")
+ })
+
+ t.Run("BITPOS rejects non-integer bit argument with BIT unit", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x80", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", "x", 0, 0, "BIT").Err()
+ require.ErrorContains(t, err, "The bit argument must be 1 or 0")
+ })
+
+ t.Run("BITPOS rejects bit argument of 2", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 2).Err()
+ require.ErrorContains(t, err, "The bit argument must be 1 or 0")
+ })
+
+ t.Run("BITPOS rejects bit argument of -1", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", -1).Err()
+ require.ErrorContains(t, err, "The bit argument must be 1 or 0")
+ })
+
+ t.Run("BITPOS rejects non-integer start offset", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, "abc").Err()
+ require.ErrorContains(t, err, "not started as an integer")
+ })
+
+ t.Run("BITPOS rejects non-integer end offset", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, "abc").Err()
+ require.ErrorContains(t, err, "not started as an integer")
+ })
+
+ t.Run("BITPOS bit=1 with nonexistent key returns -1", func(t
*testing.T) {
+ require.NoError(t, rdb.Del(ctx, "nosuchkey").Err())
+ val, err := rdb.Do(ctx, "BITPOS", "nosuchkey", 1).Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS bit=0 with nonexistent key returns 0", func(t *testing.T)
{
+ require.NoError(t, rdb.Del(ctx, "nosuchkey").Err())
+ val, err := rdb.Do(ctx, "BITPOS", "nosuchkey", 0).Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 0, val)
+ })
+
+ t.Run("BITPOS BYTE with negative start", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x00\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, -2, -1,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS BIT with negative start and end", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x00\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, -16, -9,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS returns -1 when start > end after normalization", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x00\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 2, 1,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS BIT returns -1 when start > end after normalization",
func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x00\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 16, 8,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS BYTE bit=0 with all-ones string and explicit end returns
-1", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\xff\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, 0, 2,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS BIT bit=0 with all-ones string and explicit end returns
-1", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\xff\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, 0, 23,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS BYTE bit=0 without end extends past string (finds
trailing zero)", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\xff\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0).Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 24, val)
+ })
+
+ t.Run("BITPOS BYTE with end beyond string length clamps correctly",
func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\xff\x00",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, 100,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS BIT with end beyond string length clamps correctly",
func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\xff\x00",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 0, 999,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS BYTE with only start argument", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\x00\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 2).Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 16, val)
+ })
+
+ t.Run("BITPOS BYTE with start past string returns -1 for bit=1", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff", 0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 5).Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
+ t.Run("BITPOS on wrong type returns WRONGTYPE error", func(t
*testing.T) {
+ require.NoError(t, rdb.Del(ctx, "mylist").Err())
+ require.NoError(t, rdb.LPush(ctx, "mylist", "a").Err())
+ err := rdb.Do(ctx, "BITPOS", "mylist", 1).Err()
+ require.ErrorContains(t, err, "WRONGTYPE")
+ })
+
+ t.Run("BITPOS BYTE bit=0 finds first zero in middle byte", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x0f\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, 0, 2,
"BYTE").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS BIT bit=0 finds first zero within bit range", func(t
*testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\xff\x0f\xff",
0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 0, 8, 15,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+ })
+
+ t.Run("BITPOS BIT single bit precision check", func(t *testing.T) {
+ require.NoError(t, rdb.Set(ctx, "mykey", "\x00\x80", 0).Err())
+ val, err := rdb.Do(ctx, "BITPOS", "mykey", 1, 8, 8,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, 8, val)
+
+ val, err = rdb.Do(ctx, "BITPOS", "mykey", 1, 9, 15,
"BIT").Int64()
+ require.NoError(t, err)
+ require.EqualValues(t, -1, val)
+ })
+
/* Test cases adapted from redis test cases :
https://github.com/redis/redis/blob/unstable/tests/unit/bitops.tcl
*/
t.Run("BITPOS bit=0 with empty key returns 0", func(t *testing.T) {
