fix(tdigest): merge into existing dest without OVERRIDE

[Tangruilin]

WIP: fix TDIGEST.MERGE to match Redis behavior

Issue: #3410

When destination key already exists without OVERRIDE flag:

• Redis: merge dest + sources together
• Kvrocks (before): return error

Changes:

1. Remove error return when dest exists without OVERRIDE
2. Read dest centroids and add to merge list
3. Add check to prevent duplicate merge if dest in sources
4. Update tests to verify merge and override behavior

[Tangruilin]

@jihuayu Hi, this PR is ready for review. Thanks!

[Copilot]

Pull request overview

Aligns Kvrocks’ TDIGEST.MERGE semantics with Redis when the destination key already exists and OVERRIDE is not specified, by merging destination + sources instead of returning an error.

Changes:

• Update TDigest::Merge to include the destination digest’s centroids in the merge set when dest exists without OVERRIDE.
• Refactor centroid extraction logic into a shared helper (getCentroidsForMerge) to handle buffered/unbuffered digests consistently.
• Extend Go unit tests to validate merging into an existing destination and OVERRIDE overwrite behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/gocase/unit/type/tdigest/tdigest_test.go	Updates merge tests to assert existing-dest merge semantics and override overwrite semantics.
src/types/redis_tdigest.h	Declares a new helper for extracting centroids for merge operations.
src/types/redis_tdigest.cc	Implements the new merge behavior and the centroid-extraction helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Tangruilin]

@jihuayu "I'm sorry the CI didn't pass.

However, it seems the CI format configuration is not using the default configuration. I ran ./x.py check format locally and it passed, but the CI command is ./x.py check format
--clang-format-path clang-format-18.

This can be confusing. Below is a screenshot of my local run. Perhaps we should keep the CI consistent with the default configuration and use configuration files instead of command-line
arguments."

[Tangruilin]

@jihuayu
"The lint issue was introduced by MR #3419 It seems this MR was merged without passing the format check.

Should kvrocks consider enforcing a rule that blocks merging when CI doesn't pass, to avoid similar CI pollution issues?"

[Tangruilin]

@jihuayu "The lint issue was introduced by MR #3419 It seems this MR was merged without passing the format check.

Should kvrocks consider enforcing a rule that blocks merging when CI doesn't pass, to avoid similar CI pollution issues?"

[jihuayu]

@Tangruilin Yes, you are right!

[LindaSummer]

Hi @Tangruilin ,

Generally LGTM with some comments. 😊

Could we add some cpp unit tests for this change?

[Tangruilin]

Hi @Tangruilin ,

Generally LGTM with some comments. 😊

Could we add some cpp unit tests for this change?

Sure. I will add cpp unit before merge

[jihuayu]

@Tangruilin It looks like the code incorrectly overrides the COMPRESSION parameter in the following scenario. Could you add more test cases for this?

TDIGEST.CREATE src COMPRESSION 200
TDIGEST.ADD src 1

TDIGEST.CREATE dest COMPRESSION 100
TDIGEST.ADD dest 2

TDIGEST.INFO dest
TDIGEST.MERGE dest 1 src
TDIGEST.INFO dest

The Redis results are as follows:

127.0.0.1:6379> TDIGEST.INFO dest
 1) Compression
 2) (integer) 100

[Tangruilin]

@Tangruilin It looks like the code incorrectly overrides the COMPRESSION parameter in the following scenario. Could you add more test cases for this?

TDIGEST.CREATE src COMPRESSION 200
TDIGEST.ADD src 1

TDIGEST.CREATE dest COMPRESSION 100
TDIGEST.ADD dest 2

TDIGEST.INFO dest
TDIGEST.MERGE dest 1 src
TDIGEST.INFO dest

The Redis results are as follows:

127.0.0.1:6379> TDIGEST.INFO dest
 1) Compression
 2) (integer) 100

Done

[Tangruilin]

@jihuayu ji
The CI seems to fail because of env

[jihuayu]

It looks OK.
However, the logic in this function is a bit complex, so I’ll need some help from other reviewers.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
66.9% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[LindaSummer]

Hi @Tangruilin ,

LGTM 🚀

With the compression parameter improvement, its behavior should match the Redis' now.

Thanks very much for your effort!