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.

Sign up for GitHub

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

Jump to bottom

ZRevRangeByScore's min and max are backwards #114

Closed
mosesn opened this issue Oct 18, 2012 · 1 comment
Closed

ZRevRangeByScore's min and max are backwards #114

mosesn opened this issue Oct 18, 2012 · 1 comment

Comments

@mosesn
Copy link
Contributor

mosesn commented Oct 18, 2012

problem

ZRevRangeByScore extends ZScoredRange in https://github.com/twitter/finagle/blob/master/finagle-redis/src/main/scala/com/twitter/finagle/redis/protocol/commands/SortedSets.scala but ZRevRangeByScore has a different order of arguments than ZRangeByScore. ZRevRangeByScore goes key max min [WITHSCORES] [LIMIT offset count] whereas ZRangeByScore goes key min max [WITHSCORES] [LIMIT offset count]. ZScoredRange is always min then max, which leads to shenanigans where you call client.zRevRangeByScore("key", 3, 0) and it sends to redis ZREVRANGEBYSCORE "key" 0 3.

solution

add an abstract method to the trait which forces the implementor to specify the order of min and max.
@asrinivas
Copy link

I just submitted a fix to our internal repo, should appear here soon

kil9 pushed a commit to kil9/finagle that referenced this issue Feb 26, 2014
[split] finagle-redis: fix zrevrangebyscore bug
1a461f8 
zrevrangebyscore was sending the min and max params in the wrong order, as outlined here: twitter#114. This change list should fix that, and additionally make scan/hscan take JLongs (I had missed this last review)

RB_ID=92375
kil9 pushed a commit to kil9/finagle that referenced this issue Feb 26, 2014
@mariusae
[split] finagle: 5.3.20
76d194c 
	* finagle-redis: fix zrevrangebyscore (twitter#114)
	* finagle-memcached: fix space leak when using clustering
	* finagle-core: special case 1-hosted (static) clients to bypass loadbalancer
	* finagle-core: Introduce an experimental buffer-based pool

RB_ID=93909
vkostyukov pushed a commit to finagle/finagle-stream that referenced this issue Mar 17, 2016
@mariusae
[split] finagle: 5.3.20
0c245f2 
	* finagle-redis: fix zrevrangebyscore (twitter/finagle#114)
	* finagle-memcached: fix space leak when using clustering
	* finagle-core: special case 1-hosted (static) clients to bypass loadbalancer
	* finagle-core: Introduce an experimental buffer-based pool

RB_ID=93909
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mosesn @asrinivas