Skip to content

feat(core): introduce pluggable RecordMerger trait for file-group merging#612

Open
xushiyan wants to merge 6 commits into
apache:mainfrom
xushiyan:record-merger-abstraction
Open

feat(core): introduce pluggable RecordMerger trait for file-group merging#612
xushiyan wants to merge 6 commits into
apache:mainfrom
xushiyan:record-merger-abstraction

Conversation

@xushiyan

Copy link
Copy Markdown
Member

Closes #611

Summary:

  • Introduce a RecordMerger trait and rename the existing concrete merger to RecordBatchMerger.
  • Add hoodie.read.record.merger.impl, a default record_batch factory path, and route file-slice merging through Arc.
  • Add trait-level conformance coverage for empty input, append-only, overwrite-with-latest, deletes, mixed updates/deletes, and null ordering values.

Validation:

  • make format-rust check-rust
  • cargo test -p hudi-core --no-fail-fast merge::
  • make test-rust

xushiyan added 6 commits May 15, 2026 01:05
…ordBatchMerger

Adds the RecordMerger trait abstraction while preserving the existing batch merge implementation for apache#611.
Adds the configurable record merger factory and shared validation path for apache#611.
Softens the merge contract for append-only behavior and qualifies the output schema doc link for apache#611.
Adds shared trait-level merge scenarios for the default implementation as part of apache#611.
Tightens validation coverage for distinct config error paths while working through apache#611 feedback.
}

let precombine_field = hudi_configs.try_get(OrderingFields)?;
if precombine_field.is_none() && merge_strategy == RecordMergeStrategyValue::OverwriteWithLatest

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@yihua currently ordering field must present for merging when using overwrite with latest strategy.
But i can simply change it to allow absent and then fallback to use commit time field. Then update the strategy name to event time and commit time ordering (both implicitly doing overwrite with latest). This will get these 2 ordering supported.

@codecov

codecov Bot commented May 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.44%. Comparing base (0910ca7) to head (07a23f1).

Files with missing lines Patch % Lines
crates/core/src/file_group/reader.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #612      +/-   ##
==========================================
+ Coverage   82.37%   82.44%   +0.06%     
==========================================
  Files          83       83              
  Lines        6311     6327      +16     
==========================================
+ Hits         5199     5216      +17     
+ Misses       1112     1111       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xushiyan xushiyan requested a review from yihua May 15, 2026 20:08
@xushiyan xushiyan added this to the release-0.5.0 milestone May 15, 2026
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.

feat(core): introduce pluggable RecordMerger trait for file-group merging

1 participant