Skip to content

Commit

Permalink
Fix incorrect ZRANGE command response format in RESP3 mode (#2132)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
git-hulk authored Mar 2, 2024
1 parent 2ec78bc commit 8a939dd
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/commands/cmd_zset.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
75 changes: 75 additions & 0 deletions tests/gocase/unit/protocol/protocol_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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")
})
}

0 comments on commit 8a939dd

Please sign in to comment.