[WIP] [HUDI-834] Concrete signature of HoodieRecordPayload#combineAndGetUpdateValue & HoodieRecordPayload#getInsertValue#1557
Closed
tisonkun wants to merge 1 commit intoapache:masterfrom
Closed
Conversation
…ateValue & HoodieRecordPayload#getInsertValue
Codecov Report
@@ Coverage Diff @@
## master #1557 +/- ##
============================================
- Coverage 72.35% 71.69% -0.67%
Complexity 294 294
============================================
Files 374 378 +4
Lines 16377 16549 +172
Branches 1650 1670 +20
============================================
+ Hits 11849 11864 +15
- Misses 3797 3954 +157
Partials 731 731
Continue to review full report at Codecov.
|
Member
Author
|
Hold if |
Contributor
|
+1, LGTM |
vinothchandar
requested changes
Apr 24, 2020
Member
vinothchandar
left a comment
vinothchandar
left a comment
There was a problem hiding this comment.
Marking this as WIP for further discussion.. Thanks @tisonkun for taking a pass at this..
It's an user facing API.. we cannot break it.. We could introduce something new MergeHooks and still respect the Payload class if that's doable.. say.. Need to think more deeply
vinothchandar
changed the title
Member
|
Closing with a WIP tag for future follow up. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the purpose of the pull request
So far, the return type of HoodieRecordPayload#combineAndGetUpdateValue & HoodieRecordPayload#getInsertValue is effectively Option. Instead of doing unchecked cast at
org/apache/hudi/hadoop/realtime/RealtimeCompactedRecordReader.java:88
I propose we use Option as the return type of these two method, which replaces current Option.
FYI, I encounter this ticket when trying to get rid of self type parameter in HoodieRecordPayload and found that it is a bit awkward if we don't take a self type while doing this casting. Fortunately it is the fact that we can directly concrete it.
cc @vinothchandar @leesf
Verify this pull request
This pull request is code refactor should be covered by existing tests.
Committer checklist
Has a corresponding JIRA in PR title & commit
Commit message is descriptive of the change
CI is green