From 6c0abc4a0ee1450a7fc13076f5d04a6549286cf3 Mon Sep 17 00:00:00 2001 From: Matt Stancliff Date: Thu, 23 Oct 2014 11:52:35 -0400 Subject: [PATCH] Fix zero-ordering SORT when called against lists People mostly use SORT against lists, but our prior behavior was pretending lists were an unordered bag requiring a forced-sort when no sort was requested. We can just use the native list ordering to ensure consistency across replicaion and scripting calls. Closes #2079 Closes #545 (again) --- src/sort.c | 15 ++++++--------- tests/unit/sort.tcl | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/sort.c b/src/sort.c index fedf0cf3a..d3521e663 100644 --- a/src/sort.c +++ b/src/sort.c @@ -285,16 +285,13 @@ void sortCommand(redisClient *c) { return; } - /* For the STORE option, or when SORT is called from a Lua script, - * we want to force a specific ordering even when no explicit ordering - * was asked (SORT BY nosort). This guarantees that replication / AOF - * is deterministic. + /* When sorting a set with no sort specified, we must sort the output + * so the result is consistent across scripting and replication. * - * However in the case 'dontsort' is true, but the type to sort is a - * sorted set, we don't need to do anything as ordering is guaranteed - * in this special case. */ - if ((storekey || c->flags & REDIS_LUA_CLIENT) && - (dontsort && sortval->type != REDIS_ZSET)) + * The other types (list, sorted set) will retain their native order + * even if no sort order is requested, so they remain stable across + * scripting and replication. */ + if ((dontsort && sortval->type == REDIS_SET)) { /* Force ALPHA sorting */ dontsort = 0; diff --git a/tests/unit/sort.tcl b/tests/unit/sort.tcl index 490158f14..8ebd75965 100644 --- a/tests/unit/sort.tcl +++ b/tests/unit/sort.tcl @@ -246,6 +246,24 @@ start_server { r sort mylist by num get x:*-> } {100} + test "SORT by nosort retains native order for lists" { + r del testa + r lpush testa 2 1 4 3 5 + r sort testa by nosort + } {5 3 4 1 2} + + test "SORT by nosort plus store retains native order for lists" { + r del testa + r lpush testa 2 1 4 3 5 + r sort testa by nosort store testb + r lrange testb 0 -1 + } {5 3 4 1 2} + + test "SORT by nosort with limit returns based on original list order" { + r sort testa by nosort limit 0 3 store testb + r lrange testb 0 -1 + } {5 3 4} + tags {"slow"} { set num 100 set res [create_random_dataset $num lpush]