chakkk309 commented on code in PR #3312:
URL: https://github.com/apache/kvrocks/pull/3312#discussion_r2925450042
##########
src/commands/cmd_tdigest.cc:
##########
@@ -492,6 +492,67 @@ class CommandTDigestMerge : public Commander {
TDigestMergeOptions options_;
};
+class CommandTDigestTrimmedMean : public Commander {
+ public:
+ Status Parse(const std::vector<std::string> &args) override {
+ if (args.size() != 4) {
+ return {Status::RedisParseErr, errWrongNumOfArguments};
+ }
+
+ key_name_ = args[1];
+
+ auto low_cut_quantile = ParseFloat(args[2]);
+ if (!low_cut_quantile) {
+ return {Status::RedisParseErr, errValueIsNotFloat};
+ }
+ low_cut_quantile_ = *low_cut_quantile;
+
+ auto high_cut_quantile = ParseFloat(args[3]);
+ if (!high_cut_quantile) {
+ return {Status::RedisParseErr, errValueIsNotFloat};
+ }
+ high_cut_quantile_ = *high_cut_quantile;
+
+ if (!std::isfinite(low_cut_quantile_) || low_cut_quantile_ < 0.0 ||
low_cut_quantile_ > 1.0) {
Review Comment:
I tested Redis's actual behavior and found that it accepts numeric forms
like `.5` and `+0.5` for this command. If we add a strict string-level regex
here, would that make it more restrictive than Redis?
To preserve Redis compatibility, I kept numeric parsing and aligned the
validation with Redis. Could you please let me know if this approach would be
acceptable? 👀
--
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]
