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 ZRANGE command should return an empty array when count = 0 #1492

Merged
merged 12 commits into from
Jun 15, 2023
16 changes: 15 additions & 1 deletion src/commands/cmd_zset.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "commander.h"
#include "commands/scan_base.h"
#include "error_constants.h"
#include "server/redis_reply.h"
#include "server/server.h"
#include "types/redis_zset.h"

Expand Down Expand Up @@ -454,6 +455,12 @@ class CommandZRangeStore : public Commander {
RangeScoreSpec score_spec_;
};

/*
* description:
* syntax: `ZRANGE key start stop [BYSCORE | BYLEX] [REV] [LIMIT offset count]
* [WITHSCORES]`
*
*/
class CommandZRangeGeneric : public Commander {
public:
explicit CommandZRangeGeneric(ZRangeType range_type = kZRangeAuto, ZRangeDirection direction = kZRangeDirectionAuto)
Expand Down Expand Up @@ -545,7 +552,6 @@ class CommandZRangeGeneric : public Commander {

Status Execute(Server *svr, Connection *conn, std::string *output) override {
redis::ZSet zset_db(svr->storage, conn->GetNamespace());

std::vector<MemberScore> member_scores;

rocksdb::Status s;
Expand All @@ -555,9 +561,17 @@ class CommandZRangeGeneric : public Commander {
s = zset_db.RangeByRank(key_, rank_spec_, &member_scores, nullptr);
break;
case kZRangeScore:
if (score_spec_.count == 0) {
*output = redis::MultiBulkString({});
return Status::OK();
}
s = zset_db.RangeByScore(key_, score_spec_, &member_scores, nullptr);
break;
case kZRangeLex:
if (lex_spec_.count == 0) {
*output = redis::MultiBulkString({});
return Status::OK();
}
s = zset_db.RangeByLex(key_, lex_spec_, &member_scores, nullptr);
break;
}
Expand Down
29 changes: 29 additions & 0 deletions tests/gocase/unit/type/zset/zset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"strconv"
"strings"
"testing"
"time"

"github.com/apache/incubator-kvrocks/tests/gocase/util"
"github.com/redis/go-redis/v9"
Expand Down Expand Up @@ -439,6 +440,33 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s
{4, "d"},
}, rdb.ZRangeWithScores(ctx, "ztmp", 0, -1).Val())

for i := 1; i < 10; i++ {
cmd := rdb.ZRangeArgs(ctx, redis.ZRangeArgs{Key: "ztmp", Count: 0, ByScore: true, Start: 0, Stop: -1, Offset: int64(i)})
require.NoError(t, cmd.Err())
require.Equal(t, []string{}, cmd.Val())
infdahai marked this conversation as resolved.
Show resolved Hide resolved
}

// go-redis removes the limit condition when (offset, count) == (0, 0)
// so we use (offset, count) = (0, -1)
cmd1 := rdb.Do(ctx, "zrange", "ztmp", 0, -1, "byscore", "limit", 0, 0)
require.NoError(t, cmd1.Err())
require.Equal(t, []interface{}{}, cmd1.Val())

// limit with zero count
for i := 0; i < 20; i++ {
var args [3]int64
for j := 0; j < 3; j++ {
rand.Seed(time.Now().UnixNano())
args[j] = rand.Int63n(20) - 10
}
if args[2] == 0 {
continue
}
cmd := rdb.ZRangeArgs(ctx, redis.ZRangeArgs{Key: "ztmp", Count: 0, ByScore: true, Start: args[0], Stop: args[1], Offset: args[2]})
require.NoError(t, cmd.Err())
require.Equal(t, []string{}, cmd.Val())
}

// extend zrange commands
require.Equal(t, []string{"a", "b", "c", "d"}, rdb.ZRangeArgs(ctx, redis.ZRangeArgs{Key: "ztmp", Start: 0, Stop: -1, Offset: 0, Count: -1}).Val())
require.Equal(t, []string{"d", "c", "b", "a"}, rdb.ZRangeArgs(ctx, redis.ZRangeArgs{Key: "ztmp", Start: 0, Stop: -1, Offset: 0, Count: -1, Rev: true}).Val())
Expand All @@ -454,6 +482,7 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s
{2, "b"},
{1, "a"},
}, rdb.ZRangeArgsWithScores(ctx, redis.ZRangeArgs{Key: "ztmp", Start: 0, Stop: -1, Offset: 0, Count: -1, Rev: true}).Val())

})

t.Run(fmt.Sprintf("ZREVRANGE basics - %s", encoding), func(t *testing.T) {
Expand Down