Skip to content

Fix multiple COUNT in LMPOP/BLMPOP/ZMPOP/BZMPOP #9701

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
Merged
oranagra merged 3 commits into from
Oct 31, 2021
Merged

Fix multiple COUNT in LMPOP/BLMPOP/ZMPOP/BZMPOP #9701

oranagra merged 3 commits into from
Oct 31, 2021

Conversation

@enjoy-binbin
Copy link
Contributor

@enjoy-binbin enjoy-binbin commented Oct 31, 2021 •
edited
Loading

The previous code did not check whether COUNT is set.
So we can use lmpop 2 key1 key2 left count 1 count 2.

This situation can occur in LMPOP/BLMPOP/ZMPOP/BZMPOP commands.
LMPOP/BLMPOP introduced in #9373, ZMPOP/BZMPOP introduced in #9484.

before:

127.0.0.1:6379> lmpop 2 key1 key2 left count 1
(nil)
127.0.0.1:6379> lmpop 2 key1 key2 left count 1 count 2
(nil)
127.0.0.1:6379> lmpop 2 key1 key2 left count 1 count 2 count 3
(nil)

127.0.0.1:6379> zmpop 2 key1 key2 min count 1
(nil)
127.0.0.1:6379> zmpop 2 key1 key2 min count 1 count 2
(nil)
127.0.0.1:6379> zmpop 2 key1 key2 min count 1 count 2 count 3
(nil)

after:

127.0.0.1:6379> lmpop 2 key1 key2 left count 1 count 2 count 3
(error) ERR syntax error
127.0.0.1:6379> zmpop 2 key1 key2 min count 2 count 2 count 3
(error) ERR syntax error

@enjoy-binbin
Fix multiple COUNT in LMPOP/BLMPOP/ZMPOP/BZMPOP
dfb00d9 
The previous code did not check whether COUNT is set.
So we can use `lmpop 2 key1 key2 left count 1 count 2`.

This situation can occur in LMPOP/BLMPOP/ZMPOP/BZMPOP commands.
LMPOP/BLMPOP introduced in redis#9373, ZMPOP/BZMPOP introduced in redis#9484.
@enjoy-binbin
fix typo...
74897d0
@enjoy-binbin
remove count_is_set flag
2a22f3b
@oranagra oranagra merged commit 0335788 into redis:unstable Oct 31, 2021
@enjoy-binbin enjoy-binbin deleted the fix_multiple_count branch November 1, 2021 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oranagra oranagra oranagra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@enjoy-binbin @oranagra