You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by "torwig (via GitHub)" <gi...@apache.org> on 2023/03/09 16:13:48 UTC
[GitHub] [incubator-kvrocks] torwig opened a new pull request, #1310: Allow HSETNX to receive multiple field-value pairs like HSET
torwig opened a new pull request, #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310
Close #1294
--
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 #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310#discussion_r1131318981
##########
src/commands/cmd_hash.cc:
##########
@@ -44,10 +44,22 @@ class CommandHGet : public Commander {
class CommandHSetNX : public Commander {
public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() % 2 != 0) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+ return Commander::Parse(args);
+ }
+
Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ std::vector<FieldValue> field_values;
+ for (size_t i = 2; i < args_.size(); i += 2) {
+ field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
Review Comment:
BTW I think it is more proper to put the for loop to `Parse`.
--
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 #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310#issuecomment-1463174373
Thanks @torwig, 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] PragmaTwice commented on a diff in pull request #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310#discussion_r1131317162
##########
src/commands/cmd_hash.cc:
##########
@@ -44,10 +44,22 @@ class CommandHGet : public Commander {
class CommandHSetNX : public Commander {
public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() % 2 != 0) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+ return Commander::Parse(args);
+ }
+
Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ std::vector<FieldValue> field_values;
+ for (size_t i = 2; i < args_.size(); i += 2) {
+ field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
Review Comment:
```suggestion
field_values.emplace_back(args_[i], args_[i + 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] torwig commented on a diff in pull request #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "torwig (via GitHub)" <gi...@apache.org>.
torwig commented on code in PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310#discussion_r1131355040
##########
src/commands/cmd_hash.cc:
##########
@@ -44,10 +44,22 @@ class CommandHGet : public Commander {
class CommandHSetNX : public Commander {
public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() % 2 != 0) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+ return Commander::Parse(args);
+ }
+
Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ std::vector<FieldValue> field_values;
+ for (size_t i = 2; i < args_.size(); i += 2) {
+ field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
Review Comment:
@PragmaTwice Thank you for your reasonable suggestions.
##########
src/commands/cmd_hash.cc:
##########
@@ -44,10 +44,22 @@ class CommandHGet : public Commander {
class CommandHSetNX : public Commander {
public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() % 2 != 0) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+ return Commander::Parse(args);
+ }
+
Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ std::vector<FieldValue> field_values;
+ for (size_t i = 2; i < args_.size(); i += 2) {
+ field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
Review Comment:
Done
--
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 merged pull request #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice merged PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310
--
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 #1310: Allow HSETNX to receive multiple field-value pairs like HSET
Posted by "PragmaTwice (via GitHub)" <gi...@apache.org>.
PragmaTwice commented on code in PR #1310:
URL: https://github.com/apache/incubator-kvrocks/pull/1310#discussion_r1131318981
##########
src/commands/cmd_hash.cc:
##########
@@ -44,10 +44,22 @@ class CommandHGet : public Commander {
class CommandHSetNX : public Commander {
public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() % 2 != 0) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+ return Commander::Parse(args);
+ }
+
Status Execute(Server *svr, Connection *conn, std::string *output) override {
+ std::vector<FieldValue> field_values;
+ for (size_t i = 2; i < args_.size(); i += 2) {
+ field_values.emplace_back(FieldValue{args_[i], args_[i + 1]});
Review Comment:
BTW I think it is more proper to put the for loop into `Parse`.
--
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