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

Fix get # option in sort command #13608

Merged
merged 4 commits into from
Oct 22, 2024
Merged

Fix get # option in sort command #13608

merged 4 commits into from
Oct 22, 2024

Conversation

opt-m
Copy link
Contributor

@opt-m opt-m commented Oct 17, 2024

From 7.4, Redis allows GET options in cluster mode when the pattern maps to
the same slot as the key, but GET # pattern that represents key itself is missed.
This commit resolves it.

fix #13607.

@ShooterIT
Copy link
Collaborator

cool, this bug just is introduced in 7.4, for old version, we don't support GET option in cluster mode, right?

can you add some tests also, maybe just remove tag {cluster:skip}, https://github.com/redis/redis/blob/unstable/tests/unit/sort.tcl#L74, i can do it if you are not familiar with tcl test.

@opt-m
Copy link
Contributor Author

opt-m commented Oct 17, 2024

hi @ShooterIT , i'm not familiar with tcl test. maybe you can do it

@sundb sundb added the release-notes indication that this issue needs to be mentioned in the release notes label Oct 18, 2024
@ShooterIT
Copy link
Collaborator

Hi @opt-m I updated this PR, please have a look when you are available

@opt-m
Copy link
Contributor Author

opt-m commented Oct 21, 2024

@ShooterIT I've reviewed the code, and everything looks great!

@sundb sundb changed the title Fix get # option in sort command (#13607) Fix get # option in sort command Oct 21, 2024
@ShooterIT ShooterIT requested review from sundb and tezc October 21, 2024 08:19
@ShooterIT ShooterIT merged commit 0a8e546 into redis:unstable Oct 22, 2024
16 checks passed
@ShooterIT
Copy link
Collaborator

merged, thank you @opt-m

@ShooterIT
Copy link
Collaborator

Hi @sundb @YaacovHazan maybe we need to cherry pick it to 7.4

@sundb
Copy link
Collaborator

sundb commented Oct 22, 2024

Hi @sundb @YaacovHazan maybe we need to cherry pick it to 7.4

agree, i marked the backport label for 7.4.

YaacovHazan pushed a commit that referenced this pull request Nov 4, 2024
From 7.4, Redis allows `GET` options in cluster mode when the pattern maps to
the same slot as the key, but GET # pattern that represents key itself is missed.
This commit resolves it, bug report #13607.

---------

Co-authored-by: Yuan Wang <[email protected]>
@YaacovHazan YaacovHazan mentioned this pull request Nov 4, 2024
YaacovHazan pushed a commit to YaacovHazan/redis that referenced this pull request Nov 4, 2024
From 7.4, Redis allows `GET` options in cluster mode when the pattern maps to
the same slot as the key, but GET # pattern that represents key itself is missed.
This commit resolves it, bug report redis#13607.

---------

Co-authored-by: Yuan Wang <[email protected]>
@YaacovHazan YaacovHazan mentioned this pull request Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
class:bug release-notes indication that this issue needs to be mentioned in the release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG]7.4 sort in cluster mode should ignore: get # while doing slot comparison
4 participants