LindaSummer commented on code in PR #2849:
URL: https://github.com/apache/kvrocks/pull/2849#discussion_r2072346057
##########
tests/gocase/unit/type/tdigest/tdigest_test.go:
##########
@@ -310,4 +311,66 @@ func tdigestTests(t *testing.T, configs
util.KvrocksServerConfigs) {
infoAfterEmptyReset := toTdigestInfo(t, rsp.Val())
require.EqualValues(t, 100, infoAfterEmptyReset.Compression)
})
+ t.Run("tdigest.quantile with different arguments", func(t *testing.T) {
Review Comment:
Hi @SharonIV0x86 ,
We could also add an unordered sequence and an empty tdigest as cases. 😊
Best Regards,
Edward
##########
src/commands/cmd_tdigest.cc:
##########
@@ -242,11 +242,50 @@ class CommandTDigestMax : public CommandTDigestMinMax {
public:
CommandTDigestMax() : CommandTDigestMinMax(false) {}
};
+class CommandTDigestQuantile : public Commander {
+ Status Parse(const std::vector<std::string> &args) override {
+ key_name_ = args[1];
+ values_.reserve(args.size() - 2);
+ for (size_t i = 2; i < args.size(); i++) {
+ auto value = ParseFloat(args[i]);
+ if (!value) {
+ return {Status::RedisParseErr, errValueIsNotFloat};
+ }
+ values_.push_back(*value);
+ }
+ return Status::OK();
+ }
+ Status Execute(engine::Context &ctx, Server *srv, Connection *conn,
std::string *output) override {
+ TDigest tdigest(srv->storage, conn->GetNamespace());
+ TDigestQuantitleResult result;
+ auto s = tdigest.Quantile(ctx, key_name_, values_, &result);
+ if (!s.ok()) {
+ if (s.IsNotFound()) {
+ return {Status::RedisExecErr, errKeyNotFound};
+ }
+ return {Status::RedisExecErr, s.ToString()};
+ }
+ if (values_.empty()) {
+ return {Status::RedisExecErr, "invalid quantile or empty tdigest"};
Review Comment:
Hi @SharonIV0x86 ,
Sorry for my unclear description for this behavior. 😄
Currently, redis stack returns nan as response for none centroids.
We'd better follow this behavior.
Adding the logic to tdigest ranther than command may be better to keep the
command module clean.
After modifying the tdigest, the cpp unit test should also be updated.
Best Regards,
Edward
--
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]
