You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/09/19 14:44:12 UTC
[GitHub] [incubator-kvrocks] tanruixiang opened a new pull request, #895: Support hrange command
tanruixiang opened a new pull request, #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895
It closes #894
--
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] caipengbo commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975509013
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
Maybe the syntax is such that the user can enter a negative number, but it will return an empty array.
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975509176
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
>
hash can store at most uint32_t entry, So it is ok.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#issuecomment-1252585500
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975294619
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
+ for (const auto &fv : field_values) {
+ *output += Redis::BulkString(fv.field);
+ *output += Redis::BulkString(fv.value);
+ }
+ return Status::OK();
Review Comment:
> I personally don't think it's very elegant to assemble RESP here. Maybe we could wrap it as a function that can passes `FieldValue`, or we could assemble it as `vector<string>` and pass it to our existing `Redis::MultiBulkString`
Yes. There are several places in the code where this direct assembly method is used. I can add a `Redis::BulkFieldValue` to replace them.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974353863
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
+ stop_ = std::stol(args[3]);
+ if (args.size() == 6) {
+ limit_ = std::stol(args[5]);
Review Comment:
Can use `ParseInt` here
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975513506
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
> > Maybe the syntax is such that the user can enter a negative number, but it will return an empty array.
>
> Yes, When user input negative number, It will return empty array.
You can check this case in hash_test.go.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974360345
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
Review Comment:
👍 The issue also didn't clarify those parameters clearly.
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974356954
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
+ stop_ = std::stol(args[3]);
+ if (args.size() == 6) {
+ limit_ = std::stol(args[5]);
Review Comment:
> Can use `ParseInt` here
I noticed that std::stol is used in quite a few places in the kvrocks. Should I open another issue to replace these std::stol?
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#issuecomment-1252405460
Thanks for @tanruixiang contribution, his PR looks good to me. Do you mind adding the `gocase` for the hrange command as well first, then we can migrate the other test cases later.
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975294619
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
+ for (const auto &fv : field_values) {
+ *output += Redis::BulkString(fv.field);
+ *output += Redis::BulkString(fv.value);
+ }
+ return Status::OK();
Review Comment:
> I personally don't think it's very elegant to assemble RESP here. Maybe we could wrap it as a function that can passes `FieldValue`, or we could assemble it as `vector<string>` and pass it to our existing `Redis::MultiBulkString`
Yes. There are several places in the code where this direct assembly method is used. I can assemble them into a vector<string> and send it to MultiBulkString to solve it.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975517456
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
got it. although I prefer more constrained and strongly typed behaviors.
--
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] tanruixiang commented on pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#issuecomment-1252409344
>
Yes. Originally, I wanted to complete the migration of `hash.tcl` by the way. But I see that issue seems to be assigned.Can I add to gocase after he is done migrating?
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#issuecomment-1252472805
> > Thanks for @tanruixiang contribution, this PR looks good to me. Do you mind adding the `gocase` for the hrange command as well first, then we can migrate the other test cases later.
>
> Yes. Originally, I wanted to complete the migration of `hash.tcl` by the way. But I see that issue seems to be assigned. ~Can I add to gocase after he is done migrating?~ I think I can write gocase for hrange first.
Sorry that I missed this comment, it's ok 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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975512194
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
> Maybe the syntax is such that the user can enter a negative number, but it will return an empty array.
Yes, When user input negative number, It will return empty array.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk merged PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975506226
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
I think maybe `limit_` should be an unsigned integer?
You can use `ParseInt<unsigned ...>` which will return error when parsing a negative number.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974358969
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
+ stop_ = std::stol(args[3]);
+ if (args.size() == 6) {
+ limit_ = std::stol(args[5]);
Review Comment:
Yes, I think we can do it in another PR to make the context clear.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
git-hulk commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974352758
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
Review Comment:
@tanruixiang The start/stop should be the string range instead of the index.
--
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] caipengbo commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974826268
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
+ for (const auto &fv : field_values) {
+ *output += Redis::BulkString(fv.field);
+ *output += Redis::BulkString(fv.value);
+ }
+ return Status::OK();
Review Comment:
Can we use `Redis::MultiBulkString`?
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
Review Comment:
This part of the logic, I think we should put it into the `Hash::Range`
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974889580
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
+ for (const auto &fv : field_values) {
+ *output += Redis::BulkString(fv.field);
+ *output += Redis::BulkString(fv.value);
+ }
+ return Status::OK();
Review Comment:
> Can we use `Redis::MultiBulkString`?
Yes, But it need convert `std::vector<FieldValue> field_values` to `std::vector<string> field_values`. Is there an elegant iterator conversion method in c++?
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974888285
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
Review Comment:
> This part of the logic, I think we should put it into the `Hash::Range`
Thank you for your review. Cannot get `args_` in `Hash::Range`. It seems unreasonable to pass it as a parameter.
--
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] caipengbo commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
caipengbo commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974960526
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
+ for (const auto &fv : field_values) {
+ *output += Redis::BulkString(fv.field);
+ *output += Redis::BulkString(fv.value);
+ }
+ return Status::OK();
Review Comment:
I personally don't think it's very elegant to assemble RESP here. Maybe we could wrap it as a function that can passes `FieldValue`, or we could assemble it as `vector<string>` and pass it to our existing `Redis::MultiBulkString`.
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
Review Comment:
I mean use `limit_` instead of `args_`.
If you don't want that, maybe it's better to return `Redis::NilString()` here.
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r974354398
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,48 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ try {
+ start_ = std::stol(args[2]);
Review Comment:
> @tanruixiang The start/stop should be the string range instead of the index.
Ok, I misunderstood the meaning of start and stop. I'll revise it right away.
--
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] tanruixiang commented on a diff in pull request #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
tanruixiang commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975280331
##########
src/redis_cmd.cc:
##########
@@ -1453,6 +1453,45 @@ class CommandHGetAll : public Commander {
}
};
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
+ }
+ if (args.size() == 6) {
+ auto parseResult = ParseInt<int>(args_[5], /* base= */ 10);
+ if (!parseResult.IsOK())return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = parseResult.GetValue();
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ if (args_.size() == 6 && limit_ < 0) {
+ *output = "*" + std::to_string(0) + CRLF;
+ return Status::OK();
+ }
Review Comment:
> I mean use `limit_` instead of `args_`. If you don't want that, maybe it's better to return `Redis::NilString()` here.
OK. I need to modify the current logic, after the modification, it should be able to meet the requirements you said.
--
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 #895: Support hrange command
Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on code in PR #895:
URL: https://github.com/apache/incubator-kvrocks/pull/895#discussion_r975506226
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
I think maybe `limit_` should be a unsigned integer?
##########
src/redis_cmd.cc:
##########
@@ -1444,13 +1447,50 @@ class CommandHGetAll : public Commander {
if (!s.ok()) {
return Status(Status::RedisExecErr, s.ToString());
}
- *output = "*" + std::to_string(field_values.size() * 2) + CRLF;
- for (const auto &fv : field_values) {
- *output += Redis::BulkString(fv.field);
- *output += Redis::BulkString(fv.value);
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
+ return Status::OK();
+ }
+};
+
+class CommandHRange : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 6 && args.size() != 4) {
+ return Status(Status::RedisParseErr, errWrongNumOfArguments);
+ }
+ if (args.size() == 6 && Util::ToLower(args[4]) != "limit") {
+ return Status(Status::RedisInvalidCmd, errInvalidSyntax);
}
+ if (args.size() == 6) {
+ auto parse_result = ParseInt<int64_t>(args_[5], 10);
+ if (!parse_result)return Status(Status::RedisParseErr, errValueNotInterger);
+ limit_ = *parse_result;
+ }
+ return Commander::Parse(args);
+ }
+ Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ Redis::Hash hash_db(svr->storage_, conn->GetNamespace());
+ std::vector<FieldValue> field_values;
+ rocksdb::Status s = hash_db.Range(args_[1], args_[2], args_[3], limit_, &field_values);
+ if (!s.ok()) {
+ return Status(Status::RedisExecErr, s.ToString());
+ }
+ std::vector<std::string> kv_pairs;
+ for (const auto &p : field_values) {
+ kv_pairs.emplace_back(p.field);
+ kv_pairs.emplace_back(p.value);
+ }
+ *output = MultiBulkString(kv_pairs);
return Status::OK();
}
+
+ private:
+ int64_t limit_ = LONG_MAX;
Review Comment:
I think maybe `limit_` should be an unsigned integer?
--
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