Skip to content

Conversation

@twuebi
Copy link
Contributor

@twuebi twuebi commented Sep 27, 2025

  • exports UpdateTable so that outside packages can use it
  • adds metadata log maintenance to the builder, i.e. based on currentFileLocation, an entry is added to the metadata log & on Build, we trim the metadata log
  • ports remaining tests from iceberg-rust and implements fixes for them

Comment on lines -207 to +217
b.lastUpdatedMS = metadata.LastUpdatedMillis()
b.lastUpdatedMS = 0
Copy link
Member

Choose a reason for hiding this comment

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

what's the reasoning for this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's java & rust behavior, the if condition in Build() checks if b.lastUpdatedMS == 0 and only then updates it to Now(), according to spec, it should always be updated to Now():

image

}

func NewMetadataBuilder() (*MetadataBuilder, error) {
func NewMetadataBuilder(formatVersion int) (*MetadataBuilder, error) {
Copy link
Member

Choose a reason for hiding this comment

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

we should probably error if an invalid format version is passed (i.e. one we don't support)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, missed that, added, thanks!

@twuebi twuebi force-pushed the tp/update-table-and-fixes branch from 93f64ba to 7499241 Compare October 6, 2025 11:24
@zeroshade zeroshade merged commit dba25d8 into apache:main Oct 6, 2025
11 checks passed
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.

2 participants