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 74a3ac683 perf(name_mapping): reduce allocations for wide mapping
updates (#2267)
74a3ac683 is described below
commit 74a3ac68364bda275c613518e1ee4989e7f7566e
Author: Minh Vu <[email protected]>
AuthorDate: Fri Oct 23 18:22:29 2026 +0200
perf(name_mapping): reduce allocations for wide mapping updates (#2267)
* perf(name_mapping): update wide mappings in one pass
Signed-off-by: Minh Vu <[email protected]>
* test(name_mapping): preserve nil result for removed aliases
Signed-off-by: Minh Vu <[email protected]>
* style(name_mapping): satisfy lint
Signed-off-by: Minh Vu <[email protected]>
* ci: retrigger flaky macOS race test
Signed-off-by: Minh Vu <[email protected]>
---------
Signed-off-by: Minh Vu <[email protected]>
---
name_mapping.go | 127 ++++++++++++++++++++++++------------------
name_mapping_bench_test.go | 2 +-
name_mapping_one_pass_test.go | 89 +++++++++++++++++++++++++++++
3 files changed, 164 insertions(+), 54 deletions(-)
diff --git a/name_mapping.go b/name_mapping.go
index 243a102a0..d2beb6e0f 100644
--- a/name_mapping.go
+++ b/name_mapping.go
@@ -141,13 +141,16 @@ func visitMappedFields[S, T any](fields []MappedField,
visitor NameMappingVisito
// After update: {FieldID: 1, Names: ["foo", "foo_update"]}
//
// This preserves compatibility with existing data files that reference the
old field names.
-func UpdateNameMapping(nameMapping NameMapping, updates map[int]NestedField,
adds map[int][]NestedField) (NameMapping, error) {
- result, err := VisitNameMapping(nameMapping,
&updateNameMappingVisitor{updates: updates, adds: adds})
- if err != nil {
- return nil, err
+func UpdateNameMapping(nameMapping NameMapping, updates map[int]NestedField,
adds map[int][]NestedField) (result NameMapping, err error) {
+ if nameMapping == nil {
+ return nil, fmt.Errorf("%w: cannot visit nil NameMapping",
ErrInvalidArgument)
}
- return NameMapping(result), nil
+ defer internal.RecoverError(&err)
+
+ updater := updateNameMappingVisitor{updates: updates, adds: adds}
+
+ return
NameMapping(updater.addNewFields(updater.updatedFields(nameMapping), -1)), nil
}
type updateNameMappingVisitor struct {
@@ -155,50 +158,65 @@ type updateNameMappingVisitor struct {
adds map[int][]NestedField
}
-func (u *updateNameMappingVisitor) Mapping(nm NameMapping, fieldResults
[]MappedField) []MappedField {
- return u.addNewFields(fieldResults, -1)
-}
+// updatedFields builds independent copies while handling renames at each
sibling
+// level. It avoids allocating an intermediate visitor-result slice per level.
+func (u *updateNameMappingVisitor) updatedFields(fields []MappedField)
[]MappedField {
+ if len(fields) == 0 {
+ return nil
+ }
-func (u *updateNameMappingVisitor) Fields(st []MappedField, fieldResults
[]MappedField) []MappedField {
- reassignments := make(map[string]int)
- for _, field := range fieldResults {
- if field.FieldID != nil {
+ // A rename may take an alias from a sibling, so find the new owners
before
+ // building the output. Most levels have no updates and need no map.
+ var reassignments map[string]int
+ if len(u.updates) != 0 {
+ for _, field := range fields {
+ if field.FieldID == nil {
+ continue
+ }
if update, exists := u.updates[*field.FieldID]; exists {
+ if reassignments == nil {
+ reassignments = make(map[string]int)
+ }
reassignments[update.Name] = update.ID
}
}
}
- var result []MappedField
- for _, field := range fieldResults {
- updatedField := u.removeReassignedNames(field, reassignments)
- if updatedField != nil {
- result = append(result, *updatedField)
+ result := make([]MappedField, 0, len(fields))
+ // Keep the copied IDs together rather than allocating one int per
field.
+ // Every returned FieldID points into this independent backing slice.
+ var copiedIDs []int
+ for _, field := range fields {
+ children := u.updatedFields(field.Fields)
+ updated := MappedField{
+ Names: slices.Clone(field.Names),
+ Fields: children,
}
- }
+ if field.FieldID != nil {
+ if copiedIDs == nil {
+ copiedIDs = make([]int, len(fields))
+ }
+ id := *field.FieldID
+ copiedIDs[len(result)] = id
+ updated.FieldID = &copiedIDs[len(result)]
- return result
-}
+ if rename, exists := u.updates[id]; exists &&
!slices.Contains(updated.Names, rename.Name) {
+ updated.Names = append(updated.Names,
rename.Name)
+ }
+ updated.Fields = u.addNewFields(children, id)
+ }
-func (u *updateNameMappingVisitor) Field(field MappedField, fieldResult
[]MappedField) MappedField {
- if field.FieldID == nil {
- return MappedField{
- Names: slices.Clone(field.Names),
- Fields: fieldResult,
+ if retained := u.removeReassignedNames(updated, reassignments);
retained != nil {
+ result = append(result, *retained)
}
}
- fieldID := *field.FieldID
- fieldNames := slices.Clone(field.Names)
- if update, exists := u.updates[*field.FieldID]; exists &&
!slices.Contains(fieldNames, update.Name) {
- fieldNames = append(fieldNames, update.Name)
+ // The original visitor returned nil when every alias was discarded.
+ if len(result) == 0 {
+ return nil
}
- return MappedField{
- FieldID: &fieldID,
- Names: fieldNames,
- Fields: u.addNewFields(fieldResult, *field.FieldID),
- }
+ return result
}
func (u *updateNameMappingVisitor) removeReassignedNames(field MappedField,
assignments map[string]int) *MappedField {
@@ -214,7 +232,6 @@ func (u *updateNameMappingVisitor)
removeReassignedNames(field MappedField, assi
if len(remainingNames) == 0 {
return nil
}
-
field.Names = remainingNames
return &field
@@ -226,32 +243,36 @@ func (u *updateNameMappingVisitor)
addNewFields(mappedFields []MappedField, pare
return mappedFields
}
- newFields := make([]MappedField, 0, len(fieldsToAdd))
- for _, add := range fieldsToAdd {
- fields := visitField(add, createMapping{})
- if len(fields) == 0 {
- fields = nil
- }
-
- newFields = append(newFields, MappedField{
- FieldID: &add.ID,
- Names: []string{add.Name},
- Fields: fields,
- })
- }
- reassignments := make(map[string]int)
+ reassignments := make(map[string]int, len(fieldsToAdd))
for _, add := range fieldsToAdd {
reassignments[add.Name] = add.ID
}
- fields := make([]MappedField, 0)
+ // mappedFields was freshly allocated by updatedFields, so compacting it
+ // here cannot change the caller's original mapping.
+ fields := mappedFields[:0]
+ if fields == nil {
+ fields = make([]MappedField, 0, len(fieldsToAdd))
+ }
for _, field := range mappedFields {
- if updatedField := u.removeReassignedNames(field,
reassignments); updatedField != nil {
- fields = append(fields, *updatedField)
+ if retained := u.removeReassignedNames(field, reassignments);
retained != nil {
+ fields = append(fields, *retained)
}
}
- return append(fields, newFields...)
+ for _, add := range fieldsToAdd {
+ children := visitField(add, createMapping{})
+ if len(children) == 0 {
+ children = nil
+ }
+ fields = append(fields, MappedField{
+ FieldID: &add.ID,
+ Names: []string{add.Name},
+ Fields: children,
+ })
+ }
+
+ return fields
}
type NameMappingAccessor struct{}
diff --git a/name_mapping_bench_test.go b/name_mapping_bench_test.go
index 6cd10d650..ca13e5c44 100644
--- a/name_mapping_bench_test.go
+++ b/name_mapping_bench_test.go
@@ -68,7 +68,7 @@ func benchmarkNameMappingFields(count int)
iceberg.NameMapping {
}
func BenchmarkUpdateNameMapping(b *testing.B) {
- for _, size := range []int{2, 8, 256, 1024} {
+ for _, size := range []int{2, 8, 256, 1024, 4096} {
for _, operation := range []string{"no-updates", "rename",
"rename-reused", "add-reused"} {
b.Run(fmt.Sprintf("%s/%d", operation, size), func(b
*testing.B) {
mapping := benchmarkNameMappingFields(size)
diff --git a/name_mapping_one_pass_test.go b/name_mapping_one_pass_test.go
new file mode 100644
index 000000000..27ad9c76a
--- /dev/null
+++ b/name_mapping_one_pass_test.go
@@ -0,0 +1,89 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package iceberg_test
+
+import (
+ "fmt"
+ "testing"
+
+ "github.com/apache/iceberg-go"
+ "github.com/stretchr/testify/require"
+)
+
+func TestUpdateNameMappingWideFieldIDsIndependent(t *testing.T) {
+ const fieldCount = 256
+ mapping := make(iceberg.NameMapping, fieldCount)
+ for i := range mapping {
+ mapping[i] = iceberg.MappedField{
+ FieldID: new(i + 1),
+ Names: []string{fmt.Sprintf("field_%d", i)},
+ }
+ }
+
+ updated, err := iceberg.UpdateNameMapping(mapping, nil, nil)
+ require.NoError(t, err)
+ require.Equal(t, mapping, updated)
+
+ *updated[0].FieldID = 9000
+ *updated[1].FieldID = 9001
+ updated[2].Names[0] = "changed"
+
+ require.Equal(t, 1, *mapping[0].FieldID)
+ require.Equal(t, 2, *mapping[1].FieldID)
+ require.Equal(t, "field_2", mapping[2].Names[0])
+ require.Equal(t, 3, *updated[2].FieldID)
+ require.Equal(t, 4, *updated[3].FieldID)
+}
+
+func TestUpdateNameMappingEmptyAndAnonymous(t *testing.T) {
+ _, err := iceberg.UpdateNameMapping(nil, nil, nil)
+ require.ErrorIs(t, err, iceberg.ErrInvalidArgument)
+
+ empty, err := iceberg.UpdateNameMapping(
+ iceberg.NameMapping{}, nil, map[int][]iceberg.NestedField{-1:
{}},
+ )
+ require.NoError(t, err)
+ require.NotNil(t, empty)
+ require.Empty(t, empty)
+
+ mapping := iceberg.NameMapping{
+ {Names: []string{"alias"}, Fields: []iceberg.MappedField{
+ {FieldID: new(7), Names: []string{"old", "current"}},
+ }},
+ {Names: []string{}},
+ {FieldID: new(8), Names: []string{"other"}},
+ }
+ updated, err := iceberg.UpdateNameMapping(
+ mapping,
+ map[int]iceberg.NestedField{7: {ID: 7, Name: "renamed"}},
+ nil,
+ )
+ require.NoError(t, err)
+ require.Len(t, updated, 2)
+ require.Nil(t, updated[0].FieldID)
+ require.Equal(t, []string{"old", "current", "renamed"},
updated[0].Fields[0].Names)
+ require.Equal(t, 8, *updated[1].FieldID)
+ require.Len(t, mapping, 3)
+}
+
+func TestUpdateNameMappingPrunesNamelessMappingToNil(t *testing.T) {
+ mapping := iceberg.NameMapping{{Names: []string{}}}
+ updated, err := iceberg.UpdateNameMapping(mapping, nil, nil)
+ require.NoError(t, err)
+ require.Nil(t, updated)
+}
