This is an automated email from the ASF dual-hosted git repository. caipengbo pushed a commit to branch 2.8 in repository https://gitbox.apache.org/repos/asf/kvrocks.git
commit d5367f415f1c61628b06ea06a4b755f3089667a2 Author: hulk <[email protected]> AuthorDate: Sat Mar 2 18:24:54 2024 +0800 Fix incorrect ZRANGE command response format in RESP3 mode (#2132) Currently, ZRANGE command with scores will always return an array of string and double type: ``` 1) "a" 2) (double) 1.2 3) "b" 4) (double) 1.5 ``` But in Redis, it will return an array of arrays if the RESP3 was enabled: ``` 1) 1) "a" 2) (double) 1.2 2) 1) "b" 2) (double) 1.5 ``` This different behavior cannot be found with go-redis client since it will be always parsed as the corrent member-score pairs, but using ZRANGE command in Lua script this inconsistent result format might be awareness. --- src/commands/cmd_zset.cc | 6 ++- tests/gocase/unit/protocol/protocol_test.go | 75 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/commands/cmd_zset.cc b/src/commands/cmd_zset.cc index 136b84eb..c32c976a 100644 --- a/src/commands/cmd_zset.cc +++ b/src/commands/cmd_zset.cc @@ -815,8 +815,12 @@ class CommandZRangeGeneric : public Commander { return {Status::RedisExecErr, s.ToString()}; } - output->append(redis::MultiLen(member_scores.size() * (with_scores_ ? 2 : 1))); + auto is_resp3 = conn->GetProtocolVersion() == RESP::v3; + // RESP3 with scores should return an array of arrays, + // so we don't need to multiply the size by 2 here. + output->append(redis::MultiLen(member_scores.size() * (with_scores_ && !is_resp3 ? 2 : 1))); for (const auto &ms : member_scores) { + if (with_scores_ && is_resp3) output->append(MultiLen(2)); output->append(redis::BulkString(ms.member)); if (with_scores_) output->append(conn->Double(ms.score)); } diff --git a/tests/gocase/unit/protocol/protocol_test.go b/tests/gocase/unit/protocol/protocol_test.go index 39914a33..e4f22327 100644 --- a/tests/gocase/unit/protocol/protocol_test.go +++ b/tests/gocase/unit/protocol/protocol_test.go @@ -23,6 +23,8 @@ import ( "context" "testing" + "github.com/redis/go-redis/v9" + "github.com/apache/kvrocks/tests/gocase/util" "github.com/stretchr/testify/require" ) @@ -145,8 +147,10 @@ func TestProtocolRESP2(t *testing.T) { defer srv.Close() c := srv.NewTCPClient() + rdb := srv.NewClient() defer func() { require.NoError(t, c.Close()) + require.NoError(t, rdb.Close()) }() t.Run("debug protocol string", func(t *testing.T) { @@ -188,6 +192,36 @@ func TestProtocolRESP2(t *testing.T) { require.NoError(t, c.WriteArgs("ZRANK", "no-exists-zset", "m0", "WITHSCORE")) c.MustRead(t, "*-1") }) + + t.Run("command ZRANGE should be always return an array of strings", func(t *testing.T) { + rdb.ZAddArgs(context.Background(), "zset", redis.ZAddArgs{ + Members: []redis.Z{{1, "one"}, {2, "two"}, {3, "three"}}, + }) + + require.NoError(t, c.WriteArgs("ZRANGE", "zset", "0", "-1")) + c.MustRead(t, "*3") + c.MustRead(t, "$3") + c.MustRead(t, "one") + c.MustRead(t, "$3") + c.MustRead(t, "two") + c.MustRead(t, "$5") + c.MustRead(t, "three") + + require.NoError(t, c.WriteArgs("ZRANGE", "zset", "0", "-1", "WITHSCORES")) + c.MustRead(t, "*6") + c.MustRead(t, "$3") + c.MustRead(t, "one") + c.MustRead(t, "$1") + c.MustRead(t, "1") + c.MustRead(t, "$3") + c.MustRead(t, "two") + c.MustRead(t, "$1") + c.MustRead(t, "2") + c.MustRead(t, "$5") + c.MustRead(t, "three") + c.MustRead(t, "$1") + c.MustRead(t, "3") + }) } func TestProtocolRESP3(t *testing.T) { @@ -197,8 +231,10 @@ func TestProtocolRESP3(t *testing.T) { defer srv.Close() c := srv.NewTCPClient() + rdb := srv.NewClient() defer func() { require.NoError(t, c.Close()) + require.NoError(t, rdb.Close()) }() t.Run("debug protocol string", func(t *testing.T) { @@ -246,4 +282,43 @@ func TestProtocolRESP3(t *testing.T) { require.NoError(t, c.WriteArgs("ZRANK", "no-exists-zset", "m0", "WITHSCORE")) c.MustRead(t, "_") }) + + t.Run("command ZRANGE should return an array of arrays if with score", func(t *testing.T) { + rdb.ZAddArgs(context.Background(), "zset", redis.ZAddArgs{ + Members: []redis.Z{{1, "one"}, {2, "two"}, {3, "three"}}, + }) + + require.NoError(t, c.WriteArgs("HELLO", "3")) + values := []string{"%3", "$6", "server", "$5", "redis", "$5", "proto", ":3", "$4", "mode", "$10", "standalone"} + for _, line := range values { + c.MustRead(t, line) + } + + // should return an array of strings if without score + require.NoError(t, c.WriteArgs("ZRANGE", "zset", "0", "-1")) + c.MustRead(t, "*3") + c.MustRead(t, "$3") + c.MustRead(t, "one") + c.MustRead(t, "$3") + c.MustRead(t, "two") + c.MustRead(t, "$5") + c.MustRead(t, "three") + + // should return an array of arrays if with score, + // and the score should be a double type + require.NoError(t, c.WriteArgs("ZRANGE", "zset", "0", "-1", "WITHSCORES")) + c.MustRead(t, "*3") + c.MustRead(t, "*2") + c.MustRead(t, "$3") + c.MustRead(t, "one") + c.MustRead(t, ",1") + c.MustRead(t, "*2") + c.MustRead(t, "$3") + c.MustRead(t, "two") + c.MustRead(t, ",2") + c.MustRead(t, "*2") + c.MustRead(t, "$5") + c.MustRead(t, "three") + c.MustRead(t, ",3") + }) }
