Skip to content

fix: remove incorrect Start/Stop swap in ZRange Rev+ByScore/ByLex#979

Merged
rueian merged 2 commits intoredis:mainfrom
tmchow:fix/978-zrange-rev-swap
Apr 6, 2026
Merged

fix: remove incorrect Start/Stop swap in ZRange Rev+ByScore/ByLex#979
rueian merged 2 commits intoredis:mainfrom
tmchow:fix/978-zrange-rev-swap

Conversation

@tmchow
Copy link
Copy Markdown
Contributor

@tmchow tmchow commented Apr 5, 2026

Summary

Ports redis/go-redis#3751 to rueidiscompat. ZRangeStore (and zRangeArgs/ZRangeArgsWithScores) returned 0 instead of the correct count when using Rev: true with ByScore or ByLex. The client was swapping Start/Stop before sending to Redis, but Redis 6.2+ ZRANGE with REV already expects the caller to supply the range in high-to-low order.

Removed the conditional swap in all three locations:

  • Compat.zRangeArgs
  • Compat.ZRangeStore
  • CacheCompat.zRangeArgs

Updated affected tests to pass Start/Stop in the correct high-to-low order.

Before / After

// Before: had to pass low-to-high and rely on client swap (broken for ZRangeStore)
adapter.ZRangeStore(ctx, dst, ZRangeArgs{
    Key: src, Start: 3, Stop: 5, ByScore: true, Rev: true,
}) // returned 0 (wrong)

// After: pass high-to-low, matching Redis ZRANGE REV semantics
adapter.ZRangeStore(ctx, dst, ZRangeArgs{
    Key: src, Start: 5, Stop: 3, ByScore: true, Rev: true,
}) // returns correct count

Fixes #978

This contribution was developed with AI assistance (Codex).

Compound Engineering


Note

Medium Risk
Changes argument ordering sent to Redis for ZRANGE/ZRANGESTORE when using Rev with ByScore/ByLex, which can affect query results and stored set contents if callers relied on the previous (incorrect) swap.

Overview
Fixes ZRangeArgs/ZRangeArgsWithScores and ZRangeStore to always send Start then Stop to Redis, removing the special-case swap previously done for Rev + ByScore/ByLex (also applied to the cached adapter).

Updates tests to pass Start/Stop in the expected high-to-low order when Rev: true, aligning behavior with Redis 6.2+ ZRANGE ... REV semantics and correcting ZRANGESTORE result counts.

Reviewed by Cursor Bugbot for commit 5f52ae6. Bugbot is set up for automated code reviews on this repo. Configure here.

Port of redis/go-redis#3751. Redis 6.2+ ZRANGE with REV expects the
caller to supply the range in high-to-low order. The client-side swap
was corrupting queries, causing ZRangeStore to return 0 instead of the
correct count.

Fixes redis#978

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jit-ci
Copy link
Copy Markdown

jit-ci Bot commented Apr 5, 2026

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

@rueian
Copy link
Copy Markdown
Collaborator

rueian commented Apr 5, 2026

Hi @tmchow, thanks! Could you fix the test?
image

Missed updating 3 ZRangeArgs test calls in pipeline_test.go that still
used old low-to-high ordering (Start: 1, Stop: 4) with Rev: true.
@tmchow
Copy link
Copy Markdown
Contributor Author

tmchow commented Apr 5, 2026

Hi @tmchow, thanks! Could you fix the test?

image

Fixed!

@rueian rueian merged commit dce40ec into redis:main Apr 6, 2026
28 checks passed
@tmchow
Copy link
Copy Markdown
Contributor Author

tmchow commented Apr 6, 2026

Thanks @rueian !

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.

Porting https://github.com/redis/go-redis/pull/3751 to rueidiscompat

2 participants