You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "MizukiCry (via GitHub)" <gi...@apache.org> on 2023/05/23 16:39:55 UTC

[GitHub] [incubator-kvrocks] MizukiCry opened a new pull request, #1468: Add the support of the ZMPOP command

MizukiCry opened a new pull request, #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468

   This PR supports the ZMPOP command like [Redis](https://redis.io/commands/zmpop/).
   
   Issue number: #1458 
   
   By the way, do I need to add test cases for it? Similar commands like `ZPOPMIN / MAX` also don't have corresponding test cases.


-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204123452


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,80 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    if (auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}); !parse_int) {
+      return parse_int.ToStatus();
+    } else {
+      numkeys_ = *parse_int;
+    }
+    for (int i = 0; i < numkeys_; ++i) {
+      if (!parser.Good()) {
+        return parser.InvalidSyntax();
+      }
+      keys_.emplace_back(*parser.TakeStr());

Review Comment:
   ```suggestion
         keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
   ```



-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204118366


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,80 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    if (auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}); !parse_int) {
+      return parse_int.ToStatus();

Review Comment:
   ```suggestion
       if (auto parse_int = parser.TakeInt<int>({1, std::numeric_limits<int>::max()}); !parse_int) {
         return parse_int;
   ```



-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204191087


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   Not precise. You should consider where the last position is.



-- 
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


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1563830827

   Others are good to me


-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204173540


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,80 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    if (auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}); !parse_int) {
+      return parse_int.ToStatus();
+    } else {
+      numkeys_ = *parse_int;
+    }

Review Comment:
   Fixed, but I'll get compile errors if removed `NumericRange<int>`



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204440357


##########
tests/gocase/unit/type/zset/zset_test.go:
##########
@@ -235,6 +235,62 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s
 		require.Equal(t, int64(3), rdb.ZRem(ctx, "ztmp", []string{"a", "b", "c", "d", "e", "f", "g"}).Val())
 	})
 
+	t.Run(fmt.Sprintf("ZPOPMIN basics - %s", encoding), func(t *testing.T) {
+		rdb.Del(ctx, "ztmp")
+		rdb.ZAdd(ctx, "ztmp", redis.Z{Score: 10, Member: "a"}, redis.Z{Score: 20, Member: "b"}, redis.Z{Score: 30, Member: "c"})
+		require.Equal(t, int64(3), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 10, Member: "a"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(2), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 20, Member: "b"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(1), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 30, Member: "c"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(0), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{}, rdb.ZPopMin(ctx, "ztmp").Val())
+		rdb.ZAdd(ctx, "ztmp", redis.Z{Score: 10, Member: "a"}, redis.Z{Score: 20, Member: "b"}, redis.Z{Score: 30, Member: "c"})
+		require.Equal(t, int64(3), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 10, Member: "a"}, {Score: 20, Member: "b"}}, rdb.ZPopMin(ctx, "ztmp", 2).Val())
+		require.Equal(t, int64(1), rdb.ZCard(ctx, "ztmp").Val())

Review Comment:
   Fixed.



-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1561626087

   > Issue number: #1458
   
   Please use `close` or `fixes`. The related site is https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
   
   


-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204177837


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   To my understanding, these arguments indicate keys position in the command? So maybe `2, -1, 1` is better?



-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204191087


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   Not precise. You should consider where the last position is. `-1` means the last one and it could be a `count`.
   
   For example, the `ZMPOP 2 myzset myzset2 MIN COUNT 10` command should return the range {2,3,1} because we want to watch `myzset`, `myzset2` in redis if possible. You can see  https://github.com/apache/incubator-kvrocks/pull/1444/files#diff-16b6c7fdec7b9f742f389c178e5956068721e86e5683e2d52a88855ad3efeb21R334 to view the `Range` function for syntax `SINTERCARD numkeys key [key ...] [LIMIT limit]`.



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204275743


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,77 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;
+
+    while (parser.Good()) {
+      if (parser.EatEqICase("min")) {
+        min_ = true;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("max")) {
+        min_ = false;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("count")) {
+        count_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+      } else {
+        return parser.InvalidSyntax();
+      }
+    }
+    if (!has_min_flag) {
+      return parser.InvalidSyntax();
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(Server *svr, Connection *conn, std::string *output) override {
+    redis::ZSet zset_db(svr->storage, conn->GetNamespace());
+    for (auto &user_key : keys_) {
+      std::vector<MemberScore> member_scores;
+      auto s = zset_db.Pop(user_key, count_, min_, &member_scores);
+      if (!s.ok()) {
+        return {Status::RedisExecErr, s.ToString()};
+      }
+      if (member_scores.empty()) {
+        continue;
+      }
+
+      output->append(redis::MultiLen(2));
+      output->append(redis::BulkString(user_key));
+      output->append(redis::MultiLen(member_scores.size() * 2));
+      for (const auto &ms : member_scores) {
+        output->append(redis::BulkString(ms.member));
+        output->append(redis::BulkString(util::Float2String(ms.score)));
+      }
+      return Status::OK();
+    }
+    *output = redis::NilString();
+    return Status::OK();
+  }
+
+  static CommandKeyRange Range(const std::vector<std::string> &args) {
+    uint64_t num_key = *ParseInt<int>(args[1], 10);
+    if (args.size() == num_key + 5) {
+      return {2, -4, 1};
+    }

Review Comment:
   Fixed.



-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1559913606

   > do I need to add test cases for it?
   
   Yeah, it is definitely an efficient way to prove to us that your code can work well.
   
   > ZPOPMIN / MAX also don't have corresponding test cases.
   
   You are welcome to add them if you want.


-- 
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


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1563983527

   Thanks all, merging...


-- 
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


[GitHub] [incubator-kvrocks] git-hulk merged pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk merged PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468


-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204122305


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,80 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    if (auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}); !parse_int) {
+      return parse_int.ToStatus();
+    } else {
+      numkeys_ = *parse_int;
+    }

Review Comment:
   ```suggestion
       numkeys_ = GET_OR_RET(parser.TakeInt<int>({1, std::numeric_limits<int>::max()}));
   ```



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204328332


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,77 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;
+
+    while (parser.Good()) {
+      if (parser.EatEqICase("min")) {
+        min_ = true;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("max")) {
+        min_ = false;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("count")) {
+        count_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+      } else {
+        return parser.InvalidSyntax();
+      }
+    }
+    if (!has_min_flag) {
+      return parser.InvalidSyntax();
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(Server *svr, Connection *conn, std::string *output) override {
+    redis::ZSet zset_db(svr->storage, conn->GetNamespace());
+    for (auto &user_key : keys_) {
+      std::vector<MemberScore> member_scores;
+      auto s = zset_db.Pop(user_key, count_, min_, &member_scores);
+      if (!s.ok()) {
+        return {Status::RedisExecErr, s.ToString()};
+      }
+      if (member_scores.empty()) {
+        continue;
+      }
+
+      output->append(redis::MultiLen(2));
+      output->append(redis::BulkString(user_key));
+      output->append(redis::MultiLen(member_scores.size() * 2));
+      for (const auto &ms : member_scores) {
+        output->append(redis::BulkString(ms.member));
+        output->append(redis::BulkString(util::Float2String(ms.score)));
+      }
+      return Status::OK();
+    }
+    *output = redis::NilString();
+    return Status::OK();
+  }
+
+  static CommandKeyRange Range(const std::vector<std::string> &args) {
+    int num_key = *ParseInt<int>(args[1], 10);
+    if (static_cast<int>(args.size()) > num_key + 3) {
+      return {2, 1 + num_key, 1};
+    }
+    return {2, -2, 1};

Review Comment:
   Fixed.



-- 
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


[GitHub] [incubator-kvrocks] torwig commented on pull request #1468: Add the support of the ZMPOP command

Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1559807591

   @MizukiCry Thank you for your contribution. Unfortunately, there are some functions that don't have test coverage. However, we are thriving to test all the new features and changes so it would be great if you provide test cases.


-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204257049


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,77 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;
+
+    while (parser.Good()) {
+      if (parser.EatEqICase("min")) {
+        min_ = true;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("max")) {
+        min_ = false;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("count")) {
+        count_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+      } else {
+        return parser.InvalidSyntax();
+      }
+    }
+    if (!has_min_flag) {
+      return parser.InvalidSyntax();
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(Server *svr, Connection *conn, std::string *output) override {
+    redis::ZSet zset_db(svr->storage, conn->GetNamespace());
+    for (auto &user_key : keys_) {
+      std::vector<MemberScore> member_scores;
+      auto s = zset_db.Pop(user_key, count_, min_, &member_scores);
+      if (!s.ok()) {
+        return {Status::RedisExecErr, s.ToString()};
+      }
+      if (member_scores.empty()) {
+        continue;
+      }
+
+      output->append(redis::MultiLen(2));
+      output->append(redis::BulkString(user_key));
+      output->append(redis::MultiLen(member_scores.size() * 2));
+      for (const auto &ms : member_scores) {
+        output->append(redis::BulkString(ms.member));
+        output->append(redis::BulkString(util::Float2String(ms.score)));
+      }
+      return Status::OK();
+    }
+    *output = redis::NilString();
+    return Status::OK();
+  }
+
+  static CommandKeyRange Range(const std::vector<std::string> &args) {
+    uint64_t num_key = *ParseInt<int>(args[1], 10);
+    if (args.size() == num_key + 5) {
+      return {2, -4, 1};
+    }

Review Comment:
   ```suggestion
       if (args.size() > num_key + 3) {
         return {2, 1 + num_key , 1};
       }
   ```



-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204191087


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   Not precise. You should consider where the last position is. `-1` means the last one and it could be a `count`.
   
   For example, the `ZMPOP 2 myzset myzset2 MIN COUNT 10` command should return the range {2,3,1} because we want to watch `myzset`, `myzset2` in redis if possible.



-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204129171


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,80 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    if (auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}); !parse_int) {
+      return parse_int.ToStatus();
+    } else {
+      numkeys_ = *parse_int;
+    }
+    for (int i = 0; i < numkeys_; ++i) {
+      if (!parser.Good()) {
+        return parser.InvalidSyntax();
+      }
+      keys_.emplace_back(*parser.TakeStr());
+    }
+    bool has_min_flag = false;
+
+    while (parser.Good()) {
+      if (parser.EatEqICase("min")) {
+        min_ = true;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("max")) {
+        min_ = false;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("count")) {
+        auto parse_int = parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()});
+        if (!parse_int) {
+          return parse_int.ToStatus();
+        }
+        count_ = *parse_int;

Review Comment:
   ditto



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204217883


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   > Not precise. You should consider where the last position is. `-1` means the last one and it could be a `count`.
   > 
   > For example, the `ZMPOP 2 myzset myzset2 MIN COUNT 10` command should return the range {2,3,1} because we want to watch `myzset`, `myzset2` in redis if possible. You can see https://github.com/apache/incubator-kvrocks/pull/1444/files#diff-16b6c7fdec7b9f742f389c178e5956068721e86e5683e2d52a88855ad3efeb21R334 to view the `Range` function for syntax `SINTERCARD numkeys key [key ...] [LIMIT limit]`.
   
   So I need to implement an extra function to determine the range right?



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1562148400

   > > Issue number: #1458
   > 
   > Please use `close` or `fixes`. The related site is https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue
   
   Fixed.


-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204191087


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   Not precise. You should consider the last position.



-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204319615


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +269,77 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;
+
+    while (parser.Good()) {
+      if (parser.EatEqICase("min")) {
+        min_ = true;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("max")) {
+        min_ = false;
+        has_min_flag = true;
+      } else if (parser.EatEqICase("count")) {
+        count_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+      } else {
+        return parser.InvalidSyntax();
+      }
+    }
+    if (!has_min_flag) {
+      return parser.InvalidSyntax();
+    }
+    return Commander::Parse(args);
+  }
+
+  Status Execute(Server *svr, Connection *conn, std::string *output) override {
+    redis::ZSet zset_db(svr->storage, conn->GetNamespace());
+    for (auto &user_key : keys_) {
+      std::vector<MemberScore> member_scores;
+      auto s = zset_db.Pop(user_key, count_, min_, &member_scores);
+      if (!s.ok()) {
+        return {Status::RedisExecErr, s.ToString()};
+      }
+      if (member_scores.empty()) {
+        continue;
+      }
+
+      output->append(redis::MultiLen(2));
+      output->append(redis::BulkString(user_key));
+      output->append(redis::MultiLen(member_scores.size() * 2));
+      for (const auto &ms : member_scores) {
+        output->append(redis::BulkString(ms.member));
+        output->append(redis::BulkString(util::Float2String(ms.score)));
+      }
+      return Status::OK();
+    }
+    *output = redis::NilString();
+    return Status::OK();
+  }
+
+  static CommandKeyRange Range(const std::vector<std::string> &args) {
+    int num_key = *ParseInt<int>(args[1], 10);
+    if (static_cast<int>(args.size()) > num_key + 3) {
+      return {2, 1 + num_key, 1};
+    }
+    return {2, -2, 1};

Review Comment:
   ```suggestion
       return {2, 1 + num_key, 1};
   ```
   
   Sorry, I notice that the `if` is not necessary actually. :rofl:



-- 
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


[GitHub] [incubator-kvrocks] git-hulk commented on pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1563986695

   Thanks @MizukiCry again.


-- 
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


[GitHub] [incubator-kvrocks] xiaobiaozhao commented on pull request #1468: Add the support of the ZMPOP command

Posted by "xiaobiaozhao (via GitHub)" <gi...@apache.org>.
xiaobiaozhao commented on PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#issuecomment-1559863941

   great job!
   1. Please add some test case
   2. Please explain the difference with the redis official `ZMPOP` command
       1. syntax
       2. Complexity


-- 
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


[GitHub] [incubator-kvrocks] PragmaTwice commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204133957


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   ```suggestion
                           MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 2, 2, 1),
   ```
   
   You can refer to https://github.com/apache/incubator-kvrocks/pull/1444#discussion_r1201874256 for a more concrete solution.



-- 
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


[GitHub] [incubator-kvrocks] infdahai commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "infdahai (via GitHub)" <gi...@apache.org>.
infdahai commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204191087


##########
src/commands/cmd_zset.cc:
##########
@@ -745,6 +822,7 @@ REDIS_REGISTER_COMMANDS(MakeCmdAttr<CommandZAdd>("zadd", -4, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZLexCount>("zlexcount", 4, "read-only", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMax>("zpopmax", -2, "write", 1, 1, 1),
                         MakeCmdAttr<CommandZPopMin>("zpopmin", -2, "write", 1, 1, 1),
+                        MakeCmdAttr<CommandZMPop>("zmpop", -4, "write", 1, 1, 1),

Review Comment:
   Not precise. You should consider where the last position is. `-1` means the last one and it could be a `count`.
   



-- 
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


[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1204372445


##########
tests/gocase/unit/type/zset/zset_test.go:
##########
@@ -235,6 +235,62 @@ func basicTests(t *testing.T, rdb *redis.Client, ctx context.Context, encoding s
 		require.Equal(t, int64(3), rdb.ZRem(ctx, "ztmp", []string{"a", "b", "c", "d", "e", "f", "g"}).Val())
 	})
 
+	t.Run(fmt.Sprintf("ZPOPMIN basics - %s", encoding), func(t *testing.T) {
+		rdb.Del(ctx, "ztmp")
+		rdb.ZAdd(ctx, "ztmp", redis.Z{Score: 10, Member: "a"}, redis.Z{Score: 20, Member: "b"}, redis.Z{Score: 30, Member: "c"})
+		require.Equal(t, int64(3), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 10, Member: "a"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(2), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 20, Member: "b"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(1), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 30, Member: "c"}}, rdb.ZPopMin(ctx, "ztmp").Val())
+		require.Equal(t, int64(0), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{}, rdb.ZPopMin(ctx, "ztmp").Val())
+		rdb.ZAdd(ctx, "ztmp", redis.Z{Score: 10, Member: "a"}, redis.Z{Score: 20, Member: "b"}, redis.Z{Score: 30, Member: "c"})
+		require.Equal(t, int64(3), rdb.ZCard(ctx, "ztmp").Val())
+		require.Equal(t, []redis.Z{{Score: 10, Member: "a"}, {Score: 20, Member: "b"}}, rdb.ZPopMin(ctx, "ztmp", 2).Val())
+		require.Equal(t, int64(1), rdb.ZCard(ctx, "ztmp").Val())

Review Comment:
   You can use `require.EqualValues` (a handy thing) here and in other similar places to get rid of explicit conversions like `int64(1)`.



-- 
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


[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1206252921


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +268,74 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;

Review Comment:
   It's a bit confusing, when the MAX was passed then we set the has_min_flag to true. I think we can an enum to represent this state:
   
   ```C++
   where = ZSET_NONE
   
   if (parser.EatEqICase("min")) {
     where = ZSET_MIN
   } else if (parser.EatEqICase("max")) {
     where = ZSET_MAX
   }
   if (where == ZSET_NONE) {
    // invalid syntax
   }
   ```



-- 
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


[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "git-hulk (via GitHub)" <gi...@apache.org>.
git-hulk commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1206252921


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +268,74 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;

Review Comment:
   It's a bit confusing, when the MAX was passed then we set the has_min_flag to true. I think we can an enum to represent this state:
   
   where = ZSET_NONE
   
   if (parser.EatEqICase("min")) {
     where = ZSET_MIN
   } else if (parser.EatEqICase("max")) {
     where = ZSET_MAX
   }
   if (where == ZSET_NONE) {
    // invalid syntax
   }



-- 
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


[GitHub] [incubator-kvrocks] MizukiCry commented on a diff in pull request #1468: Add the support of the ZMPOP command

Posted by "MizukiCry (via GitHub)" <gi...@apache.org>.
MizukiCry commented on code in PR #1468:
URL: https://github.com/apache/incubator-kvrocks/pull/1468#discussion_r1206307216


##########
src/commands/cmd_zset.cc:
##########
@@ -266,6 +268,74 @@ class CommandZPopMax : public CommandZPop {
   CommandZPopMax() : CommandZPop(false) {}
 };
 
+class CommandZMPop : public Commander {
+ public:
+  CommandZMPop() = default;
+
+  Status Parse(const std::vector<std::string> &args) override {
+    CommandParser parser(args, 1);
+    numkeys_ = GET_OR_RET(parser.TakeInt<int>(NumericRange<int>{1, std::numeric_limits<int>::max()}));
+    for (int i = 0; i < numkeys_; ++i) {
+      keys_.emplace_back(GET_OR_RET(parser.TakeStr()));
+    }
+    bool has_min_flag = false;

Review Comment:
   Fixed.



-- 
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