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

Command SRANDMEMBER returns empty array when [count] is a negative number #2072

Closed
2 tasks done
sam-hn opened this issue Jan 30, 2024 · 6 comments
Closed
2 tasks done
Assignees
Labels
bug type bug

Comments

@sam-hn
Copy link

sam-hn commented Jan 30, 2024

Search before asking

  • I had searched in the issues and found no similar issues.

Version

2.7.0

Minimal reproduce step

10.70.163.45:6666> srandmember set1 5

  1. "mem1"
  2. "mem2"
  3. "mem3"
    10.70.163.45:6666> srandmember set1 -5
    (empty array)
    10.70.163.45:6666> srandmember set1 -2
    (empty array)

What did you expect to see?

According to https://redis.io/commands/srandmember/, If called with a negative count, the behavior changes and the command is allowed to return the same element multiple times. In this case, the number of returned elements is the absolute value of the specified count

What did you see instead?

Anything Else?

No response

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@sam-hn sam-hn added the bug type bug label Jan 30, 2024
@jihuayu
Copy link
Member

jihuayu commented Jan 30, 2024

@sam-hn Thank you.
You can find related code in this
And it is better if you can add a test case

@git-hulk
Copy link
Member

git-hulk commented Jan 30, 2024

It has been resolved in #2032?

@jihuayu
Copy link
Member

jihuayu commented Jan 30, 2024

@git-hulk It looks like no. First is Redis, second is kvrocks
image

@mapleFU
Copy link
Member

mapleFU commented Jan 30, 2024

rocksdb::Status Set::Take(const Slice &user_key, std::vector<std::string> *members, int count, bool pop) {
  int n = 0;
  members->clear();
  if (count <= 0) return rocksdb::Status::OK();

The problem it's the 4th line... It would be easy to fix

@git-hulk
Copy link
Member

@git-hulk It looks like no. First is Redis, second is kvrocks image

Got it, thank you!

@mapleFU
Copy link
Member

mapleFU commented Feb 26, 2024

I've fixed this via: #2113

@mapleFU mapleFU closed this as completed Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug type bug
Projects
None yet
Development

No branches or pull requests

4 participants