From 548efd91e5a33e1358213a902b8533d88f40f7cf Mon Sep 17 00:00:00 2001 From: antirez Date: Tue, 31 Jan 2012 16:09:21 +0100 Subject: [PATCH] Order output of commands returning random arrays using table.sort when called from Lua, partially fixing issue #165. The issue is yet not completely fixed since we can't add the REDIS_CMD_SORT_FOR_SCRIPT flag in SORT currently, both because it may contain NULLs and because it is not cool to re-sort everything at every call when instead this should be sorted only if BY is used. --- src/redis.c | 20 ++++++++++++-------- src/redis.h | 1 + src/scripting.c | 24 +++++++++++++++++++++++- 3 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/redis.c b/src/redis.c index e89a9c581..a76a56185 100644 --- a/src/redis.c +++ b/src/redis.c @@ -102,7 +102,10 @@ struct redisCommand *commandTable; * s: command not allowed in scripts. * R: random command. Command is not deterministic, that is, the same command * with the same arguments, with the same key space, may have different - * results. For instance SPOP and RANDOMKEY are two random commands. */ + * results. For instance SPOP and RANDOMKEY are two random commands. + * S: Sort command output array if called from script, so that the output + * is deterministic. + */ struct redisCommand redisCommandTable[] = { {"get",getCommand,2,"r",0,NULL,1,1,1,0,0}, {"set",setCommand,3,"wm",0,noPreloadGetKeys,1,1,1,0,0}, @@ -145,13 +148,13 @@ struct redisCommand redisCommandTable[] = { {"scard",scardCommand,2,"r",0,NULL,1,1,1,0,0}, {"spop",spopCommand,2,"wRs",0,NULL,1,1,1,0,0}, {"srandmember",srandmemberCommand,2,"rR",0,NULL,1,1,1,0,0}, - {"sinter",sinterCommand,-2,"r",0,NULL,1,-1,1,0,0}, + {"sinter",sinterCommand,-2,"rS",0,NULL,1,-1,1,0,0}, {"sinterstore",sinterstoreCommand,-3,"wm",0,NULL,1,-1,1,0,0}, - {"sunion",sunionCommand,-2,"r",0,NULL,1,-1,1,0,0}, + {"sunion",sunionCommand,-2,"rS",0,NULL,1,-1,1,0,0}, {"sunionstore",sunionstoreCommand,-3,"wm",0,NULL,1,-1,1,0,0}, - {"sdiff",sdiffCommand,-2,"r",0,NULL,1,-1,1,0,0}, + {"sdiff",sdiffCommand,-2,"rS",0,NULL,1,-1,1,0,0}, {"sdiffstore",sdiffstoreCommand,-3,"wm",0,NULL,1,-1,1,0,0}, - {"smembers",sinterCommand,2,"r",0,NULL,1,1,1,0,0}, + {"smembers",sinterCommand,2,"rS",0,NULL,1,1,1,0,0}, {"zadd",zaddCommand,-4,"wm",0,NULL,1,1,1,0,0}, {"zincrby",zincrbyCommand,4,"wm",0,NULL,1,1,1,0,0}, {"zrem",zremCommand,-3,"w",0,NULL,1,1,1,0,0}, @@ -177,8 +180,8 @@ struct redisCommand redisCommandTable[] = { {"hincrbyfloat",hincrbyfloatCommand,4,"wm",0,NULL,1,1,1,0,0}, {"hdel",hdelCommand,-3,"w",0,NULL,1,1,1,0,0}, {"hlen",hlenCommand,2,"r",0,NULL,1,1,1,0,0}, - {"hkeys",hkeysCommand,2,"r",0,NULL,1,1,1,0,0}, - {"hvals",hvalsCommand,2,"r",0,NULL,1,1,1,0,0}, + {"hkeys",hkeysCommand,2,"rS",0,NULL,1,1,1,0,0}, + {"hvals",hvalsCommand,2,"rS",0,NULL,1,1,1,0,0}, {"hgetall",hgetallCommand,2,"r",0,NULL,1,1,1,0,0}, {"hexists",hexistsCommand,3,"r",0,NULL,1,1,1,0,0}, {"incrby",incrbyCommand,3,"wm",0,NULL,1,1,1,0,0}, @@ -196,7 +199,7 @@ struct redisCommand redisCommandTable[] = { {"expireat",expireatCommand,3,"w",0,NULL,1,1,1,0,0}, {"pexpire",pexpireCommand,3,"w",0,NULL,1,1,1,0,0}, {"pexpireat",pexpireatCommand,3,"w",0,NULL,1,1,1,0,0}, - {"keys",keysCommand,2,"r",0,NULL,0,0,0,0,0}, + {"keys",keysCommand,2,"rS",0,NULL,0,0,0,0,0}, {"dbsize",dbsizeCommand,1,"r",0,NULL,0,0,0,0,0}, {"auth",authCommand,2,"rs",0,NULL,0,0,0,0,0}, {"ping",pingCommand,1,"r",0,NULL,0,0,0,0,0}, @@ -1118,6 +1121,7 @@ void populateCommandTable(void) { case 'f': c->flags |= REDIS_CMD_FORCE_REPLICATION; break; case 's': c->flags |= REDIS_CMD_NOSCRIPT; break; case 'R': c->flags |= REDIS_CMD_RANDOM; break; + case 'S': c->flags |= REDIS_CMD_SORT_FOR_SCRIPT; break; default: redisPanic("Unsupported command flag"); break; } f++; diff --git a/src/redis.h b/src/redis.h index 615916fe7..3a5e3d4a8 100644 --- a/src/redis.h +++ b/src/redis.h @@ -77,6 +77,7 @@ #define REDIS_CMD_PUBSUB 32 /* "p" flag */ #define REDIS_CMD_NOSCRIPT 64 /* "s" flag */ #define REDIS_CMD_RANDOM 128 /* "R" flag */ +#define REDIS_CMD_SORT_FOR_SCRIPT 256 /* "S" flag */ /* Object types */ #define REDIS_STRING 0 diff --git a/src/scripting.c b/src/scripting.c index 255184e7d..92fb928a2 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -27,7 +27,7 @@ int redis_math_randomseed (lua_State *L); * is like a normal client that bypasses all the slow I/O paths. * * Note: in this function we do not do any sanity check as the reply is - * generated by Redis directly. This allows use to go faster. + * generated by Redis directly. This allows us to go faster. * The reply string can be altered during the parsing as it is discared * after the conversion is completed. * @@ -128,6 +128,22 @@ void luaPushError(lua_State *lua, char *error) { lua_settable(lua,-3); } +/* Sort the array currently in the stack. We do this to make the output + * of commands like KEYS or SMEMBERS something deterministic when called + * from Lua (to play well with AOf/replication). + * + * The array is sorted using table.sort itself, and assuming all the + * list elements are strings. */ +void luaSortArray(lua_State *lua) { + /* Initial Stack: array */ + lua_getglobal(lua,"table"); + lua_pushstring(lua,"sort"); + lua_gettable(lua,-2); /* Stack: array, table, table.sort */ + lua_pushvalue(lua,-3); /* Stack: array, table, table.sort, array */ + lua_call(lua,1,0); /* Stack: array (sorted), table */ + lua_pop(lua,1); /* Stack: array (sorted) */ +} + int luaRedisGenericCommand(lua_State *lua, int raise_error) { int j, argc = lua_gettop(lua); struct redisCommand *cmd; @@ -208,6 +224,12 @@ int luaRedisGenericCommand(lua_State *lua, int raise_error) { } if (raise_error && reply[0] != '-') raise_error = 0; redisProtocolToLuaType(lua,reply); + /* Sort the output array if needed, assuming it is a non-null multi bulk + * reply as expected. */ + if ((cmd->flags & REDIS_CMD_SORT_FOR_SCRIPT) && + (reply[0] == '*' && reply[1] != '-')) { + luaSortArray(lua); + } sdsfree(reply); cleanup: