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

diff --git a/src/sort.c b/src/sort.c
index f70810b9baeeef1a66eb4b6500c0f5d25c8e2e17..1a8376a8bf59e2580b1e1b1dd33be6458334681a 100644 (file)
--- a/src/sort.c
+++ b/src/sort.c
@@ -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]);
 
     /* 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);
         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 */
     /* 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) {
 
     /* 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. */
     }
 
     /* 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) {
 
     /* Load the sorting vector with all the objects to sort */
     switch(sortval->type) {


@@ -367,12 +367,9 @@ void sortCommand(redisClient *c) {
                 }
             }
         }
                 }
             }
         }

-        setKey(c->db,storekey,sobj);
+        if (outputlen) setKey(c->db,storekey,sobj);

         decrRefCount(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);
     }
 
         addReplyLongLong(c,outputlen);
     }
 

diff --git a/tests/unit/sort.tcl b/tests/unit/sort.tcl
index 3a4c855f59fcf8bbca05cd3dcf083d680d0b266d..e474e50cf19557c1730388407a4edf877273a5ef 100644 (file)
--- a/tests/unit/sort.tcl
+++ b/tests/unit/sort.tcl
@@ -134,6 +134,18 @@ start_server {
         assert_equal [lsort -real $floats] [r sort mylist]
     }
 
         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]
     tags {"slow"} {
         set num 100
         set res [create_random_dataset $num lpush]