Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[HUDI-1255] Add new Payload(OverwriteNonDefaultsWithLatestAvroPayload) for updating specified fields in storage #2056

Merged
merged 10 commits into from
Sep 10, 2020
Merged

[HUDI-1255] Add new Payload(OverwriteNonDefaultsWithLatestAvroPayload) for updating specified fields in storage #2056

merged 10 commits into from
Sep 10, 2020

Conversation

Karl-WangSK
Copy link
Contributor

@Karl-WangSK Karl-WangSK commented Aug 31, 2020 •
edited
Loading

Tips

What is the purpose of the pull request

Add new Payload(OverwriteNonDefaultsWithLatestAvroPayload) for updating specified fields in storage

Brief change log

update current value for several fields that you want to change.

The default payload OverwriteWithLatestAvroPayload overwrite the whole record when

compared to orderingVal.This doesn't meet our need when we just want to change specified fields.
For example: (suppose Default value is null)

current Value 
Field:      name   age   gender
Value:     karl     20    male

insert Value
Field:      name   age   gender
Value:     null     30    null

After insert:
Field:      name   age   gender
Value:     karl     30    male

Verify this pull request

Added TestOverwriteNonDefaultsWithLatestAvroPayload to verify the change.

Committer checklist

  • Has a corresponding JIRA in PR title & commit

  • Commit message is descriptive of the change

  • CI is green

  • Necessary doc changes done or have another open PR

  • For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.

@Karl-WangSK
Copy link
Contributor Author

hi, can u take a look when freeee ? @yanghua

vinothchandar
vinothchandar reviewed Aug 31, 2020
View reviewed changes
Copy link
Member

@vinothchandar vinothchandar left a comment

Choose a reason for hiding this comment

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

Some high level comments. This is a great contribution. thanks!

hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteWithLatestAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteMulColAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteMulColAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteMulColAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteWithLatestAvroPayload.java Outdated Show resolved Hide resolved
yanghua
yanghua reviewed Sep 1, 2020
View reviewed changes
Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

Left some minor comments.

...on/src/main/java/org/apache/hudi/common/model/OverwriteNonDefaultsWithLatestAvroPayload.java Outdated Show resolved Hide resolved
...on/src/main/java/org/apache/hudi/common/model/OverwriteNonDefaultsWithLatestAvroPayload.java Outdated Show resolved Hide resolved
@yanghua yanghua changed the title [HUDI-1255]:Add new Payload(OverwriteMulColAvroPayload) for updating specified fields in storage [HUDI-1255] Add new Payload(OverwriteMulColAvroPayload) for updating specified fields in storage Sep 1, 2020
@Karl-WangSK
Copy link
Contributor Author

Karl-WangSK commented Sep 3, 2020 •
edited
Loading

hi, ready to merge if no other problems @yanghua @vinothchandar .Thanks!!!

@yanghua
Copy link
Contributor

yanghua commented Sep 3, 2020

hi, ready to merge if no other problems @yanghua @vinothchandar.Thanks!!!

@Karl-WangSK I will review again soon.

yanghua
yanghua requested changes Sep 3, 2020
View reviewed changes
Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@Karl-WangSK Left some new comments.

...on/src/main/java/org/apache/hudi/common/model/OverwriteNonDefaultsWithLatestAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteWithLatestAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteWithLatestAvroPayload.java Outdated Show resolved Hide resolved
hudi-common/src/main/java/org/apache/hudi/common/model/OverwriteWithLatestAvroPayload.java Outdated Show resolved Hide resolved
...rc/test/java/org/apache/hudi/common/model/TestOverwriteNonDefaultsWithLatestAvroPayload.java Outdated Show resolved Hide resolved
@Karl-WangSK Karl-WangSK changed the title [HUDI-1255] Add new Payload(OverwriteMulColAvroPayload) for updating specified fields in storage [HUDI-1255] Add new Payload(OverwriteNonDefaultsWithLatestAvroPayload) for updating specified fields in storage Sep 3, 2020
yanghua
yanghua approved these changes Sep 3, 2020
View reviewed changes
Copy link
Contributor

@yanghua yanghua left a comment

Choose a reason for hiding this comment

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

@Karl-WangSK thanks for your contribution! LGTM now, let's wait for a moment to see if @vinothchandar has any concern, OK?

@Karl-WangSK
Copy link
Contributor Author

@Karl-WangSK thanks for your contribution! LGTM now, let's wait for a moment to see if @vinothchandar has any concern, OK?

OK，Thanks for your time and review! @yanghua

@yanghua
Copy link
Contributor

yanghua commented Sep 3, 2020

@Karl-WangSK thanks for your contribution! LGTM now, let's wait for a moment to see if @vinothchandar has any concern, OK?

OK，Thanks for your time and review! @yanghua

These are not worth mentioning. It is lucky for the community to have contributors like you.

@yanghua
Copy link
Contributor

yanghua commented Sep 7, 2020

@vinothchandar Any feedback on this PR?

@vinothchandar
Copy link
Member

sorry, long weekend here in the states. will take a look today

@vinothchandar vinothchandar merged commit a1cff8a into apache:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yanghua yanghua yanghua approved these changes

@vinothchandar vinothchandar vinothchandar approved these changes

Assignees

@vinothchandar vinothchandar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Karl-WangSK @yanghua @vinothchandar