Copilot commented on code in PR #3054:
URL: https://github.com/apache/kvrocks/pull/3054#discussion_r2207714238
##########
src/commands/cmd_tdigest.cc:
##########
@@ -281,11 +284,103 @@ class CommandTDigestQuantile : public Commander {
std::string key_name_;
std::vector<double> quantiles_;
};
+
+class CommandTDigestMerge : public Commander {
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 1);
+ dest_key_ = GET_OR_RET(parser.TakeStr());
+ auto numkeys = parser.TakeInt();
+ if (!numkeys) {
+ return {Status::RedisParseErr, errParsingNumkeys};
+ }
+
+ if (*numkeys <= 0) {
+ return {Status::RedisParseErr, errCompressionMustBePositive};
Review Comment:
This error constant refers to 'compression' but is used for a numkeys check;
it would be clearer to return errNumkeysMustBePositive when numkeys is
non-positive.
```suggestion
return {Status::RedisParseErr, errNumkeysMustBePositive};
```
##########
src/commands/cmd_tdigest.cc:
##########
@@ -281,11 +284,103 @@ class CommandTDigestQuantile : public Commander {
std::string key_name_;
std::vector<double> quantiles_;
};
+
+class CommandTDigestMerge : public Commander {
+ Status Parse(const std::vector<std::string> &args) override {
+ CommandParser parser(args, 1);
+ dest_key_ = GET_OR_RET(parser.TakeStr());
+ auto numkeys = parser.TakeInt();
+ if (!numkeys) {
+ return {Status::RedisParseErr, errParsingNumkeys};
+ }
+
+ if (*numkeys <= 0) {
+ return {Status::RedisParseErr, errCompressionMustBePositive};
+ }
+
+ if (static_cast<int64_t>(args.size()) < (3 + *numkeys)) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+
+ std::set<std::string> unique_source_keys;
+
+ for (auto i = 3; i < (3 + *numkeys); i++) {
+ auto src_digest = GET_OR_RET(parser.TakeStr());
+ unique_source_keys.emplace(std::move(src_digest));
+ }
+ source_keys_ = ranges::to_vector(unique_source_keys);
Review Comment:
Using a std::set here removes duplicates and sorts the source keys; to
support merging the same key multiple times and preserve order, consider using
a vector and pushing each key directly.
```suggestion
std::vector<std::string> source_keys;
for (auto i = 3; i < (3 + *numkeys); i++) {
auto src_digest = GET_OR_RET(parser.TakeStr());
source_keys.push_back(std::move(src_digest));
}
source_keys_ = std::move(source_keys);
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
