Unverified Commit f11a2d4d authored by guybe7's avatar guybe7 Committed by GitHub
Browse files

Fix COMMAND GETKEYS on EVAL without keys (#9733)

Add new no-mandatory-keys flag to support COMMAND GETKEYS of commands
which have no mandatory keys.

In the past we would have got this error:
```
127.0.0.1:6379> command getkeys eval "return 1" 0
(error) ERR Invalid arguments specified for command
```
parent 77d3c6bf
...@@ -803,6 +803,7 @@ int64_t commandFlagsFromString(char *s) { ...@@ -803,6 +803,7 @@ int64_t commandFlagsFromString(char *s) {
else if (!strcasecmp(t,"may-replicate")) flags |= CMD_MAY_REPLICATE; else if (!strcasecmp(t,"may-replicate")) flags |= CMD_MAY_REPLICATE;
else if (!strcasecmp(t,"getkeys-api")) flags |= CMD_MODULE_GETKEYS; else if (!strcasecmp(t,"getkeys-api")) flags |= CMD_MODULE_GETKEYS;
else if (!strcasecmp(t,"no-cluster")) flags |= CMD_MODULE_NO_CLUSTER; else if (!strcasecmp(t,"no-cluster")) flags |= CMD_MODULE_NO_CLUSTER;
else if (!strcasecmp(t,"no-mandatory-keys")) flags |= CMD_NO_MANDATORY_KEYS;
else break; else break;
} }
sdsfreesplitres(tokens,count); sdsfreesplitres(tokens,count);
...@@ -891,6 +892,7 @@ RedisModuleCommandProxy *moduleCreateCommandProxy(RedisModuleCtx *ctx, const cha ...@@ -891,6 +892,7 @@ RedisModuleCommandProxy *moduleCreateCommandProxy(RedisModuleCtx *ctx, const cha
* to authenticate a client. * to authenticate a client.
* * **"may-replicate"**: This command may generate replication traffic, even * * **"may-replicate"**: This command may generate replication traffic, even
* though it's not a write command. * though it's not a write command.
* * **"no-mandatory-keys"**: All the keys this command may take are optional
* *
* The last three parameters specify which arguments of the new command are * The last three parameters specify which arguments of the new command are
* Redis keys. See https://redis.io/commands/command for more information. * Redis keys. See https://redis.io/commands/command for more information.
......
...@@ -175,6 +175,8 @@ struct redisServer server; /* Server global state */ ...@@ -175,6 +175,8 @@ struct redisServer server; /* Server global state */
* *
* sentinel-only: This command is present only when in sentinel mode. * sentinel-only: This command is present only when in sentinel mode.
* *
* no-mandatory-keys: This key arguments for this command are optional.
*
* The following additional flags are only used in order to put commands * The following additional flags are only used in order to put commands
* in a specific ACL category. Commands can have multiple ACL categories. * in a specific ACL category. Commands can have multiple ACL categories.
* See redis.conf for the exact meaning of each. * See redis.conf for the exact meaning of each.
...@@ -1751,28 +1753,28 @@ struct redisCommand redisCommandTable[] = { ...@@ -1751,28 +1753,28 @@ struct redisCommand redisCommandTable[] = {
* as opposed to after. * as opposed to after.
*/ */
{"eval",evalCommand,-3, {"eval",evalCommand,-3,
"no-script no-monitor may-replicate @scripting", "no-script no-monitor may-replicate no-mandatory-keys @scripting",
{{"read write", /* We pass both read and write because these flag are worst-case-scenario */ {{"read write", /* We pass both read and write because these flag are worst-case-scenario */
KSPEC_BS_INDEX,.bs.index={2}, KSPEC_BS_INDEX,.bs.index={2},
KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}}, KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}},
evalGetKeys}, evalGetKeys},
{"eval_ro",evalRoCommand,-3, {"eval_ro",evalRoCommand,-3,
"no-script no-monitor @scripting", "no-script no-monitor no-mandatory-keys @scripting",
{{"read", {{"read",
KSPEC_BS_INDEX,.bs.index={2}, KSPEC_BS_INDEX,.bs.index={2},
KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}}, KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}},
evalGetKeys}, evalGetKeys},
{"evalsha",evalShaCommand,-3, {"evalsha",evalShaCommand,-3,
"no-script no-monitor may-replicate @scripting", "no-script no-monitor may-replicate no-mandatory-keys @scripting",
{{"read write", /* We pass both read and write because these flag are worst-case-scenario */ {{"read write", /* We pass both read and write because these flag are worst-case-scenario */
KSPEC_BS_INDEX,.bs.index={2}, KSPEC_BS_INDEX,.bs.index={2},
KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}}, KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}},
evalGetKeys}, evalGetKeys},
{"evalsha_ro",evalShaRoCommand,-3, {"evalsha_ro",evalShaRoCommand,-3,
"no-script no-monitor @scripting", "no-script no-monitor no-mandatory-keys @scripting",
{{"read", {{"read",
KSPEC_BS_INDEX,.bs.index={2}, KSPEC_BS_INDEX,.bs.index={2},
KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}}, KSPEC_FK_KEYNUM,.fk.keynum={0,1,1}}},
...@@ -4510,6 +4512,8 @@ void parseCommandFlags(struct redisCommand *c, char *strflags) { ...@@ -4510,6 +4512,8 @@ void parseCommandFlags(struct redisCommand *c, char *strflags) {
} else if (!strcasecmp(flag,"only-sentinel")) { } else if (!strcasecmp(flag,"only-sentinel")) {
c->flags |= CMD_SENTINEL; /* Obviously it's s sentinel command */ c->flags |= CMD_SENTINEL; /* Obviously it's s sentinel command */
c->flags |= CMD_ONLY_SENTINEL; c->flags |= CMD_ONLY_SENTINEL;
} else if (!strcasecmp(flag,"no-mandatory-keys")) {
c->flags |= CMD_NO_MANDATORY_KEYS;
} else { } else {
/* Parse ACL categories here if the flag name starts with @. */ /* Parse ACL categories here if the flag name starts with @. */
uint64_t catflag; uint64_t catflag;
...@@ -5906,7 +5910,11 @@ void getKeysSubcommand(client *c) { ...@@ -5906,7 +5910,11 @@ void getKeysSubcommand(client *c) {
} }
if (!getKeysFromCommand(cmd,c->argv+2,c->argc-2,&result)) { if (!getKeysFromCommand(cmd,c->argv+2,c->argc-2,&result)) {
addReplyError(c,"Invalid arguments specified for command"); if (cmd->flags & CMD_NO_MANDATORY_KEYS) {
addReplyArrayLen(c,0);
} else {
addReplyError(c,"Invalid arguments specified for command");
}
} else { } else {
addReplyArrayLen(c,result.numkeys); addReplyArrayLen(c,result.numkeys);
for (j = 0; j < result.numkeys; j++) addReplyBulk(c,c->argv[result.keys[j]+2]); for (j = 0; j < result.numkeys; j++) addReplyBulk(c,c->argv[result.keys[j]+2]);
......
...@@ -236,6 +236,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT]; ...@@ -236,6 +236,7 @@ extern int configOOMScoreAdjValuesDefaults[CONFIG_OOM_COUNT];
#define CMD_SENTINEL (1ULL<<40) /* "sentinel" flag */ #define CMD_SENTINEL (1ULL<<40) /* "sentinel" flag */
#define CMD_ONLY_SENTINEL (1ULL<<41) /* "only-sentinel" flag */ #define CMD_ONLY_SENTINEL (1ULL<<41) /* "only-sentinel" flag */
#define CMD_NO_MANDATORY_KEYS (1ULL<<42) /* "no-mandatory-keys" flag */
/* AOF states */ /* AOF states */
#define AOF_OFF 0 /* AOF is off */ #define AOF_OFF 0 /* AOF is off */
......
...@@ -89,6 +89,14 @@ start_server {tags {"introspection"}} { ...@@ -89,6 +89,14 @@ start_server {tags {"introspection"}} {
assert_equal {key} [r command getkeys xgroup create key groupname $] assert_equal {key} [r command getkeys xgroup create key groupname $]
} }
test {COMMAND GETKEYS EVAL with keys} {
assert_equal {key} [r command getkeys eval "return 1" 1 key]
}
test {COMMAND GETKEYS EVAL without keys} {
assert_equal {} [r command getkeys eval "return 1" 0]
}
test "COMMAND LIST FILTERBY ACLCAT" { test "COMMAND LIST FILTERBY ACLCAT" {
set reply [r command list filterby aclcat hyperloglog] set reply [r command list filterby aclcat hyperloglog]
assert_equal [lsort $reply] {pfadd pfcount pfdebug pfmerge pfselftest} assert_equal [lsort $reply] {pfadd pfcount pfdebug pfmerge pfselftest}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment