This is an automated email from the ASF dual-hosted git repository.
edwardxu pushed a commit to branch unstable
in repository https://gitbox.apache.org/repos/asf/kvrocks.git
The following commit(s) were added to refs/heads/unstable by this push:
new 27fbff104 fix(tdigest): correct TDIGEST.MERGE parser for COMPRESSION
parameter. (#3449)
27fbff104 is described below
commit 27fbff104fe918ff8733bbf6d9ac8810facc0552
Author: Reilly.tang <[email protected]>
AuthorDate: Wed Apr 15 14:25:16 2026 +0800
fix(tdigest): correct TDIGEST.MERGE parser for COMPRESSION parameter.
(#3449)
Fix #3447
The parser incorrectly used two independent `if` statements to check
COMPRESSION and OVERRIDE parameters. When COMPRESSION was successfully
parsed, the code continued to the second `if` which would fail since the
parser had reached the end, causing "ERR wrong keyword" error.
Fixed by changing the second `if` to `else if` to form a proper
if-else-if-else chain.
Signed-off-by: reilly <[email protected]>
Co-authored-by: 纪华裕 <[email protected]>
---
src/commands/cmd_tdigest.cc | 4 +--
tests/gocase/unit/type/tdigest/tdigest_test.go | 47 ++++++++++++++++++++++++++
2 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/src/commands/cmd_tdigest.cc b/src/commands/cmd_tdigest.cc
index ad144bf37..c0baf8ca1 100644
--- a/src/commands/cmd_tdigest.cc
+++ b/src/commands/cmd_tdigest.cc
@@ -466,9 +466,7 @@ class CommandTDigestMerge : public Commander {
} else {
options_.compression = *compression;
}
- }
-
- if (parser.EatEqICase(kOverrideArg)) {
+ } else if (parser.EatEqICase(kOverrideArg)) {
if (options_.override_flag) { // override already set
return {Status::RedisParseErr, errWrongNumOfArguments};
}
diff --git a/tests/gocase/unit/type/tdigest/tdigest_test.go
b/tests/gocase/unit/type/tdigest/tdigest_test.go
index f61eb9ffc..b7b4893a7 100644
--- a/tests/gocase/unit/type/tdigest/tdigest_test.go
+++ b/tests/gocase/unit/type/tdigest/tdigest_test.go
@@ -642,6 +642,53 @@ func tdigestTests(t *testing.T, configs
util.KvrocksServerConfigs) {
validation(newDestKey2)
})
+ // https://github.com/apache/kvrocks/issues/3447
+ t.Run("tdigest.merge with COMPRESSION parameter (issue #3447)", func(t
*testing.T) {
+ keyPrefix := "tdigest_merge_compression_"
+
+ srcKey := keyPrefix + "src"
+ destKey := keyPrefix + "dest"
+
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", srcKey,
"compression", "100").Err())
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", srcKey, "1", "2",
"3", "4", "5").Err())
+
+ // This should succeed, not return "ERR wrong keyword"
+ // Issue #3447: TDIGEST.MERGE dest 1 src COMPRESSION 100 fails
with "ERR wrong keyword"
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.MERGE", destKey, 1,
srcKey, "COMPRESSION", "100").Err())
+
+ // Verify the merge worked
+ rsp := rdb.Do(ctx, "TDIGEST.INFO", destKey)
+ require.NoError(t, rsp.Err())
+ info := toTdigestInfo(t, rsp.Val())
+ require.EqualValues(t, 100, info.Compression)
+ require.EqualValues(t, 5, info.Observations)
+ })
+
+ // Test COMPRESSION + OVERRIDE combination
+ t.Run("tdigest.merge with COMPRESSION and OVERRIDE together", func(t
*testing.T) {
+ keyPrefix := "tdigest_merge_compression_override_"
+
+ srcKey := keyPrefix + "src"
+ destKey := keyPrefix + "dest"
+
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", srcKey,
"compression", "100").Err())
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", srcKey, "1", "2",
"3", "4", "5").Err())
+
+ // Create destination key first to test OVERRIDE
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.CREATE", destKey,
"compression", "50").Err())
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.ADD", destKey, "10",
"20").Err())
+
+ // COMPRESSION + OVERRIDE should work together
+ require.NoError(t, rdb.Do(ctx, "TDIGEST.MERGE", destKey, 1,
srcKey, "COMPRESSION", "101", "OVERRIDE").Err())
+
+ // Verify the merge worked with new compression
+ rsp := rdb.Do(ctx, "TDIGEST.INFO", destKey)
+ require.NoError(t, rsp.Err())
+ info := toTdigestInfo(t, rsp.Val())
+ require.EqualValues(t, 101, info.Compression)
+ require.EqualValues(t, 5, info.Observations) // src data
replaced dest data due to OVERRIDE
+ })
+
t.Run("tdigest.revrank with different arguments", func(t *testing.T) {
keyPrefix := "tdigest_revrank_"
