Skip to content

Core: View history entry core implementation - #6565

Merged
jackye1995 merged 1 commit into
apache:masterfrom
amogh-jahagirdar:view-history-entry-core
Jan 14, 2023
Merged

Core: View history entry core implementation#6565
jackye1995 merged 1 commit into
apache:masterfrom
amogh-jahagirdar:view-history-entry-core

Conversation

@amogh-jahagirdar

@amogh-jahagirdaramogh-jahagirdar commented Jan 11, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

Co-authored-by: John Zhuge jzhuge@apache.org

Separating this PR from https://github.com/apache/iceberg/pull/6559/files#diff-2a70d3056d3d0cca0da3ff4ddabc83c41c06af2296f281a5b37c5b54ead98915 for easier review from the community.

This PR contains the core implementation for view history entry and parsing/serializing logic for view history logs.

cc : @jzhuge@rdblue@jackye1995

jackye1995
jackye1995 reviewed Jan 11, 2023
View reviewed changes
Comment threadcore/src/test/resources/ViewHistoryEntry.json Outdated
jackye1995
jackye1995 reviewed Jan 11, 2023
View reviewed changes
Comment threadcore/src/main/java/org/apache/iceberg/view/BaseViewHistoryEntry.java Outdated
jackye1995
jackye1995 reviewed Jan 11, 2023
View reviewed changes
Comment threadcore/src/main/java/org/apache/iceberg/view/ViewHistoryEntryParser.java Outdated
jackye1995
jackye1995 reviewed Jan 11, 2023
View reviewed changes
Comment threadcore/src/test/java/org/apache/iceberg/view/ParserTestBase.java Outdated
@amogh-jahagirdar
amogh-jahagirdarforce-pushed the view-history-entry-core branch 4 times, most recently from 45fac9e to c95e4a8CompareJanuary 11, 2023 17:34
@amogh-jahagirdar

amogh-jahagirdar commented Jan 11, 2023

Copy link
Copy Markdown
ContributorAuthor

@jackye1995 I updated the PR so it should be a bit simpler now , I agree at this point we don't need the abstractions of base test classes . Thanks for the review! @jzhuge@rdblue let me know your thoughts on this!

jackye1995
jackye1995 approved these changes Jan 11, 2023
View reviewed changes

@jackye1995jackye1995 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me!

@jackye1995

jackye1995 commented Jan 12, 2023

Copy link
Copy Markdown
Contributor

Also ping a few people in #4925 to as we are starting to get all the implementations in

@rdblue@stevenzwu@danielcweeks@wmoustafa@nastra

nastra
nastra reviewed Jan 13, 2023
View reviewed changes

@nastranastra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

overall LGTM but needs a few small updates

Comment threadcore/src/main/java/org/apache/iceberg/view/ViewHistoryEntryParser.java Outdated
Comment threadcore/src/main/java/org/apache/iceberg/view/ViewHistoryEntryParser.java
Comment threadcore/src/test/java/org/apache/iceberg/view/TestViewHistoryEntryParser.java
@nastranastra mentioned this pull request Jan 13, 2023
Closed
rdblue
rdblue reviewed Jan 13, 2023
View reviewed changes
Comment threadcore/src/main/java/org/apache/iceberg/view/BaseViewHistoryEntry.java Outdated
@rdblue

rdblue commented Jan 13, 2023

Copy link
Copy Markdown
Contributor

Looks good other than the name of the history entry interface. Thanks, @amogh-jahagirdar!

@github-actionsgithub-actionsBot removed the API label Jan 13, 2023
@amogh-jahagirdar@jzhuge
Core: View history entry core implementation
2c334da
Co-authored-by: John Zhuge <jzhuge@apache.org>
@jackye1995

jackye1995 commented Jan 14, 2023

Copy link
Copy Markdown
Contributor

Since the immutable is addressed and we have enough approvals, I will go ahead to merge the PR, thanks for the review @rdblue and @nastra !

@jackye1995
jackye1995 merged commit c05bde8 into apache:masterJan 14, 2023
@jzhuge

jzhuge commented Jan 17, 2023

Copy link
Copy Markdown
Member

LGTM!

zhongyujiang pushed a commit to zhongyujiang/iceberg that referenced this pull request Apr 16, 2025
@amogh-jahagirdar@jzhuge钟宇江
Core: View history entry core implementation (apache#6565)
f322b84
Co-authored-by: John Zhuge <jzhuge@apache.org>
(cherry picked from commit c05bde8)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nastranastranastra left review comments
@rdbluerdbluerdblue approved these changes
@jackye1995jackye1995jackye1995 approved these changes

Assignees

No one assigned

Labels

APIcore

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@amogh-jahagirdar@jackye1995@rdblue@jzhuge@nastra