Skip to content

Conversation

@rdblue
Copy link
Contributor

@rdblue rdblue commented Dec 5, 2022

This removes the use of TableOperations from metadata tables. Where TableMetadata is needed, operations is fetched from the Table instance instead of passing the ops explicitly. This is in preparation for metadata tables that are not tied to a particular Iceberg table.

@github-actions github-actions bot added the core label Dec 5, 2022
@rdblue rdblue force-pushed the core-remove-ops-from-metadata-tables branch 2 times, most recently from b8fbba7 to f9ed0d0 Compare January 8, 2023 21:33
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.

Looks good to me. I guess it also as a consequence removes TableOperations from the BaseTableScan hierarchy, but seems to me a good simplification if we never needed it there.

@Override
public FileIO io() {
return operations().io();
return ops.io();
Copy link
Member

Choose a reason for hiding this comment

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

These are unrelated , and just simplification/standardization, right? Fine with me either way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Where possible, I removed references to methods that expose TableOperations and deprecated the methods.

Here in BaseTable, I just wanted to be more consistent and use the instance field directly.

@nastra
Copy link
Contributor

nastra commented Jan 16, 2023

@rdblue looks like this needs some RevAPI changes because the APIs changed

@rdblue rdblue force-pushed the core-remove-ops-from-metadata-tables branch from f9ed0d0 to de03fc7 Compare January 16, 2023 17:47
@rdblue rdblue merged commit 8a70fe0 into apache:master Jan 16, 2023
@rdblue
Copy link
Contributor Author

rdblue commented Jan 16, 2023

Thanks for the review, @szehon-ho!

krvikash pushed a commit to krvikash/iceberg that referenced this pull request Mar 16, 2023
zhongyujiang pushed a commit to zhongyujiang/iceberg that referenced this pull request Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants