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 c9b8a7e3f perf(table): reuse partition formatting plans in snapshot
summaries (#2200)
c9b8a7e3f is described below
commit c9b8a7e3fb1713a12de78db375ce297018875d11
Author: Minh Vu <[email protected]>
AuthorDate: Mon Oct 19 19:34:00 2026 +0200
perf(table): reuse partition formatting plans in snapshot summaries (#2200)
* perf(table): reuse partition formatting plans in snapshot summaries
* perf(table): guard snapshot partition plan cache against spec ID reuse
Signed-off-by: Minh Vu <[email protected]>
* test(table): benchmark escaped partition path formatting
* style(table): simplify escaped path benchmark cases
---------
Signed-off-by: Minh Vu <[email protected]>
---
table/snapshots.go | 56 ++++++++++++++++++++--
table/snapshots_bench_test.go | 100 ++++++++++++++++++++++++++++++++++++++-
table/snapshots_internal_test.go | 62 ++++++++++++++++++++++++
3 files changed, 213 insertions(+), 5 deletions(-)
diff --git a/table/snapshots.go b/table/snapshots.go
index a66acce83..6e75bbce5 100644
--- a/table/snapshots.go
+++ b/table/snapshots.go
@@ -580,9 +580,59 @@ func SnapshotLogEntryAsOf(entries
iter.Seq[SnapshotLogEntry], timestampMs int64,
type SnapshotSummaryCollector struct {
metrics updateMetrics
partitionMetrics map[string]updateMetrics
+ partitionPathPlans
map[snapshotPartitionPathKey]snapshotPartitionPathPlan
maxChangedPartitionsForSummaries int
}
+// Spec IDs are unique only within a table. Include schema and spec identity
+// so an independently constructed spec with the same ID cannot reuse a stale
+// formatting plan if the collector is used across table metadata.
+type snapshotPartitionPathKey struct {
+ schema *iceberg.Schema
+ specID int
+ firstField *iceberg.PartitionField
+}
+
+type snapshotPartitionPathPlan struct {
+ partitionType *iceberg.StructType
+ pathPlan partitionPathPlan
+}
+
+// partitionPath reuses schema-dependent formatting state across files in the
+// same snapshot summary. The partition field slice is immutable for the
+// lifetime of its spec, so its first element identifies the spec's layout.
+func (s *SnapshotSummaryCollector) partitionPath(partition map[int]any, sc
*iceberg.Schema, spec iceberg.PartitionSpec) string {
+ if s.partitionPathPlans == nil {
+ s.partitionPathPlans =
make(map[snapshotPartitionPathKey]snapshotPartitionPathPlan)
+ }
+
+ fields := spec.FieldsRef(internal.PartitionSpecRef{})
+ key := snapshotPartitionPathKey{schema: sc, specID: spec.ID()}
+ if len(fields) > 0 {
+ key.firstField = &fields[0]
+ }
+
+ plan, ok := s.partitionPathPlans[key]
+ if !ok {
+ partitionType := spec.PartitionType(sc)
+ pathPlan := partitionPathPlan{fields:
make([]partitionPathField, 0, len(partitionType.FieldList))}
+ for i, field := range spec.Fields() {
+ pathField := partitionPathField{
+ escapedName: field.EscapedName(),
+ transform: field.Transform,
+ resultType: partitionType.FieldList[i].Type,
+ }
+ pathPlan.fields = append(pathPlan.fields, pathField)
+ pathPlan.estimatedSize += len(pathField.escapedName) +
20
+ }
+
+ plan = snapshotPartitionPathPlan{partitionType: partitionType,
pathPlan: pathPlan}
+ s.partitionPathPlans[key] = plan
+ }
+
+ return plan.pathPlan.format(borrowedPartitionRecord{partition:
partition, partitionType: plan.partitionType})
+}
+
func (s *SnapshotSummaryCollector) setPartitionSummaryLimit(limit int) {
s.maxChangedPartitionsForSummaries = limit
}
@@ -614,8 +664,7 @@ func (s *SnapshotSummaryCollector) addFile(df
iceberg.DataFile, sc *iceberg.Sche
partition := dataFilePartition(df)
if len(partition) > 0 {
- partitionPath := spec.PartitionToPath(
- newPartitionRecord(partition, spec.PartitionType(sc)),
sc)
+ partitionPath := s.partitionPath(partition, sc, spec)
return s.updatePartitionMetrics(partitionPath, df, true)
}
@@ -630,8 +679,7 @@ func (s *SnapshotSummaryCollector) removeFile(df
iceberg.DataFile, sc *iceberg.S
partition := dataFilePartition(df)
if len(partition) > 0 {
- partitionPath := spec.PartitionToPath(
- newPartitionRecord(partition, spec.PartitionType(sc)),
sc)
+ partitionPath := s.partitionPath(partition, sc, spec)
return s.updatePartitionMetrics(partitionPath, df, false)
}
diff --git a/table/snapshots_bench_test.go b/table/snapshots_bench_test.go
index d8af4d272..42f8dd887 100644
--- a/table/snapshots_bench_test.go
+++ b/table/snapshots_bench_test.go
@@ -17,7 +17,13 @@
package table
-import "testing"
+import (
+ "fmt"
+ "strconv"
+ "testing"
+
+ "github.com/apache/iceberg-go"
+)
var partitionSummaryBenchmarkResult string
@@ -54,3 +60,95 @@ func BenchmarkPartitionSummary(b *testing.B) {
})
}
}
+
+var snapshotSummaryPathBenchmarkResult string
+
+func BenchmarkSnapshotSummaryPartitionPath(b *testing.B) {
+ for _, fieldCount := range []int{1, 3, 16, 64} {
+ fields := make([]iceberg.NestedField, fieldCount)
+ partitionFields := make([]iceberg.PartitionField, fieldCount)
+ partition := make(map[int]any, fieldCount)
+ for i := range fields {
+ name := "field_" + strconv.Itoa(i)
+ fields[i] = iceberg.NestedField{ID: i + 1, Name: name,
Type: iceberg.PrimitiveTypes.Int32}
+ partitionFields[i] = iceberg.PartitionField{
+ SourceIDs: []int{i + 1}, FieldID: 1000 + i,
Name: name,
+ Transform: iceberg.IdentityTransform{},
+ }
+ partition[1000+i] = int32(i)
+ }
+ schema := iceberg.NewSchema(0, fields...)
+ spec := iceberg.NewPartitionSpec(partitionFields...)
+
+ b.Run(fmt.Sprintf("fields=%d", fieldCount), func(b *testing.B) {
+ b.Run("baseline", func(b *testing.B) {
+ b.ReportAllocs()
+ for b.Loop() {
+ snapshotSummaryPathBenchmarkResult =
spec.PartitionToPath(
+ newPartitionRecord(partition,
spec.PartitionType(schema)), schema)
+ }
+ })
+
+ b.Run("cold", func(b *testing.B) {
+ b.ReportAllocs()
+ for b.Loop() {
+ var collector SnapshotSummaryCollector
+ snapshotSummaryPathBenchmarkResult =
collector.partitionPath(partition, schema, spec)
+ }
+ })
+
+ b.Run("cached", func(b *testing.B) {
+ var collector SnapshotSummaryCollector
+ snapshotSummaryPathBenchmarkResult =
collector.partitionPath(partition, schema, spec)
+ b.ReportAllocs()
+ b.ResetTimer()
+ for b.Loop() {
+ snapshotSummaryPathBenchmarkResult =
collector.partitionPath(partition, schema, spec)
+ }
+ })
+ })
+ }
+}
+
+func BenchmarkSnapshotSummaryPartitionPathEscaped(b *testing.B) {
+ schema := iceberg.NewSchema(1, iceberg.NestedField{
+ ID: 1, Name: "region", Type: iceberg.PrimitiveTypes.String,
+ })
+ spec := iceberg.NewPartitionSpec(iceberg.PartitionField{
+ SourceIDs: []int{1}, FieldID: 1000, Name: "region",
+ Transform: iceberg.IdentityTransform{},
+ })
+
+ for _, tc := range []struct {
+ name, value string
+ }{
+ {name: "plain", value: "berlin"},
+ {name: "escaped", value: "berlin/east +%?#"},
+ {name: "unicode", value: "München/東京"},
+ } {
+ b.Run(tc.name, func(b *testing.B) {
+ partition := map[int]any{1000: tc.value}
+ want :=
spec.PartitionToPath(newPartitionRecord(partition, spec.PartitionType(schema)),
schema)
+ var collector SnapshotSummaryCollector
+ if got := collector.partitionPath(partition, schema,
spec); got != want {
+ b.Fatalf("partition path changed: got %q, want
%q", got, want)
+ }
+
+ b.Run("baseline", func(b *testing.B) {
+ b.ReportAllocs()
+ for b.Loop() {
+ snapshotSummaryPathBenchmarkResult =
spec.PartitionToPath(
+ newPartitionRecord(partition,
spec.PartitionType(schema)), schema)
+ }
+ })
+
+ b.Run("cached", func(b *testing.B) {
+ b.ReportAllocs()
+ b.ResetTimer()
+ for b.Loop() {
+ snapshotSummaryPathBenchmarkResult =
collector.partitionPath(partition, schema, spec)
+ }
+ })
+ })
+ }
+}
diff --git a/table/snapshots_internal_test.go b/table/snapshots_internal_test.go
index edefaead6..b35d5f70a 100644
--- a/table/snapshots_internal_test.go
+++ b/table/snapshots_internal_test.go
@@ -73,6 +73,7 @@ func TestSnapshotSummaryCollectorWithPartition(t *testing.T) {
ssc.addFile(dataFile1, sc, spec)
ssc.removeFile(dataFile1, sc, spec)
ssc.removeFile(dataFile2, sc, spec)
+ require.Len(t, ssc.partitionPathPlans, 1)
assert.Equal(t, iceberg.Properties{
"added-files-size": "1234",
@@ -98,6 +99,67 @@ func TestSnapshotSummaryCollectorWithPartition(t *testing.T)
{
}, ssc.build())
}
+func TestSnapshotSummaryCollectorPartitionPathParity(t *testing.T) {
+ schema := iceberg.NewSchema(0,
+ iceberg.NestedField{ID: 1, Name: "ts", Type:
iceberg.PrimitiveTypes.TimestampTz},
+ iceberg.NestedField{ID: 2, Name: "text", Type:
iceberg.PrimitiveTypes.String},
+ iceberg.NestedField{ID: 3, Name: "integer", Type:
iceberg.PrimitiveTypes.Int32},
+ iceberg.NestedField{ID: 4, Name: "bytes", Type:
iceberg.PrimitiveTypes.Binary},
+ )
+ unknown, err := iceberg.ParseTransform("custom_transform[42]")
+ require.NoError(t, err)
+ spec := iceberg.NewPartitionSpecID(7,
+ iceberg.PartitionField{SourceIDs: []int{1}, FieldID: 1000,
Name: "ts /tz", Transform: iceberg.IdentityTransform{}},
+ iceberg.PartitionField{SourceIDs: []int{2}, FieldID: 1001,
Name: "text#part", Transform: iceberg.TruncateTransform{Width: 8}},
+ iceberg.PartitionField{SourceIDs: []int{3}, FieldID: 1002,
Name: "bucket", Transform: iceberg.BucketTransform{NumBuckets: 16}},
+ iceberg.PartitionField{SourceIDs: []int{9}, FieldID: 1003,
Name: "missing", Transform: iceberg.IdentityTransform{}},
+ iceberg.PartitionField{SourceIDs: []int{3}, FieldID: 1004,
Name: "custom", Transform: unknown},
+ iceberg.PartitionField{SourceIDs: []int{4}, FieldID: 1005,
Name: "binary", Transform: iceberg.IdentityTransform{}},
+ )
+
+ partitions := []map[int]any{
+ {1000: iceberg.Timestamp(1705314600000000), 1001: "a/b c",
1002: int32(7), 1003: nil, 1004: int32(42), 1005: []byte{0, 1, 255}},
+ {1000: iceberg.Timestamp(0), 1001: "", 1002: int32(0), 1003:
nil, 1004: nil, 1005: []byte{}},
+ }
+
+ var collector SnapshotSummaryCollector
+ for _, partition := range partitions {
+ want := spec.PartitionToPath(newPartitionRecord(partition,
spec.PartitionType(schema)), schema)
+ assert.Equal(t, want, collector.partitionPath(partition,
schema, spec))
+ }
+ require.Len(t, collector.partitionPathPlans, 1)
+
+ otherSpec := iceberg.NewPartitionSpecID(8,
+ iceberg.PartitionField{SourceIDs: []int{2}, FieldID: 1100,
Name: "other#text", Transform: iceberg.IdentityTransform{}},
+ )
+ otherPartition := map[int]any{1100: "a/b c"}
+ want := otherSpec.PartitionToPath(newPartitionRecord(otherPartition,
otherSpec.PartitionType(schema)), schema)
+ assert.Equal(t, want, collector.partitionPath(otherPartition, schema,
otherSpec))
+ require.Len(t, collector.partitionPathPlans, 2)
+
+ // Partition spec IDs are table-local. Reusing an ID must not reuse the
+ // cached plan when a different spec has been supplied.
+ sameIDSpec := iceberg.NewPartitionSpecID(spec.ID(),
+ iceberg.PartitionField{SourceIDs: []int{2}, FieldID: 1101,
Name: "same-id#text", Transform: iceberg.IdentityTransform{}},
+ )
+ sameIDPartition := map[int]any{1101: "a/b c"}
+ want = sameIDSpec.PartitionToPath(
+ newPartitionRecord(sameIDPartition,
sameIDSpec.PartitionType(schema)), schema)
+ assert.Equal(t, want, collector.partitionPath(sameIDPartition, schema,
sameIDSpec))
+ require.Len(t, collector.partitionPathPlans, 3)
+
+ // A schema change can also change how an existing spec formats values.
+ alternateSchema := iceberg.NewSchema(1,
+ iceberg.NestedField{ID: 1, Name: "ts", Type:
iceberg.PrimitiveTypes.Timestamp},
+ iceberg.NestedField{ID: 2, Name: "text", Type:
iceberg.PrimitiveTypes.String},
+ iceberg.NestedField{ID: 3, Name: "integer", Type:
iceberg.PrimitiveTypes.Int32},
+ iceberg.NestedField{ID: 4, Name: "bytes", Type:
iceberg.PrimitiveTypes.Binary},
+ )
+ want = spec.PartitionToPath(newPartitionRecord(partitions[0],
spec.PartitionType(alternateSchema)), alternateSchema)
+ assert.Equal(t, want, collector.partitionPath(partitions[0],
alternateSchema, spec))
+ require.Len(t, collector.partitionPathPlans, 4)
+}
+
func TestMergeSnapshotSummaries(t *testing.T) {
tests := []struct {
sum Summary