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.

Sign up for GitHub

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

Jump to bottom

Fix ZRANGE command should return an empty array when count = 0 #1492

Merged
merged 12 commits into from
Jun 15, 2023
Merged

Fix ZRANGE command should return an empty array when count = 0 #1492

merged 12 commits into from
Jun 15, 2023

Conversation

infdahai
Copy link
Contributor

@infdahai infdahai commented Jun 11, 2023 •
edited
Loading

closes #1483

we parse the all procedure and return the empty array quickly when the count of limit is zero. The output is same as redis.

@infdahai

This comment was marked as off-topic.

Sign in to view
@infdahai

This comment was marked as off-topic.

Sign in to view
@infdahai

This comment was marked as outdated.

Sign in to view
@git-hulk
Copy link
Member

@infdahai You can use expression like require.EqualError(t, r.Err(), redis.Nil.Error())

@infdahai infdahai changed the title Fix zset error: return nilstring when limit cnt == 0 Fix zset error: return the empty array when limit cnt == 0 Jun 12, 2023
@infdahai infdahai closed this Jun 12, 2023
@infdahai infdahai reopened this Jun 12, 2023
git-hulk
git-hulk reviewed Jun 13, 2023
View reviewed changes
src/commands/cmd_zset.cc Outdated Show resolved Hide resolved
infdahai
infdahai commented Jun 13, 2023
View reviewed changes
src/commands/cmd_zset.cc Outdated Show resolved Hide resolved
@git-hulk git-hulk changed the title Fix zset error: return the empty array when limit cnt == 0 Fix ZRANG command should return an empty array when count = 0 Jun 14, 2023
@git-hulk git-hulk changed the title Fix ZRANG command should return an empty array when count = 0 Fix ZRANGE command should return an empty array when count = 0 Jun 14, 2023
torwig
torwig approved these changes Jun 14, 2023
View reviewed changes
Copy link
Contributor

@torwig torwig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@git-hulk
Copy link
Member

Thanks all, merging..

@git-hulk git-hulk merged commit 46e7eaf into apache:unstable Jun 15, 2023
@infdahai infdahai deleted the zran_zero branch June 15, 2023 03:42
jihuayu pushed a commit to jihuayu/incubator-kvrocks that referenced this pull request Jun 16, 2023
@infdahai @jihuayu
Fix ZRANGE command should return an empty array when count = 0 (apach…
39a0bf3 
…e#1492)

Currently, ZRANGE will return all matched members if the count = 0 which is not consistent
with the Redis behavior. For example:

```shell
127.0.0.1:6666> zadd zset11 1 a 2 b 3 c 4 d 5 e 6 f 7 g
(integer) 7
127.0.0.1:6666> zrange zset11 0 6 byscore limit 0 0
```

Redis will return an empty array:
```shell
127.0.0.1:6379> zadd zset11 1 a 2 b 3 c 4 d 5 e 6 f 7 g
(integer) 7
127.0.0.1:6379> zrange zset11 0 6 byscore limit 0 0
(empty array)
```

But we got all matched members in ZSET:
```shell
127.0.0.1:6666> zrange zset11 0 6 byscore limit 0 0
1) "a"
2) "b"
3) "c"
4) "d"
5) "e"
6) "f"
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@torwig torwig torwig approved these changes

@git-hulk git-hulk git-hulk approved these changes

@PragmaTwice PragmaTwice Awaiting requested review from PragmaTwice

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ZRANGE with LIMIT and COUNT=0
3 participants
@infdahai @git-hulk @torwig