Skip to content

feat(tdigest): add TDIGEST.MERGE command implementation #3054

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 17, 2025

Conversation

LindaSummer
Copy link
Member

Issue

#2805

Proposed Changes

  • Add TDIGEST.MERGE command implementation
  • Add go unit tests

@git-hulk git-hulk requested a review from Copilot July 15, 2025 14:41
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Implements the TDIGEST.MERGE command end-to-end and adds corresponding tests.

  • Adds TDigestMerge overloads in the core and hooks them into the Redis command layer.
  • Defines parsing, validation, and execution logic for TDIGEST.MERGE in the server.
  • Introduces Go unit tests to cover argument errors and successful merge scenarios.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/gocase/unit/type/tdigest/tdigest_test.go New Go tests for TDIGEST.MERGE argument handling and merge logic
src/types/tdigest.h Declares new TDigestMerge overloads with delta parameter
src/types/tdigest.cc Implements TDigestMerge functions and removes unused include
src/types/redis_tdigest.h Adds Merge method and TDigestMergeOptions struct
src/types/redis_tdigest.cc Implements the TDigest::Merge method
src/commands/error_constants.h Adds error constants for parsing numkeys and wrong keywords
src/commands/cmd_tdigest.cc Adds CommandTDigestMerge parser/executor and registers the command

@git-hulk git-hulk requested review from PragmaTwice and mapleFU July 15, 2025 15:05
@mapleFU
Copy link
Member

mapleFU commented Jul 17, 2025

( I'm currently focus on other things, I might have time on Friday or later, you can merge this firstly. I would post-review this weekend)

Copy link

@PragmaTwice PragmaTwice merged commit 2482dc7 into apache:unstable Jul 17, 2025
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants