Skip to content

Conversation

@aokolnychyi
Copy link
Contributor

@aokolnychyi aokolnychyi commented Feb 22, 2022

This PR fixes the name of the method that reads both data and delete files but claims to read only data files.

@github-actions github-actions bot added the spark label Feb 22, 2022
@aokolnychyi
Copy link
Contributor Author

@RussellSpitzer
Copy link
Member

All changes are private or protected so I don't think we have any issues with doing the rename. LGTM

Copy link
Member

@szehon-ho szehon-ho left a comment

Choose a reason for hiding this comment

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

Agree, looks good for internal refactor.

Grepping for 'data file', there are some comments on top of "BaseDeleteOrphanFilesSparkAction" and "BaseExpireSnapshotsSparkAction" that still refer to 'data file' not sure if they need to change or it's ok as is. In either case, this change LGTM

Copy link
Contributor

@kbendick kbendick left a comment

Choose a reason for hiding this comment

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

LGTM if all changes are package private etc. This is more coherent. Thanks @aokolnychyi!

@flyrain
Copy link
Contributor

flyrain commented Feb 22, 2022

Not directly related to this PR. The interface DataFile seems still a bit confusing since it can be a delete file.

"Contents of the file: 0=data, 1=position deletes, 2=equality deletes");
.

@github-actions github-actions bot added the API label Feb 22, 2022
@aokolnychyi
Copy link
Contributor Author

Fixed Javadoc, @szehon-ho.

@aokolnychyi aokolnychyi merged commit 3aace96 into apache:master Feb 22, 2022
@aokolnychyi
Copy link
Contributor Author

Thanks for the reviews, everyone!

arminnajafi pushed a commit to arminnajafi/iceberg that referenced this pull request Feb 23, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 9, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 10, 2022
hililiwei added a commit to hililiwei/iceberg that referenced this pull request Aug 11, 2022
aokolnychyi pushed a commit that referenced this pull request Aug 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants