Dediacted member to hold RedisModuleCommand (#10681)

Fix #10552

We no longer piggyback getkeys_proc to hold the RedisModuleCommand struct, when exists

Others:
Use `doesCommandHaveKeys` in `RM_GetCommandKeysWithFlags` and `getKeysSubcommandImpl`.
It causes a very minor behavioral change in commands that don't have actual keys, but have a spec
with `CMD_KEY_NOT_KEY`.
For example, before this command `COMMAND GETKEYS SPUBLISH` would return
`Invalid arguments specified for command` but not it returns `The command has no key arguments`
This commit is contained in:
guybe7 2022-05-10 13:56:12 +02:00 committed by GitHub
parent c2d8d4e648
commit 815a6f846a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 19 additions and 24 deletions

View File

@ -1878,7 +1878,6 @@ int getKeysFromCommandWithSpecs(struct redisCommand *cmd, robj **argv, int argc,
/* Flags indicating that we have a getkeys callback */
int has_module_getkeys = cmd->flags & CMD_MODULE_GETKEYS;
int has_native_getkeys = !(cmd->flags & CMD_MODULE) && cmd->getkeys_proc;
/* The key-spec that's auto generated by RM_CreateCommand sets VARIABLE_FLAGS since no flags are given.
* If the module provides getkeys callback, we'll prefer it, but if it didn't, we'll use key-spec anyway. */
@ -1900,14 +1899,14 @@ int getKeysFromCommandWithSpecs(struct redisCommand *cmd, robj **argv, int argc,
/* We use native getkeys as a last resort, since not all these native getkeys provide
* flags properly (only the ones that correspond to INVALID, INCOMPLETE or VARIABLE_FLAGS do.*/
if (has_native_getkeys)
if (cmd->getkeys_proc)
return cmd->getkeys_proc(cmd,argv,argc,result);
return 0;
}
/* This function returns a sanity check if the command may have keys. */
int doesCommandHaveKeys(struct redisCommand *cmd) {
return (!(cmd->flags & CMD_MODULE) && cmd->getkeys_proc) || /* has getkeys_proc (non modules) */
return cmd->getkeys_proc || /* has getkeys_proc (non modules) */
(cmd->flags & CMD_MODULE_GETKEYS) || /* module with GETKEYS */
(getAllKeySpecsFlags(cmd, 1) & CMD_KEY_NOT_KEY); /* has at least one key-spec not marked as NOT_KEY */
}
@ -2056,7 +2055,7 @@ int getKeysUsingLegacyRangeSpec(struct redisCommand *cmd, robj **argv, int argc,
int getKeysFromCommand(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result) {
if (cmd->flags & CMD_MODULE_GETKEYS) {
return moduleGetCommandKeysViaAPI(cmd,argv,argc,result);
} else if (!(cmd->flags & CMD_MODULE) && cmd->getkeys_proc) {
} else if (cmd->getkeys_proc) {
return cmd->getkeys_proc(cmd,argv,argc,result);
} else {
return getKeysUsingLegacyRangeSpec(cmd,argv,argc,result);

View File

@ -781,7 +781,7 @@ void moduleCreateContext(RedisModuleCtx *out_ctx, RedisModule *module, int ctx_f
/* This Redis command binds the normal Redis command invocation with commands
* exported by modules. */
void RedisModuleCommandDispatcher(client *c) {
RedisModuleCommand *cp = (void*)(unsigned long)c->cmd->getkeys_proc;
RedisModuleCommand *cp = c->cmd->module_cmd;
RedisModuleCtx ctx;
moduleCreateContext(&ctx, cp->module, REDISMODULE_CTX_NONE);
@ -816,7 +816,7 @@ void RedisModuleCommandDispatcher(client *c) {
* the context in a way that the command can recognize this is a special
* "get keys" call by calling RedisModule_IsKeysPositionRequest(ctx). */
int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result) {
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
RedisModuleCtx ctx;
moduleCreateContext(&ctx, cp->module, REDISMODULE_CTX_KEYS_POS_REQUEST);
@ -836,7 +836,7 @@ int moduleGetCommandKeysViaAPI(struct redisCommand *cmd, robj **argv, int argc,
* moduleGetCommandKeysViaAPI, for modules that declare "getchannels-api"
* during registration. Unlike keys, this is the only way to declare channels. */
int moduleGetCommandChannelsViaAPI(struct redisCommand *cmd, robj **argv, int argc, getKeysResult *result) {
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
RedisModuleCtx ctx;
moduleCreateContext(&ctx, cp->module, REDISMODULE_CTX_CHANNELS_POS_REQUEST);
@ -1130,10 +1130,7 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, sds dec
/* Create a command "proxy", which is a structure that is referenced
* in the command table, so that the generic command that works as
* binding between modules and Redis, can know what function to call
* and what the module is.
*
* Note that we use the Redis command table 'getkeys_proc' in order to
* pass a reference to the command proxy structure. */
* and what the module is. */
cp = zcalloc(sizeof(*cp));
cp->module = module;
cp->func = cmdfunc;
@ -1143,7 +1140,7 @@ RedisModuleCommand *moduleCreateCommandProxy(struct RedisModule *module, sds dec
cp->rediscmd->group = COMMAND_GROUP_MODULE;
cp->rediscmd->proc = RedisModuleCommandDispatcher;
cp->rediscmd->flags = flags | CMD_MODULE;
cp->rediscmd->getkeys_proc = (redisGetKeysProc*)(unsigned long)cp;
cp->rediscmd->module_cmd = cp;
cp->rediscmd->key_specs_max = STATIC_KEY_SPECS_NUM;
cp->rediscmd->key_specs = cp->rediscmd->key_specs_static;
if (firstkey != 0) {
@ -1186,7 +1183,7 @@ RedisModuleCommand *RM_GetCommand(RedisModuleCtx *ctx, const char *name) {
if (!cmd || !(cmd->flags & CMD_MODULE))
return NULL;
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
if (cp->module != ctx->module)
return NULL;
@ -1230,7 +1227,7 @@ int RM_CreateSubcommand(RedisModuleCommand *parent, const char *name, RedisModul
if (parent_cmd->parent)
return REDISMODULE_ERR; /* We don't allow more than one level of subcommands */
RedisModuleCommand *parent_cp = (void*)(unsigned long)parent_cmd->getkeys_proc;
RedisModuleCommand *parent_cp = parent_cmd->module_cmd;
if (parent_cp->func)
return REDISMODULE_ERR; /* A parent command should be a pure container of subcommands */
@ -1977,7 +1974,7 @@ int moduleIsModuleCommand(void *module_handle, struct redisCommand *cmd) {
return 0;
if (module_handle == NULL)
return 0;
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
return (cp->module == module_handle);
}
@ -6088,7 +6085,7 @@ const char *moduleTypeModuleName(moduleType *mt) {
const char *moduleNameFromCommand(struct redisCommand *cmd) {
serverAssert(cmd->proc == RedisModuleCommandDispatcher);
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
return cp->module->name;
}
@ -10936,7 +10933,7 @@ int moduleFreeCommand(struct RedisModule *module, struct redisCommand *cmd) {
if (cmd->proc != RedisModuleCommandDispatcher)
return C_ERR;
RedisModuleCommand *cp = (void*)(unsigned long)cmd->getkeys_proc;
RedisModuleCommand *cp = cmd->module_cmd;
if (cp->module != module)
return C_ERR;
@ -12020,7 +12017,7 @@ int *RM_GetCommandKeysWithFlags(RedisModuleCtx *ctx, RedisModuleString **argv, i
}
/* Bail out if command has no keys */
if (cmd->getkeys_proc == NULL && cmd->key_specs_num == 0) {
if (!doesCommandHaveKeys(cmd)) {
errno = 0;
return NULL;
}

View File

@ -3482,11 +3482,8 @@ void afterCommand(client *c) {
* spec doesn't cover all of them. */
void populateCommandMovableKeys(struct redisCommand *cmd) {
int movablekeys = 0;
if (cmd->getkeys_proc && !(cmd->flags & CMD_MODULE)) {
/* Redis command with getkeys proc */
movablekeys = 1;
} else if (cmd->flags & CMD_MODULE_GETKEYS) {
/* Module command with getkeys proc */
if (cmd->getkeys_proc || (cmd->flags & CMD_MODULE_GETKEYS)) {
/* Command with getkeys proc */
movablekeys = 1;
} else {
/* Redis command without getkeys proc, but possibly has
@ -4709,7 +4706,7 @@ void getKeysSubcommandImpl(client *c, int with_flags) {
if (!cmd) {
addReplyError(c,"Invalid command specified");
return;
} else if (cmd->getkeys_proc == NULL && cmd->key_specs_num == 0) {
} else if (!doesCommandHaveKeys(cmd)) {
addReplyError(c,"The command has no key arguments");
return;
} else if ((cmd->arity > 0 && cmd->arity != c->argc-2) ||

View File

@ -676,6 +676,7 @@ struct redisObject;
struct RedisModuleDefragCtx;
struct RedisModuleInfoCtx;
struct RedisModuleKeyOptCtx;
struct RedisModuleCommand;
/* Each module type implementation should export a set of methods in order
* to serialize and deserialize the value in the RDB file, rewrite the AOF
@ -2260,6 +2261,7 @@ struct redisCommand {
dict *subcommands_dict; /* A dictionary that holds the subcommands, the key is the subcommand sds name
* (not the fullname), and the value is the redisCommand structure pointer. */
struct redisCommand *parent;
struct RedisModuleCommand *module_cmd; /* A pointer to the module command data (NULL if native command) */
};
struct redisError {