This is an automated email from the ASF dual-hosted git repository.
hulk 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 8a939dd2 Fix incorrect ZRANGE command response format in RESP3 mode
(#2132)
8a939dd2 is described below
commit 8a939dd2d065a586923d787960e7f2c57c1e025a
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")
+ })
}
