Skip to content

Issue 7416: Add GT and LT options to ZADD for conditional score updates#7818

Merged
oranagra merged 9 commits intoredis:unstablefrom
alexronke-channeladvisor:zadd-gx-lx
Sep 23, 2020
Merged

Issue 7416: Add GT and LT options to ZADD for conditional score updates#7818
oranagra merged 9 commits intoredis:unstablefrom
alexronke-channeladvisor:zadd-gx-lx

Conversation

@alexronke-channeladvisor
Copy link
Contributor

@alexronke-channeladvisor alexronke-channeladvisor commented Sep 18, 2020

No description provided.

@alexronke-channeladvisor
Copy link
Contributor Author

alexronke-channeladvisor commented Sep 18, 2020

This pull request should be linked to issue 7416 if/when it is approved.

oranagra
oranagra reviewed Sep 19, 2020
View reviewed changes
@oranagra
Copy link
Member

oranagra commented Sep 19, 2020

@redis/core-team seems like a useful and trivial feature. what do you think?

@oranagra oranagra linked an issue Sep 19, 2020 that may be closed by this pull request
ZADD option: Conditionally Update Score for Existing Keys #7416
Closed
@oranagra oranagra added class:feature state:major-decision Requires core team consensus state:needs-doc-pr requires a PR to redis-doc repository labels Sep 19, 2020
@oranagra oranagra added this to the Redis 6.2 milestone Sep 19, 2020
@oranagra
Copy link
Member

oranagra commented Sep 22, 2020

@alexronke-channeladvisor thanks. everything looks good to me.
one thing that just now occurred to me is why use GX and LX and not GT and LT?
i.e. I suppose the X in NX is "exists", but here the condition is Greater Than.

@alexronke-channeladvisor
Copy link
Contributor Author

alexronke-channeladvisor commented Sep 22, 2020

I did not realize that the X meant to stand for anything in particular. GT and LT make perfect sense to me.

@alexronke-channeladvisor alexronke-channeladvisor changed the title Issue 7416: Add GX and LX options to ZADD for conditional score updates Issue 7416: Add GT and LT options to ZADD for conditional score updates Sep 22, 2020
@oranagra oranagra added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Sep 23, 2020
@oranagra
Copy link
Member

oranagra commented Sep 23, 2020

@redis/core-team please approve trivial extension for the ZADD command (seems useful to me)

@oranagra oranagra merged commit 66a13cc into redis:unstable Sep 23, 2020
@oranagra
Copy link
Member

oranagra commented Sep 23, 2020

@alexronke-channeladvisor thank you.
care to also make a PR for the command documentatoin? https://github.com/redis/redis-doc

@alexronke-channeladvisor
Copy link
Contributor Author

alexronke-channeladvisor commented Sep 23, 2020

@alexronke-channeladvisor thank you.
care to also make a PR for the command documentatoin? https://github.com/redis/redis-doc

Will do.

@alexronke-channeladvisor alexronke-channeladvisor mentioned this pull request Sep 23, 2020
Merged
@alexronke-channeladvisor
Copy link
Contributor Author

alexronke-channeladvisor commented Sep 23, 2020

PR for docs:
redis/redis-doc#1397

@oranagra oranagra mentioned this pull request Sep 24, 2020
Closed
@alexronke-channeladvisor alexronke-channeladvisor deleted the zadd-gx-lx branch September 24, 2020 14:27
@oranagra oranagra removed this from the Next minor backlog milestone Oct 19, 2020
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Nov 4, 2020
@alexronke-channeladvisor @nopunin10did
Add GT and LT options to ZADD for conditional score updates (redis#7818)
19429db 
Co-authored-by: Alex Ronke <[email protected]>
@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Dec 9, 2020
@jiekun jiekun mentioned this pull request Dec 21, 2020
Closed
18 tasks
@oranagra oranagra mentioned this pull request Jan 13, 2021
Merged
@fatkodima fatkodima mentioned this pull request Oct 7, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yossigo yossigo yossigo approved these changes

@itamarhaber itamarhaber itamarhaber approved these changes

@oranagra oranagra oranagra approved these changes

Assignees

No one assigned

Labels

class:feature release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus state:needs-doc-pr requires a PR to redis-doc repository state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

ZADD option: Conditionally Update Score for Existing Keys

5 participants

@alexronke-channeladvisor @oranagra @yossigo @itamarhaber @nopunin10did