fix: resolve current_schema via current_schema_id, not the snapshot pin - #378
Merged
Merged
Conversation
TableMetadata::current_schema preferred the current snapshot's pinned schema_id over current_schema_id. That made SetCurrentSchema unreachable on any table with a snapshot: the old snapshot pinned the old schema id, and the Append/Replace operations stamp their new snapshot's schema_id from current_schema(), re-pinning the old schema on every commit — so a schema evolution could never take effect and writers kept producing the old shape. Per the Iceberg spec, the table's authoritative schema is current-schema-id; a snapshot's schema-id records the schema that was current when the snapshot was written (still reachable through TableMetadata::schema(snapshot_id) for snapshot-scoped reads). Fixes JanKaul#376.
This was referenced Jul 28, 2026
Closed
Closed
JanKaul
reviewed
Jul 29, 2026
JanKaul
left a comment
JanKaul
left a comment
Owner
There was a problem hiding this comment.
Thanks a lot the PR. The changes look good. I left a minor comment.
…tion_fields The table's current schema is a table-level property (current-schema-id) and does not vary by branch, so the parameter was dead. Remove it from TableMetadata::current_schema, TableMetadata::current_partition_fields, and the Table::current_schema delegate, and drop the now-unused branch plumbing from the crate-internal ManifestWriter and ManifestListWriter constructors. Views and materialized views keep their branch parameter, where the schema legitimately varies by version ref. Addresses review feedback on JanKaul#378.
cedricziel
added a commit
to cedricziel/signaldb
that referenced
this pull request
Jul 29, 2026
… bloom filters (#774) Pulls in JanKaul/iceberg-rust#378 (current_schema resolves via current_schema_id instead of the snapshot pin, unblocking schema evolution from the compaction path, #734) and #379 (per-column bloom-filter table properties in the Parquet writer). Adapts to the breaking signature change (current_schema no longer takes a branch parameter) and un-ignores test_schema_evolution_add_label_column, which was blocked on exactly the bug #378 fixed and now passes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #376.
TableMetadata::current_schemapreferred the current snapshot's pinnedschema_idovercurrent_schema_id. That madeSetCurrentSchemaunreachable on any table that already has a snapshot: the old snapshot pins the old schema id, and theAppend/Replaceoperations stamp their new snapshot'sschema_idfromcurrent_schema(), re-pinning the old schema on every commit — so a schema evolution could never take effect and writers (which derive their Arrow schema fromcurrent_schema) kept producing the old shape.Per the Iceberg spec, the table's authoritative schema is
current-schema-id; a snapshot'sschema-idrecords the schema that was current when that snapshot was written, and stays reachable viaTableMetadata::schema(snapshot_id)for snapshot-scoped reads (time travel is unaffected).With this change the four snapshot-stamping sites in
operation.rs(which all callcurrent_schema()) automatically stamp new snapshots with the current schema id, so the AddSchema → SetCurrentSchema → write sequence works end to end.Includes a regression test: metadata with
schemas {0, 1},current-schema-id: 1, and a snapshot pinned to schema 0 —current_schema(None)must return schema 1 whileschema(snapshot_id)still returns schema 0.Context: SignalDB drives schema evolution from its compaction path (cedricziel/signaldb#734) and hit this as a hard blocker.