[GitHub] [incubator-kvrocks] tanruixiang commented on a diff in pull request #1119: Support more `ZRANGE` options

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

tanruixiang commented on code in PR #1119:
URL: https://github.com/apache/incubator-kvrocks/pull/1119#discussion_r1021041473


##########
src/commands/redis_cmd.cc:
##########
@@ -2554,44 +2555,117 @@ class CommandZRange : public Commander {
   explicit CommandZRange(bool reversed = false) : reversed_(reversed) {}
 
   Status Parse(const std::vector<std::string> &args) override {
-    auto parse_start = ParseInt<int>(args[2], 10);
-    auto parse_stop = ParseInt<int>(args[3], 10);
-    if (!parse_start || !parse_stop) {
-      return Status(Status::RedisParseErr, errValueNotInteger);
+    CommandParser parser(args, 4);
+    while (parser.Good()) {
+      if (parser.EatEqICaseFlag("BYSCORE", by_flag_)) {
+      } else if (parser.EatEqICaseFlag("BYLEX", by_flag_)) {
+      } else if (parser.EatEqICase("REV")) {
+        reversed_ = true;
+      } else if (parser.EatEqICase("LIMIT")) {
+        offset_ = GET_OR_RET(parser.TakeInt());
+        count_ = GET_OR_RET(parser.TakeInt());
+      } else if (parser.EatEqICase("withscores")) {
+        with_scores_ = true;
+      } else {
+        return parser.InvalidSyntax();
+      }
     }
-    start_ = *parse_start;
-    stop_ = *parse_stop;
-    if (args.size() > 4 && (Util::ToLower(args[4]) == "withscores")) {
-      with_scores_ = true;
+    Status s;
+    if (by_flag_ == "BYSCORE") {
+      spec_.count = count_;
+      spec_.offset = offset_;
+      spec_.reversed = reversed_;
+      if (spec_.reversed) {
+        s = Redis::ZSet::ParseRangeSpec(args[3], args[2], &spec_);
+      } else {
+        s = Redis::ZSet::ParseRangeSpec(args[2], args[3], &spec_);
+      }
+      if (!s.IsOK()) {
+        return Status(Status::RedisParseErr, s.Msg());
+      }
+    } else if (by_flag_ == "BYLEX") {
+      specLex_.count = count_;
+      specLex_.offset = offset_;
+      specLex_.reversed = reversed_;
+      if (specLex_.reversed) {
+        s = Redis::ZSet::ParseRangeLexSpec(args[3], args[2], &specLex_);
+      } else {
+        s = Redis::ZSet::ParseRangeLexSpec(args[2], args[3], &specLex_);
+      }
+      if (!s.IsOK()) {
+        return Status(Status::RedisParseErr, s.Msg());
+      }
+    } else {
+      by_flag_ = "BYINDEX";
+      auto parse_start = ParseInt<int>(args[2], 10);
+      auto parse_stop = ParseInt<int>(args[3], 10);
+      if (!parse_start || !parse_stop) {
+        return Status(Status::RedisParseErr, errValueNotInteger);
+      }
+      start_ = *parse_start;
+      stop_ = *parse_stop;

Review Comment:
   Here it is straightforward to use `reversed_`. It is handled in the following `execute`.
   ```
   uint8_t flags = !reversed_ ? 0 : kZSetReversed;
   ```



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