Introduce subcommand as a first-class command abstraction
- Dominant language
- C++
- Stars
- 4.4k
- Forks
- 658
- Avg merge
- 1d 20h
- Merged PRs (30d)
- 10
Description
### Search before asking
- [x] I had searched in the issues and found no similar framework-level issue. I found issues about specific subcommands or documentation, but not one about making subcommands a first-class command abstraction.
### 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?
- [ ] I'm willing to submit a PR!
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.