Skip to content

Conversation

gty404
Copy link
Contributor

@gty404 gty404 commented Oct 12, 2025

This commit introduces the metadata update and update requirements system for table metadata modifications.

@gty404 gty404 force-pushed the table-metadata branch 4 times, most recently from 537df38 to b147683 Compare October 12, 2025 07:25
This commit introduces the metadata update and update requirements
system for table metadata modifications.
@gty404 gty404 changed the title feat: implement metadata updates and update requirements feat: add metadata updates and requirements interface Oct 15, 2025
Copy link
Collaborator

@zhjwpku zhjwpku left a comment

Choose a reason for hiding this comment

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

I was initially confused about renaming MetadataUpdate to TableUpdate and UpdateRequirement to TableRequirement, but after checking the iceberg-rust and iceberg-python implementations, it now makes sense to me.

return input;
}

static bool EqualsIgnoreCase(const std::string& a, const std::string& b) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: We have some operator== definitions that use lhs/rhs as the parameters names, should we follow the same convention here?

table.cc
table_metadata.cc
table_properties.cc
table_requirement.cc
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess you should also add these files to meson.build.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Perhaps the CI does not fail because the file is not referenced by others.

@wgtmac
Copy link
Member

wgtmac commented Oct 16, 2025

While I know both iceberg-python and iceberg-rust name them as TableUpdate and TableRequirement. However, it might looks weird to use Table prefix in terms of dealing with updates to table, view and perhaps the in-discussion materialized view.

WDYT? @Fokko @liurenjie1024

@liurenjie1024
Copy link

While I know both iceberg-python and iceberg-rust name them as TableUpdate and TableRequirement. However, it might looks weird to use Table prefix in terms of dealing with updates to table, view and perhaps the in-discussion materialized view.

WDYT? @Fokko @liurenjie1024

Thanks @wgtmac for pinging me. I prefer to add an prefix to improve readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants