[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1047: fix: supports multiple rename-commands

[GitBox <gi...@apache.org>]

git-hulk commented on code in PR #1047:
URL: https://github.com/apache/incubator-kvrocks/pull/1047#discussion_r1006978648


##########
src/config/config.cc:
##########
@@ -273,23 +273,26 @@ void Config::initFieldValidator() {
        }},
       {"rename-command",
        [](const std::string &k, const std::string &v) -> Status {
-         std::vector<std::string> args = Util::Split(v, " \t");
-         if (args.size() != 2) {
-           return Status(Status::NotOK, "Invalid rename-command format");
-         }
-         auto commands = Redis::GetCommands();
-         auto cmd_iter = commands->find(Util::ToLower(args[0]));
-         if (cmd_iter == commands->end()) {
-           return Status(Status::NotOK, "No such command in rename-command");
-         }
-         if (args[1] != "\"\"") {
-           auto new_command_name = Util::ToLower(args[1]);
-           if (commands->find(new_command_name) != commands->end()) {
-             return Status(Status::NotOK, "Target command name already exists");
+         std::vector<std::string> all_args = Util::Split(v, "\n");

Review Comment:
   The rename command should not support:
   ```
   rename-command a b
   c d
   e f 
   ``` 
   so it makes no sense to split with newline here?



##########
src/config/config.cc:
##########
@@ -722,8 +725,15 @@ void Config::Get(std::string key, std::vector<std::string> *values) {
   values->clear();
   for (const auto &iter : fields_) {
     if (key == "*" || Util::ToLower(key) == iter.first) {
-      values->emplace_back(iter.first);
-      values->emplace_back(iter.second->ToString());
+      if (iter.second->GetConfigType() == configType::MultiConfig) {

Review Comment:
   Can you add a test case for this line, it looks not correct for the rename-command now.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org