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

Add support for GT and LT in ZADD #1789

Closed
chorin1 opened this issue Jun 8, 2021 · 5 comments · Fixed by #1792
Closed

Add support for GT and LT in ZADD #1789

chorin1 opened this issue Jun 8, 2021 · 5 comments · Fixed by #1792
Projects

Comments

@chorin1
Copy link

chorin1 commented Jun 8, 2021

Expected Behavior

Since Redis 6.2 ZADD has additional useful options - LT (less than) and GT (greater than).
There should be corresponding functions in go-redis -> ZAddLT & ZAddGT.

Possible Implementation

Since GT, LT and NX options are mutually exclusive there are two ways to go from here:

  1. Add ZAddLT/GT + ZIncrGT/LT + ZAddXXGT/LT (total of 6 functions)
  2. Have a single ZAdd function and introduce a Zadd options struct => breaking change.
@chorin1
Copy link
Author

chorin1 commented Jun 8, 2021

If there's interest, I can cook up a PR

@monkey92t monkey92t added this to To do in sync-cmd via automation Jun 8, 2021
@monkey92t
Copy link
Collaborator

We can keep the more commonly used API, such as:
zadd key score member...
zaddNX..

In addition, we can add an API that supports all parameters, such as ZAddArgs(ctx, key, *ZAddArgs)
What do you think?

@monkey92t monkey92t moved this from To do to In progress in sync-cmd Jun 15, 2021
@vmihailenco
Copy link
Collaborator

+1 to ZAddArgs(ctx, key, ZAddArgs) (not a *ZAddArgs since it is a small struct)

@monkey92t
Copy link
Collaborator

@chorin1 HI~
I am currently upgrading the sorted set series of commands. If you have no plans at this time, I will modify the ZADD command

@chorin1
Copy link
Author

chorin1 commented Jun 21, 2021

@monkey92t go ahead.
I saw that you already transitioned the item to "in-progress" so I didn't start any work on my end.
LMK if you need help.

@monkey92t monkey92t linked a pull request Jun 23, 2021 that will close this issue
sync-cmd automation moved this from In progress to Done Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants