-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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-4920] fix PartialUpdatePayload cannot return deleted record in … #6799
Conversation
2004f89
to
893c80d
Compare
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.
LGTM. is it possible to write a test covering the change.
|
hudi-common/src/main/java/org/apache/hudi/common/model/PartialUpdateAvroPayload.java
Outdated
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 update. Few comments. Looking closer to approving.
hudi-common/src/main/java/org/apache/hudi/common/model/PartialUpdateAvroPayload.java
Outdated
Show resolved
Hide resolved
hudi-common/src/test/java/org/apache/hudi/common/model/TestPartialUpdateAvroPayload.java
Show resolved
Hide resolved
LGTM. Will approve once the test passes and I make a final pass. |
@fengjian428 : Can you rebase against latest master and also check the failing test. |
b828393
to
4468648
Compare
done |
@hudi-bot run azure |
@bvaradar fixed |
@hudi-bot run azure |
apache#6799) Co-authored-by: fengjian <[email protected]>
apache#6799) Co-authored-by: fengjian <[email protected]>
apache#6799) Co-authored-by: fengjian <[email protected]>
Change Logs
fix PartialUpdatePayload cannot return deleted record in preCombine function issue
Impact
minor
Risk level (write none, low medium or high below)
low
Contributor's checklist