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 SORT GET to ignore special pattern # in cluster slot check #1182

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from

Conversation

enjoy-binbin
Copy link
Member

@enjoy-binbin enjoy-binbin commented Oct 17, 2024

This special pattern '#' is used to get the element itself,
it does not actually participate in the slot check.

In this case, passing GET # will cause '#' to participate
in the slot check, causing the command to get an
pattern may be in different slots error.

This special pattern '#' is used to get the element itself,
it does not actually participate in the slot check.

Signed-off-by: Binbin <[email protected]>
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.67%. Comparing base (701ab72) to head (3f020e4).
Report is 1 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1182      +/-   ##
============================================
+ Coverage     70.47%   70.67%   +0.19%     
============================================
  Files           114      114              
  Lines         61799    61802       +3     
============================================
+ Hits          43555    43677     +122     
+ Misses        18244    18125     -119     
Files with missing lines Coverage Δ
src/db.c 88.51% <ø> (ø)
src/sort.c 94.77% <100.00%> (+0.05%) ⬆️

... and 14 files with indirect coverage changes

Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

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

Was this a bug (wrong result) or just an optimization problem?

We should include in release notes and backport if it was a bugfix.

src/sort.c Outdated Show resolved Hide resolved
@enjoy-binbin
Copy link
Member Author

enjoy-binbin commented Oct 17, 2024

it was a bug, it (see the test example) will get an error since we will do a slot check with '#', sorry i forget to mention it, i updated the top comment

                addReplyError(c, "GET option of SORT denied in Cluster mode when "
                                 "keys formed by the pattern may be in different slots.");
                syntax_error++;

Co-authored-by: Viktor Söderqvist <[email protected]>
Signed-off-by: Binbin <[email protected]>
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Oct 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes This issue should get a line item in the release notes
Projects
Status: To be backported
Development

Successfully merging this pull request may close these issues.

2 participants