This is an automated email from the ASF dual-hosted git repository.
zeroshade pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-go.git
The following commit(s) were added to refs/heads/main by this push:
new a1e50aa5c fix(table): avoid duplicate column IDs when last-column-id
is stale (#2453)
a1e50aa5c is described below
commit a1e50aa5c9b0bace998eaece66a5c0850af8977e
Author: Shruthi <[email protected]>
AuthorDate: Mon Nov 9 09:27:28 2026 -0800
fix(table): avoid duplicate column IDs when last-column-id is stale (#2453)
* fix(table): return error from UpdateSchema when last-column-id is below
assigned field IDs
Signed-off-by: Shruthi Yaramada <[email protected]>
* fix(table): seed column ID counter from schema history
Signed-off-by: Shruthi Yaramada <[email protected]>
---------
Signed-off-by: Shruthi Yaramada <[email protected]>
Co-authored-by: Shruthi Yaramada <[email protected]>
---
table/metadata.go | 14 ++++++++++-
table/update_schema.go | 13 ++++++-----
table/update_schema_test.go | 57 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 77 insertions(+), 7 deletions(-)
diff --git a/table/metadata.go b/table/metadata.go
index 1ba78b3bf..01e203b18 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -823,7 +823,7 @@ func (b *MetadataBuilder) AddSchema(schema *iceberg.Schema)
error {
return nil
}
- b.lastColumnId = max(b.lastColumnId, schema.HighestFieldID())
+ b.lastColumnId = max(columnIDFloor(b.lastColumnId, b.schemaList),
schema.HighestFieldID())
schema.ID = newSchemaID
@@ -852,6 +852,18 @@ func partitionFieldIDFloor(lastPartitionID *int, specs
[]iceberg.PartitionSpec)
return floor
}
+// columnIDFloor returns an allocation floor for new field IDs that accounts
+// for the persisted last-column-id and every field ID in schema history, so
+// a stale counter can never cause an ID to be reused.
+func columnIDFloor(lastColumnID int, schemas []*iceberg.Schema) int {
+ floor := lastColumnID
+ for _, schema := range schemas {
+ floor = max(floor, schema.HighestFieldID())
+ }
+
+ return floor
+}
+
func (b *MetadataBuilder) AddPartitionSpec(spec *iceberg.PartitionSpec,
initial bool) error {
newSpecID := b.reuseOrCreateNewPartitionSpecID(*spec)
curSchema := b.CurrentSchema()
diff --git a/table/update_schema.go b/table/update_schema.go
index 69ee4951d..45f7e8cc3 100644
--- a/table/update_schema.go
+++ b/table/update_schema.go
@@ -167,12 +167,13 @@ func NewUpdateSchema(txn *Transaction, caseSensitive
bool, allowIncompatibleChan
return u
}
- // Seed from metadata's last-column-id rather than the current schema's
- // highest field id. Per the Iceberg spec, last-column-id is a monotonic
- // counter that preserves ids across schema evolution (including
- // deletions) so that newly-allocated ids never collide with ids still
- // referenced by historical schemas.
- u.lastColumnID = meta.LastColumnID()
+ // Seed from the larger of metadata's last-column-id and the highest
+ // field id in any schema, including historical ones. Per the Iceberg
+ // spec, last-column-id is a monotonic counter that preserves ids across
+ // schema evolution (including deletions), but it may be stale in
+ // metadata written by other tools. Taking the max ensures
newly-allocated
+ // ids never collide with ids in the current or historical schemas.
+ u.lastColumnID = columnIDFloor(meta.LastColumnID(), meta.schemaList)
for _, opt := range opts {
opt(u)
diff --git a/table/update_schema_test.go b/table/update_schema_test.go
index 6a1d795a6..db1a73b06 100644
--- a/table/update_schema_test.go
+++ b/table/update_schema_test.go
@@ -19,6 +19,7 @@ package table
import (
"encoding/json"
+ "strings"
"testing"
"github.com/apache/iceberg-go"
@@ -58,6 +59,62 @@ func TestNewUpdateSchemaWithNilTransactionReturnsError(t
*testing.T) {
assert.ErrorContains(t, err, "transaction is nil")
}
+func TestUpdateSchemaStaleLastColumnIDAssignsFreshID(t *testing.T) {
+ // Field "z" has ID 3, but the stale counter says 2. The new column
+ // must get an ID above every existing one (4), not reuse ID 3.
+ data := strings.Replace(ExampleTableMetadataV2,
+ `"last-column-id": 3`, `"last-column-id": 2`, 1)
+ meta, err := ParseMetadataString(data)
+ require.NoError(t, err)
+ require.Equal(t, 2, meta.LastColumnID())
+
+ tbl := New([]string{"id"}, meta, "", nil, nil)
+ txn := tbl.NewTransaction()
+
+ newSchema, err := txn.UpdateSchema(true, false).
+ AddColumn([]string{"w"}, iceberg.PrimitiveTypes.Int32, "",
false, nil).
+ Apply()
+ require.NoError(t, err)
+
+ w, ok := newSchema.FindFieldByName("w")
+ require.True(t, ok)
+ assert.Equal(t, 4, w.ID)
+}
+
+func TestUpdateSchemaAssignsIDAboveHistoricalSchemas(t *testing.T) {
+ // Field "old" (ID 5) exists only in the older schema 0, as if it was
+ // dropped, and last-column-id is still 3. The new column must not reuse
+ // the dropped column's ID.
+ data := strings.Replace(ExampleTableMetadataV2,
+ `"schema-id": 0, "fields": [{"id": 1, "name": "x", "required":
true, "type": "long"}]`,
+ `"schema-id": 0, "fields": [{"id": 1, "name": "x", "required":
true, "type": "long"}, {"id": 5, "name": "old", "required": false, "type":
"long"}]`,
+ 1)
+ require.NotEqual(t, ExampleTableMetadataV2, data)
+ meta, err := ParseMetadataString(data)
+ require.NoError(t, err)
+ require.Equal(t, 3, meta.LastColumnID())
+
+ tbl := New([]string{"id"}, meta, "", nil, nil)
+ txn := tbl.NewTransaction()
+
+ newSchema, err := txn.UpdateSchema(true, false).
+ AddColumn([]string{"w"}, iceberg.PrimitiveTypes.Int32, "",
false, nil).
+ Apply()
+ require.NoError(t, err)
+
+ w, ok := newSchema.FindFieldByName("w")
+ require.True(t, ok)
+ assert.Equal(t, 6, w.ID)
+
+ // Even a rename should fix the stored counter (3 -> 5).
+ require.NoError(t, txn.UpdateSchema(true, false).
+ RenameColumn([]string{"y"}, "y2").
+ Commit())
+ written, err := txn.txnMeta()
+ require.NoError(t, err)
+ assert.Equal(t, 5, written.LastColumnID())
+}
+
func TestAddColumn(t *testing.T) {
t.Run("test update schema with add primitive type on top level", func(t
*testing.T) {
table := New([]string{"id"}, testMetadata, "", nil, nil)
