From 184d74abc628a1fccf66d34505a8c5505f82d21b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 15 Jun 2010 15:40:28 +0200 Subject: [PATCH 1/3] more tests for zrange and zrevrange; fix behavior for out-of-range negative end index --- redis.c | 5 ++- tests/unit/type/zset.tcl | 69 ++++++++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 12 deletions(-) diff --git a/redis.c b/redis.c index f6a765da..30b4f6f1 100644 --- a/redis.c +++ b/redis.c @@ -6662,11 +6662,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; } diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index 60459783..1155de1e 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -17,6 +17,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 +129,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} { From 4774a53b247bb1f75fb175039c6af70a61462a94 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 15 Jun 2010 16:21:42 +0200 Subject: [PATCH 2/3] fix behavior for out-of-range negative end index on ZREMRANGEBYRANK --- redis.c | 4 ++-- tests/unit/type/zset.tcl | 42 +++++++++++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/redis.c b/redis.c index 30b4f6f1..6ac410d0 100644 --- a/redis.c +++ b/redis.c @@ -6409,9 +6409,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; diff --git a/tests/unit/type/zset.tcl b/tests/unit/type/zset.tcl index 1155de1e..da264845 100644 --- a/tests/unit/type/zset.tcl +++ b/tests/unit/type/zset.tcl @@ -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 @@ -339,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 From f483ce5ffef0beb6e5ab50987691d00166e8216b Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Thu, 24 Jun 2010 15:12:42 -0700 Subject: [PATCH 3/3] fix unexpected behavior on an out of range end index for LRANGE and LTRIM --- redis.c | 8 ++++---- tests/unit/type/list.tcl | 11 +++++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/redis.c b/redis.c index 6ac410d0..085d3553 100644 --- a/redis.c +++ b/redis.c @@ -5404,9 +5404,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); @@ -5444,9 +5444,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; diff --git a/tests/unit/type/list.tcl b/tests/unit/type/list.tcl index ecae5d22..8a7ea278 100644 --- a/tests/unit/type/list.tcl +++ b/tests/unit/type/list.tcl @@ -340,6 +340,12 @@ start_server { create_$type mylist {1 2 3} assert_equal {1 2 3} [r lrange mylist -1000 1000] } + + test "LRANGE out of range negative end index - $type" { + create_$type mylist {1 2 3} + assert_equal {1} [r lrange mylist 0 -3] + assert_equal {} [r lrange mylist 0 -4] + } } test {LRANGE against non existing key} { @@ -369,6 +375,11 @@ start_server { assert_equal {1 2 3 4 5} [trim_list $type 0 10] } + test "LTRIM out of range negative end index - $type" { + assert_equal {1} [trim_list $type 0 -5] + assert_equal {} [trim_list $type 0 -6] + } + tags {"slow"} { test "LTRIM stress testing - $type" { set mylist {}