Permalink
Browse files

Fixed SORT bugs (issue #224) with regression tests.

  • Loading branch information...
1 parent 67d6b29 commit bc62bc5eaceba9d3a3b84c01121b12c01f22643b @antirez committed Dec 1, 2011
Showing with 21 additions and 12 deletions.
  1. +9 −12 src/sort.c
  2. +12 −0 tests/unit/sort.tcl
View
@@ -141,11 +141,7 @@ void sortCommand(redisClient *c) {
/* Lookup the key to sort. It must be of the right types */
sortval = lookupKeyRead(c->db,c->argv[1]);
- if (sortval == NULL) {
- addReply(c,shared.emptymultibulk);
- return;
- }
- if (sortval->type != REDIS_SET && sortval->type != REDIS_LIST &&
+ if (sortval && sortval->type != REDIS_SET && sortval->type != REDIS_LIST &&
sortval->type != REDIS_ZSET)
{
addReply(c,shared.wrongtypeerr);
@@ -161,7 +157,10 @@ void sortCommand(redisClient *c) {
/* Now we need to protect sortval incrementing its count, in the future
* SORT may have options able to overwrite/delete keys during the sorting
* and the sorted key itself may get destroied */
- incrRefCount(sortval);
+ if (sortval)
+ incrRefCount(sortval);
+ else
+ sortval = createListObject();
/* The SORT command has an SQL-alike syntax, parse it */
while(j < c->argc) {
@@ -200,7 +199,8 @@ void sortCommand(redisClient *c) {
}
/* Destructively convert encoded sorted sets for SORT. */
- if (sortval->type == REDIS_ZSET) zsetConvert(sortval, REDIS_ENCODING_SKIPLIST);
+ if (sortval->type == REDIS_ZSET)
+ zsetConvert(sortval, REDIS_ENCODING_SKIPLIST);
/* Load the sorting vector with all the objects to sort */
switch(sortval->type) {
@@ -366,12 +366,9 @@ void sortCommand(redisClient *c) {
}
}
}
- setKey(c->db,storekey,sobj);
+ if (outputlen) setKey(c->db,storekey,sobj);
decrRefCount(sobj);
- /* Note: we add 1 because the DB is dirty anyway since even if the
- * SORT result is empty a new key is set and maybe the old content
- * replaced. */
- server.dirty += 1+outputlen;
+ server.dirty += outputlen;
addReplyLongLong(c,outputlen);
}
View
@@ -134,6 +134,18 @@ start_server {
assert_equal [lsort -real $floats] [r sort mylist]
}
+ test "SORT with STORE returns zero if result is empty (github isse 224)" {
+ r flushdb
+ r sort foo store bar
+ } {0}
+
+ test "SORT with STORE does not create empty lists (github issue 224)" {
+ r flushdb
+ r lpush foo bar
+ r sort foo limit 10 10 store zap
+ r exists zap
+ } {0}
+
tags {"slow"} {
set num 100
set res [create_random_dataset $num lpush]

0 comments on commit bc62bc5

Please sign in to comment.