Search before asking
Motivation
Kvrocks has many Redis-compatible command families whose behavior is selected by the second token, such as COMMAND GETKEYS, CONFIG GET/SET, CLIENT LIST/KILL, CLUSTER *, SCRIPT *, and NAMESPACE GET/SET/ADD/DEL/CURRENT.
Currently, these subcommands are usually handled inside the root command implementation by manually parsing args_[1]. This makes subcommands second-class from the command framework's point of view:
- Subcommands cannot have their own registered metadata, such as arity, flags, key specs, and category.
- Command resolution, key extraction, command renaming, Lua command invocation, and command introspection have to reason about only the root command.
- Root command implementations tend to become large dispatchers instead of focused command handlers.
- It is harder to keep compatibility behavior consistent when different command families implement their own subcommand parsing rules.
A previous implementation attempt in #3413 demonstrated that subcommands can be represented explicitly in the command table and used by command lookup paths. It would be useful to formalize this direction as an enhancement.
Solution
Introduce subcommand as a first-class abstraction in the command framework.
A possible implementation could include:
- Add subcommand registration APIs, for example
MakeSubCmdAttr(parent, subcommand, ...).
- Store subcommand metadata separately from root command metadata while keeping a canonical internal name such as
namespace|get.
- Add command resolution based on the full command token vector, for example resolving
{"namespace", "get", "ns"} to the namespace|get attributes while preserving namespace as the root command.
- Update command creation and lookup paths to use the resolved command attributes:
- client command execution
- Lua scripting command invocation
COMMAND GETKEYS
- command renaming paths
- Allow each subcommand to define its own arity, flags, key range, and handler class.
- Refactor one existing command family, such as
NAMESPACE, as the initial reference implementation.
- Preserve existing externally visible behavior, including legacy error messages for unknown or invalid subcommands.
The implementation in #3413 can be used as a reference. In particular, it covered:
CommandTable::Resolve(...)
- subcommand registration and metadata lookup
- dedicated
NAMESPACE subcommand handlers
- renamed root command resolution
- key extraction for registered subcommands
- C++ and Go regression tests
Non-goals
- This should not change public command behavior unless required for correctness.
- This should not add new Redis commands by itself.
- This should not force all command families to migrate in a single PR. It is reasonable to introduce the framework first and migrate command families incrementally.
Suggested tests
- C++ unit tests for root command resolution, registered subcommand resolution, unknown subcommand fallback, arity checks, key range extraction, and renamed root command behavior.
- Go integration tests for externally visible behavior, especially:
COMMAND GETKEYS with subcommands
- command renaming with subcommands
NAMESPACE legacy error behavior
- Lua invocation of commands with subcommands
Are you willing to submit a PR?
Search before asking
Motivation
Kvrocks has many Redis-compatible command families whose behavior is selected by the second token, such as
COMMAND GETKEYS,CONFIG GET/SET,CLIENT LIST/KILL,CLUSTER *,SCRIPT *, andNAMESPACE GET/SET/ADD/DEL/CURRENT.Currently, these subcommands are usually handled inside the root command implementation by manually parsing
args_[1]. This makes subcommands second-class from the command framework's point of view:A previous implementation attempt in #3413 demonstrated that subcommands can be represented explicitly in the command table and used by command lookup paths. It would be useful to formalize this direction as an enhancement.
Solution
Introduce subcommand as a first-class abstraction in the command framework.
A possible implementation could include:
MakeSubCmdAttr(parent, subcommand, ...).namespace|get.{"namespace", "get", "ns"}to thenamespace|getattributes while preservingnamespaceas the root command.COMMAND GETKEYSNAMESPACE, as the initial reference implementation.The implementation in #3413 can be used as a reference. In particular, it covered:
CommandTable::Resolve(...)NAMESPACEsubcommand handlersNon-goals
Suggested tests
COMMAND GETKEYSwith subcommandsNAMESPACElegacy error behaviorAre you willing to submit a PR?