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

feat: add spop by count #1434

Merged

Conversation

ChanphongGu
Copy link
Contributor

@ChanphongGu ChanphongGu commented May 5, 2023

fix #889:
redis API: spop with count.

@wanghenshui wanghenshui changed the title feature: add spop by count feat: add spop by count May 5, 2023
src/pika_set.cc Outdated Show resolved Hide resolved
@@ -424,6 +424,9 @@ class Storage {
// Removes and returns one random elements from the set value store at key.
Status SPop(const Slice& key, std::string* member);

// Removes and returns several random elements specified by count from the set value store at key.
Status SPop(const Slice& key, std::vector<std::string>* members, int64_t count);
Copy link
Collaborator

Choose a reason for hiding this comment

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

这里加接口,可以把旧的接口删了

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok,那set test里面也需要修改了。

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok,那set test里面也需要修改了。

嗯,你一块改了就行。另外里面那个50那个magic值,最好能导出来

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok,那set test里面也需要修改了。

嗯,你一块改了就行。另外里面那个50那个magic值,最好能导出来

导出来啥意思?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok,那set test里面也需要修改了。

嗯,你一块改了就行。另外里面那个50那个magic值,最好能导出来

导出来啥意思?

最好可配置

Copy link
Collaborator

Choose a reason for hiding this comment

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

有道理

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok,那set test里面也需要修改了。

嗯,你一块改了就行。另外里面那个50那个magic值,最好能导出来

导出来啥意思?

最好可配置

这个可以建个 issue,后续推进吧。

@AlexStocks
Copy link
Collaborator

image

你的实现有问题,你看上面例子,redis 是允许 count 大于 set 里面 member 个数的

@ChanphongGu
Copy link
Contributor Author

改动点:
1、command参数修改为-2;
2、去掉旧spop接口;
3、增加length < cnt的情况删除set;
4、修改set单元测试。

@ChanphongGu
Copy link
Contributor Author

改动的已经commit了,等待review

@AlexStocks
Copy link
Collaborator

ping @wanghenshui

@wanghenshui wanghenshui merged commit 277ebe0 into OpenAtomFoundation:unstable May 9, 2023
7 checks passed
bigdaronlee163 pushed a commit to bigdaronlee163/pika that referenced this pull request Jun 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

redis API: spop with count
3 participants