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

SETNX command should return integer value #951

Merged
merged 8 commits into from
Jan 25, 2025

Conversation

prvyk
Copy link
Contributor

@prvyk prvyk commented Jan 24, 2025

https://redis.io/docs/latest/commands/setnx/

Per redis docs, SETNX command should return 1 or 0. However, garnet translates it internally to SET with NX flag and therefore incorrectly returns SET return values ("OK" or nil). Fix by inlining the correct code path into NetworkSETNX function and then returning the correct response. Also add test and fix existing test.

Copy link
Contributor

@Vijay-Nirmal Vijay-Nirmal left a comment

Choose a reason for hiding this comment

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

Nice catch

Copy link
Contributor

@TalZaccai TalZaccai left a comment

Choose a reason for hiding this comment

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

Good catch! A few comments...

@prvyk prvyk requested a review from TalZaccai January 24, 2025 21:42
@TalZaccai TalZaccai merged commit 7fb0f8f into microsoft:main Jan 25, 2025
15 checks passed
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.

4 participants