-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[HUDI-7652] Add new HoodieMergeKey
API to support simple and composite keys
#11077
Conversation
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieMergeKey.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/HoodieSimpleMergeKey.java
Outdated
Show resolved
Hide resolved
19a23e3
to
7ee2125
Compare
d3106b9
to
1c75aaf
Compare
1c75aaf
to
0eff97c
Compare
12e1585
to
e399a80
Compare
@danny0405 Based on our discussion, I have removed the |
hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
...mon/src/main/java/org/apache/hudi/common/table/log/HoodieMetadataMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
...mon/src/main/java/org/apache/hudi/common/table/log/HoodieMetadataMergedLogRecordScanner.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, I have left some comments.
...mon/src/main/java/org/apache/hudi/common/table/log/HoodieMetadataMergedLogRecordScanner.java
Show resolved
Hide resolved
e399a80
to
10084e8
Compare
hudi-common/src/main/java/org/apache/hudi/common/table/log/HoodieMergedLogRecordScanner.java
Outdated
Show resolved
Hide resolved
hudi-common/src/main/java/org/apache/hudi/metadata/HoodieMetadataLogRecordReader.java
Outdated
Show resolved
Hide resolved
bac0e33
to
17ab2be
Compare
hudi-cli/src/main/java/org/apache/hudi/cli/commands/HoodieLogFileCommand.java
Outdated
Show resolved
Hide resolved
17ab2be
to
fca2d12
Compare
...mon/src/main/java/org/apache/hudi/common/table/log/HoodieMetadataMergedLogRecordScanner.java
Show resolved
Hide resolved
Add test for new record merger
…ss mergedLogRecordScanner
…and reuse builders
dce60c0
to
399ffff
Compare
Change Logs
This PR introduces a new class hierarchy for handling merge keys in a more flexible and decoupled manner. It adds the
HoodieMergeKey
interface, along with two implementations:HoodieSimpleMergeKey
andHoodieCompositeMergeKey
. This design allows us to extend key-based merge strategies easily.Motivation
The need for introducing a new merge key handling mechanism was driven by the requirement to support different types of keys (simple and complex) without overloading the existing HoodieKey class, which is central to the write path. By segregating merge key handling into its own hierarchy, we avoid potential conflicts and keep modifications localised, improving the maintainability of the code.
Changes
HoodieMergeKey
: New API to ensure consistent handling including simple keys and composite keys. It includes methods for retrieving the key and partition path.HoodieSimpleMergeKey
: WrapsHoodieKey
and implements theHoodieMergeKey
interface for simple scenarios where the key is a string.HoodieCompositeMergeKey
: Implements theHoodieMergeKey
interface but allows for complex types as keys, enhancing flexibility for scenarios where a simple string key is not sufficient.HoodieMergeKeyBasedRecordMerger
: A new implementation ofHoodieRecordMerger
based onHoodieMergeKey
. If the merge keys are of typeHoodieCompositeMergeKey
, then it returns the older and newer records. Otherwise, it calls the merge method from the parent class.HoodieMergedLogRecordScanner
: Changes to merge based onHoodieMergeKey
.These changes do not affect existing functionalities that do not rely on merge keys. It introduces additional classes that are used explicitly for new functionalities involving various key types in merging operations. This ensures minimal to no risk for existing processes.
Impact
Enhancing the flexibility and robustness of our key-based merge strategies. It helps in keeping our codebase scalable and maintainable, allowing easy extensions and modifications in the future.
Risk level (write none, low medium or high below)
low
Documentation Update
Describe any necessary documentation update if there is any new feature, config, or user-facing change. If not, put "none".
ticket number here and follow the instruction to make
changes to the website.
Contributor's checklist