Permalink
Browse files

backport of patches that fix the behavior of LRANGE, LTRIM, ZRANGE, Z…

…REVRANGE

and ZREMRANGEBYSCORE when an out of range negative end index is provided
original commits: 184d74, 4774a5, f483ce
  • Loading branch information...
pietern committed Jul 12, 2010
1 parent 75d675d commit 797c9ffb6b98a17542cbc708a6a30ecfd4d94485
Showing with 110 additions and 27 deletions.
  1. +8 −9 redis.c
  2. +9 −0 tests/unit/type/list.tcl
  3. +93 −18 tests/unit/type/zset.tcl
View
17 redis.c
@@ -4887,9 +4887,9 @@ static void lrangeCommand(redisClient *c) {
if (start < 0) start = llen+start;
if (end < 0) end = llen+end;
if (start < 0) start = 0;
- if (end < 0) end = 0;
- /* indexes sanity checks */
+ /* Invariant: start >= 0, so this test will be true when end < 0.
+ * The range is empty when start > end or start >= length. */
if (start > end || start >= llen) {
/* Out of range start or start > end result in empty list */
addReply(c,shared.emptymultibulk);
@@ -4926,9 +4926,9 @@ static void ltrimCommand(redisClient *c) {
if (start < 0) start = llen+start;
if (end < 0) end = llen+end;
if (start < 0) start = 0;
- if (end < 0) end = 0;
- /* indexes sanity checks */
+ /* Invariant: start >= 0, so this test will be true when end < 0.
+ * The range is empty when start > end or start >= length. */
if (start > end || start >= llen) {
/* Out of range start or start > end result in empty list */
ltrim = llen;
@@ -5893,9 +5893,9 @@ static void zremrangebyrankCommand(redisClient *c) {
if (start < 0) start = llen+start;
if (end < 0) end = llen+end;
if (start < 0) start = 0;
- if (end < 0) end = 0;
- /* indexes sanity checks */
+ /* Invariant: start >= 0, so this test will be true when end < 0.
+ * The range is empty when start > end or start >= length. */
if (start > end || start >= llen) {
addReply(c,shared.czero);
return;
@@ -6143,11 +6143,10 @@ static void zrangeGenericCommand(redisClient *c, int reverse) {
if (start < 0) start = llen+start;
if (end < 0) end = llen+end;
if (start < 0) start = 0;
- if (end < 0) end = 0;
- /* indexes sanity checks */
+ /* Invariant: start >= 0, so this test will be true when end < 0.
+ * The range is empty when start > end or start >= length. */
if (start > end || start >= llen) {
- /* Out of range start or start > end result in empty list */
addReply(c,shared.emptymultibulk);
return;
}
View
@@ -210,6 +210,10 @@ start_server {tags {"list"}} {
r lrange mylist -1000 1000
} {0 1 2 3 4 5 6 7 8 9}
+ test {LRANGE out of range negative end index} {
+ list [r lrange mylist 0 -10] [r lrange mylist 0 -11]
+ } {0 {}}
+
test {LRANGE against non existing key} {
r lrange nosuchkey 0 1
} {}
@@ -223,6 +227,11 @@ start_server {tags {"list"}} {
r lrange mylist 0 -1
} {99 98 97 96 95}
+ test {LTRIM with out of range negative end index} {
+ r ltrim mylist 0 -6
+ r llen mylist
+ } {0}
+
test {LTRIM stress testing} {
set mylist {}
set err {}
View
@@ -1,4 +1,11 @@
start_server {tags {"zset"}} {
+ proc create_zset {key items} {
+ r del $key
+ foreach {score entry} $items {
+ r zadd $key $score $entry
+ }
+ }
+
test {ZSET basic ZADD and score update} {
r zadd ztmp 10 x
r zadd ztmp 20 y
@@ -17,6 +24,66 @@ start_server {tags {"zset"}} {
r zcard ztmp-blabla
} {0}
+ test "ZRANGE basics" {
+ r del ztmp
+ r zadd ztmp 1 a
+ r zadd ztmp 2 b
+ r zadd ztmp 3 c
+ r zadd ztmp 4 d
+
+ assert_equal {a b c d} [r zrange ztmp 0 -1]
+ assert_equal {a b c} [r zrange ztmp 0 -2]
+ assert_equal {b c d} [r zrange ztmp 1 -1]
+ assert_equal {b c} [r zrange ztmp 1 -2]
+ assert_equal {c d} [r zrange ztmp -2 -1]
+ assert_equal {c} [r zrange ztmp -2 -2]
+
+ # out of range start index
+ assert_equal {a b c} [r zrange ztmp -5 2]
+ assert_equal {a b} [r zrange ztmp -5 1]
+ assert_equal {} [r zrange ztmp 5 -1]
+ assert_equal {} [r zrange ztmp 5 -2]
+
+ # out of range end index
+ assert_equal {a b c d} [r zrange ztmp 0 5]
+ assert_equal {b c d} [r zrange ztmp 1 5]
+ assert_equal {} [r zrange ztmp 0 -5]
+ assert_equal {} [r zrange ztmp 1 -5]
+
+ # withscores
+ assert_equal {a 1 b 2 c 3 d 4} [r zrange ztmp 0 -1 withscores]
+ }
+
+ test "ZREVRANGE basics" {
+ r del ztmp
+ r zadd ztmp 1 a
+ r zadd ztmp 2 b
+ r zadd ztmp 3 c
+ r zadd ztmp 4 d
+
+ assert_equal {d c b a} [r zrevrange ztmp 0 -1]
+ assert_equal {d c b} [r zrevrange ztmp 0 -2]
+ assert_equal {c b a} [r zrevrange ztmp 1 -1]
+ assert_equal {c b} [r zrevrange ztmp 1 -2]
+ assert_equal {b a} [r zrevrange ztmp -2 -1]
+ assert_equal {b} [r zrevrange ztmp -2 -2]
+
+ # out of range start index
+ assert_equal {d c b} [r zrevrange ztmp -5 2]
+ assert_equal {d c} [r zrevrange ztmp -5 1]
+ assert_equal {} [r zrevrange ztmp 5 -1]
+ assert_equal {} [r zrevrange ztmp 5 -2]
+
+ # out of range end index
+ assert_equal {d c b a} [r zrevrange ztmp 0 5]
+ assert_equal {c b a} [r zrevrange ztmp 1 5]
+ assert_equal {} [r zrevrange ztmp 0 -5]
+ assert_equal {} [r zrevrange ztmp 1 -5]
+
+ # withscores
+ assert_equal {d 4 c 3 b 2 a 1} [r zrevrange ztmp 0 -1 withscores]
+ }
+
test {ZRANK basics} {
r zadd zranktmp 10 x
r zadd zranktmp 20 y
@@ -69,15 +136,6 @@ start_server {tags {"zset"}} {
set _ $err
} {}
- test {ZRANGE and ZREVRANGE basics} {
- list [r zrange ztmp 0 -1] [r zrevrange ztmp 0 -1] \
- [r zrange ztmp 1 -1] [r zrevrange ztmp 1 -1]
- } {{y x z} {z x y} {x z} {x y}}
-
- test {ZRANGE WITHSCORES} {
- r zrange ztmp 0 -1 withscores
- } {y 1 x 10 z 30}
-
test {ZSETs stress tester - sorting is working well?} {
set delta 0
for {set test 0} {$test < 2} {incr test} {
@@ -288,15 +346,32 @@ start_server {tags {"zset"}} {
list [r zremrangebyscore zset -inf +inf] [r zrange zset 0 -1]
} {5 {}}
- test {ZREMRANGEBYRANK basics} {
- r del zset
- r zadd zset 1 a
- r zadd zset 2 b
- r zadd zset 3 c
- r zadd zset 4 d
- r zadd zset 5 e
- list [r zremrangebyrank zset 1 3] [r zrange zset 0 -1]
- } {3 {a e}}
+ test "ZREMRANGEBYRANK basics" {
+ proc remrangebyrank {min max} {
+ create_zset zset {1 a 2 b 3 c 4 d 5 e}
+ r zremrangebyrank zset $min $max
+ }
+
+ # inner range
+ assert_equal 3 [remrangebyrank 1 3]
+ assert_equal {a e} [r zrange zset 0 -1]
+
+ # start underflow
+ assert_equal 1 [remrangebyrank -10 0]
+ assert_equal {b c d e} [r zrange zset 0 -1]
+
+ # start overflow
+ assert_equal 0 [remrangebyrank 10 -1]
+ assert_equal {a b c d e} [r zrange zset 0 -1]
+
+ # end underflow
+ assert_equal 0 [remrangebyrank 0 -10]
+ assert_equal {a b c d e} [r zrange zset 0 -1]
+
+ # end overflow
+ assert_equal 5 [remrangebyrank 0 10]
+ assert_equal {} [r zrange zset 0 -1]
+ }
test {ZUNIONSTORE against non-existing key doesn't set destination} {
r del zseta

0 comments on commit 797c9ff

Please sign in to comment.