Skip to content

Avoid unneeded schema change for view updates#10

Closed
SergeiPatiakin wants to merge 2 commits intomainfrom
update-view-order
Closed

Avoid unneeded schema change for view updates#10
SergeiPatiakin wants to merge 2 commits intomainfrom
update-view-order

Conversation

@SergeiPatiakin
Copy link
Copy Markdown

If the new schema is the same as the old schema, there is no need to update it

@SergeiPatiakin SergeiPatiakin requested a review from a team April 28, 2025 14:40
branch,
} => {
let schema_changed = metadata.current_schema(branch.as_deref())
.map(|s| schema != *s.fields())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

In theory identifier_field_ids can change while the fields remain the same, so we should also test for that when deducing a schema change?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In theory you're right, but looks like iceberg-rs doesn't support changing identifier_field_ids, so identifier_field_ids are not even represented in variant Operation::UpdateRepresentation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh I see; then no need to test it now.

@SergeiPatiakin
Copy link
Copy Markdown
Author

Merged upstream as JanKaul#166

@SergeiPatiakin
Copy link
Copy Markdown
Author

Closing because merged upstream

@SergeiPatiakin SergeiPatiakin deleted the update-view-order branch May 27, 2025 08:49
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.

3 participants