From 4e7f842b1c3c3d3c5ea07b0a6570ec65669c760a Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 28 Jul 2026 19:03:28 +0200 Subject: [PATCH 1/5] fix(manifest): protect DataFile metadata from external mutation Signed-off-by: Minh Vu --- manifest.go | 89 ++++++++++++++++++++++++++++++++++++------------ manifest_test.go | 75 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 22 deletions(-) diff --git a/manifest.go b/manifest.go index 532660b7b..e7ae33766 100644 --- a/manifest.go +++ b/manifest.go @@ -23,6 +23,7 @@ import ( "fmt" "io" "iter" + "maps" "math" "math/big" "reflect" @@ -2201,7 +2202,45 @@ func (d *dataFile) FileFormat() FileFormat { return d.Format } func (d *dataFile) Partition() map[int]any { d.initPartitionData() - return d.fieldIDToPartitionData + return clonePartitionMap(d.fieldIDToPartitionData) +} + +func clonePartitionMap(src map[int]any) map[int]any { + if src == nil { + return nil + } + + out := maps.Clone(src) + for id, value := range out { + if bytes, ok := value.([]byte); ok { + out[id] = slices.Clone(bytes) + } + } + + return out +} + +func cloneByteMap(src map[int][]byte) map[int][]byte { + if src == nil { + return nil + } + + out := make(map[int][]byte, len(src)) + for id, value := range src { + out[id] = slices.Clone(value) + } + + return out +} + +func clonePointer[T any](value *T) *T { + if value == nil { + return nil + } + + cloned := *value + + return &cloned } func (d *dataFile) Count() int64 { return d.RecordCount } @@ -2211,43 +2250,43 @@ func (d *dataFile) SpecID() int32 { return d.specID } func (d *dataFile) ColumnSizes() map[int]int64 { d.initColumnStatsData() - return d.colSizeMap + return maps.Clone(d.colSizeMap) } func (d *dataFile) ValueCounts() map[int]int64 { d.initColumnStatsData() - return d.valCntMap + return maps.Clone(d.valCntMap) } func (d *dataFile) NullValueCounts() map[int]int64 { d.initColumnStatsData() - return d.nullCntMap + return maps.Clone(d.nullCntMap) } func (d *dataFile) NaNValueCounts() map[int]int64 { d.initColumnStatsData() - return d.nanCntMap + return maps.Clone(d.nanCntMap) } func (d *dataFile) DistinctValueCounts() map[int]int64 { d.initColumnStatsData() - return d.distinctCntMap + return maps.Clone(d.distinctCntMap) } func (d *dataFile) LowerBoundValues() map[int][]byte { d.initColumnStatsData() - return d.lowerBoundMap + return cloneByteMap(d.lowerBoundMap) } func (d *dataFile) UpperBoundValues() map[int][]byte { d.initColumnStatsData() - return d.upperBoundMap + return cloneByteMap(d.upperBoundMap) } func (d *dataFile) KeyMetadata() []byte { @@ -2255,7 +2294,7 @@ func (d *dataFile) KeyMetadata() []byte { return nil } - return *d.Key + return slices.Clone(*d.Key) } func (d *dataFile) SplitOffsets() []int64 { @@ -2263,7 +2302,7 @@ func (d *dataFile) SplitOffsets() []int64 { return nil } - return *d.Splits + return slices.Clone(*d.Splits) } func (d *dataFile) EqualityFieldIDs() []int { @@ -2271,15 +2310,15 @@ func (d *dataFile) EqualityFieldIDs() []int { return nil } - return *d.EqualityIDs + return slices.Clone(*d.EqualityIDs) } -func (d *dataFile) SortOrderID() *int { return d.SortOrder } +func (d *dataFile) SortOrderID() *int { return clonePointer(d.SortOrder) } -func (d *dataFile) FirstRowID() *int64 { return d.FirstRowIDField } -func (d *dataFile) ReferencedDataFile() *string { return d.ReferencedDataFileField } -func (d *dataFile) ContentSizeInBytes() *int64 { return d.ContentSizeInBytesField } -func (d *dataFile) ContentOffset() *int64 { return d.ContentOffsetField } +func (d *dataFile) FirstRowID() *int64 { return clonePointer(d.FirstRowIDField) } +func (d *dataFile) ReferencedDataFile() *string { return clonePointer(d.ReferencedDataFileField) } +func (d *dataFile) ContentSizeInBytes() *int64 { return clonePointer(d.ContentSizeInBytesField) } +func (d *dataFile) ContentOffset() *int64 { return clonePointer(d.ContentOffsetField) } type ManifestEntryBuilder struct { m *manifestEntry @@ -2462,6 +2501,9 @@ func NewDataFileBuilder( fieldNameToID := make(map[string]int) for _, p := range spec.fields { if pData, ok := fieldIDToPartitionData[p.FieldID]; ok { + if bytes, ok := pData.([]byte); ok { + pData = slices.Clone(bytes) + } partitionData[p.Name] = pData fieldNameToID[p.Name] = p.FieldID } @@ -2476,7 +2518,7 @@ func NewDataFileBuilder( RecordCount: recordCount, FileSize: fileSize, specID: int32(spec.id), - fieldIDToPartitionData: fieldIDToPartitionData, + fieldIDToPartitionData: clonePartitionMap(fieldIDToPartitionData), fieldNameToID: fieldNameToID, fieldIDToLogicalType: fieldIDToLogicalType, }, @@ -2534,35 +2576,38 @@ func (b *DataFileBuilder) DistinctValueCounts(counts map[int]int64) *DataFileBui // LowerBoundValues sets the lower bound values for the data file. func (b *DataFileBuilder) LowerBoundValues(bounds map[int][]byte) *DataFileBuilder { - b.d.LowerBounds = mapToAvroColMap(bounds) + b.d.LowerBounds = mapToAvroColMap(cloneByteMap(bounds)) return b } // UpperBoundValues sets the upper bound values for the data file. func (b *DataFileBuilder) UpperBoundValues(bounds map[int][]byte) *DataFileBuilder { - b.d.UpperBounds = mapToAvroColMap(bounds) + b.d.UpperBounds = mapToAvroColMap(cloneByteMap(bounds)) return b } // KeyMetadata sets the key metadata for the data file. func (b *DataFileBuilder) KeyMetadata(key []byte) *DataFileBuilder { - b.d.Key = &key + cloned := slices.Clone(key) + b.d.Key = &cloned return b } // SplitOffsets sets the split offsets for the data file. func (b *DataFileBuilder) SplitOffsets(offsets []int64) *DataFileBuilder { - b.d.Splits = &offsets + cloned := slices.Clone(offsets) + b.d.Splits = &cloned return b } // EqualityFieldIDs sets the equality field ids for the data file. func (b *DataFileBuilder) EqualityFieldIDs(ids []int) *DataFileBuilder { - b.d.EqualityIDs = &ids + cloned := slices.Clone(ids) + b.d.EqualityIDs = &cloned return b } diff --git a/manifest_test.go b/manifest_test.go index ffb163699..bdfa58d06 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -2969,6 +2969,81 @@ func (m *ManifestTestSuite) TestManifestEntryPresentEmptyListSurvivesRewrite() { "present-empty column_sizes must survive a decode -> re-encode rewrite as a present array") } +func (m *ManifestTestSuite) TestDataFileMetadataIsIsolatedFromExternalMutation() { + partition := []byte{0x01, 0x02} + partitionData := map[int]any{1000: partition} + spec := NewPartitionSpec(PartitionField{SourceIDs: []int{1}, FieldID: 1000, Name: "part", Transform: IdentityTransform{}}) + builder, err := NewDataFileBuilder(spec, EntryContentData, "s3://bucket/file.parquet", ParquetFile, + partitionData, nil, nil, 1, 10) + m.Require().NoError(err) + + columnSizes := map[int]int64{1: 10} + valueCounts := map[int]int64{1: 2} + nullCounts := map[int]int64{1: 1} + nanCounts := map[int]int64{1: 0} + distinctCounts := map[int]int64{1: 2} + lower := map[int][]byte{1: {0x03, 0x04}} + upper := map[int][]byte{1: {0x05, 0x06}} + key := []byte{0x07, 0x08} + splits := []int64{10, 20} + equalityIDs := []int{1, 2} + + dataFile := builder. + ColumnSizes(columnSizes). + ValueCounts(valueCounts). + NullValueCounts(nullCounts). + NaNValueCounts(nanCounts). + DistinctValueCounts(distinctCounts). + LowerBoundValues(lower). + UpperBoundValues(upper). + KeyMetadata(key). + SplitOffsets(splits). + EqualityFieldIDs(equalityIDs). + SortOrderID(3). + FirstRowID(4). + ReferencedDataFile("data.parquet"). + ContentOffset(5). + ContentSizeInBytes(6). + Build() + + partition[0], lower[1][0], upper[1][0], key[0], splits[0], equalityIDs[0] = 0xff, 0xff, 0xff, 0xff, 99, 99 + columnSizes[1], valueCounts[1], nullCounts[1], nanCounts[1], distinctCounts[1] = 99, 99, 99, 99, 99 + + dataFile.Partition()[1000].([]byte)[0] = 0xff + dataFile.ColumnSizes()[1] = 99 + dataFile.ValueCounts()[1] = 99 + dataFile.NullValueCounts()[1] = 99 + dataFile.NaNValueCounts()[1] = 99 + dataFile.DistinctValueCounts()[1] = 99 + dataFile.LowerBoundValues()[1][0] = 0xff + dataFile.UpperBoundValues()[1][0] = 0xff + dataFile.KeyMetadata()[0] = 0xff + dataFile.SplitOffsets()[0] = 99 + dataFile.EqualityFieldIDs()[0] = 99 + *dataFile.SortOrderID() = 99 + *dataFile.FirstRowID() = 99 + *dataFile.ReferencedDataFile() = "changed" + *dataFile.ContentOffset() = 99 + *dataFile.ContentSizeInBytes() = 99 + + m.Equal([]byte{0x01, 0x02}, dataFile.Partition()[1000]) + m.Equal(map[int]int64{1: 10}, dataFile.ColumnSizes()) + m.Equal(map[int]int64{1: 2}, dataFile.ValueCounts()) + m.Equal(map[int]int64{1: 1}, dataFile.NullValueCounts()) + m.Equal(map[int]int64{1: 0}, dataFile.NaNValueCounts()) + m.Equal(map[int]int64{1: 2}, dataFile.DistinctValueCounts()) + m.Equal([]byte{0x03, 0x04}, dataFile.LowerBoundValues()[1]) + m.Equal([]byte{0x05, 0x06}, dataFile.UpperBoundValues()[1]) + m.Equal([]byte{0x07, 0x08}, dataFile.KeyMetadata()) + m.Equal([]int64{10, 20}, dataFile.SplitOffsets()) + m.Equal([]int{1, 2}, dataFile.EqualityFieldIDs()) + m.Equal(3, *dataFile.SortOrderID()) + m.Equal(int64(4), *dataFile.FirstRowID()) + m.Equal("data.parquet", *dataFile.ReferencedDataFile()) + m.Equal(int64(5), *dataFile.ContentOffset()) + m.Equal(int64(6), *dataFile.ContentSizeInBytes()) +} + func (m *ManifestTestSuite) TestWriteManifestListClosesWriterOnError() { // A v2 manifest list cannot reference v3 manifests because the v2 entry // schema has no first_row_id column; this gives us a deterministic From 3b006be46107472eb1b708c8dceb93e8e10a9ec5 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 28 Jul 2026 19:08:17 +0200 Subject: [PATCH 2/5] test(manifest): strengthen DataFile builder isolation Signed-off-by: Minh Vu --- manifest_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/manifest_test.go b/manifest_test.go index bdfa58d06..072532094 100644 --- a/manifest_test.go +++ b/manifest_test.go @@ -3007,6 +3007,7 @@ func (m *ManifestTestSuite) TestDataFileMetadataIsIsolatedFromExternalMutation() Build() partition[0], lower[1][0], upper[1][0], key[0], splits[0], equalityIDs[0] = 0xff, 0xff, 0xff, 0xff, 99, 99 + partitionData[1000] = []byte{0xff} columnSizes[1], valueCounts[1], nullCounts[1], nanCounts[1], distinctCounts[1] = 99, 99, 99, 99, 99 dataFile.Partition()[1000].([]byte)[0] = 0xff From c2928e97ab962820e705af9692afd2d86f7eada0 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Tue, 28 Jul 2026 22:55:51 +0200 Subject: [PATCH 3/5] perf(manifest): avoid defensive copies in internal evaluators Signed-off-by: Hoang Minh Vu --- data_file_refs.go | 35 +++++++++ internal/data_file_ref.go | 23 ++++++ table/data_file_stats_ref.go | 48 ++++++++++++ table/data_file_stats_ref_test.go | 121 ++++++++++++++++++++++++++++++ table/evaluators.go | 8 +- 5 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 data_file_refs.go create mode 100644 internal/data_file_ref.go create mode 100644 table/data_file_stats_ref.go create mode 100644 table/data_file_stats_ref_test.go diff --git a/data_file_refs.go b/data_file_refs.go new file mode 100644 index 000000000..bf6b06ce9 --- /dev/null +++ b/data_file_refs.go @@ -0,0 +1,35 @@ +// 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 + +import "github.com/apache/iceberg-go/internal" + +// DataFileStatsRef returns statistics maps owned by the data file. The token +// restricts this zero-copy accessor to trusted in-module callers; the public +// DataFile getters continue returning defensive copies. +func (d *dataFile) DataFileStatsRef(_ internal.DataFileRef) ( + valueCounts map[int]int64, + nullCounts map[int]int64, + nanCounts map[int]int64, + lowerBounds map[int][]byte, + upperBounds map[int][]byte, +) { + d.initColumnStatsData() + + return d.valCntMap, d.nullCntMap, d.nanCntMap, d.lowerBoundMap, d.upperBoundMap +} diff --git a/internal/data_file_ref.go b/internal/data_file_ref.go new file mode 100644 index 000000000..2a17de5f7 --- /dev/null +++ b/internal/data_file_ref.go @@ -0,0 +1,23 @@ +// 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 internal + +// DataFileRef authorizes zero-copy access to immutable DataFile state from +// trusted packages within this module. Go's internal-package rule prevents +// external callers from constructing this token. +type DataFileRef struct{} diff --git a/table/data_file_stats_ref.go b/table/data_file_stats_ref.go new file mode 100644 index 000000000..4b444219b --- /dev/null +++ b/table/data_file_stats_ref.go @@ -0,0 +1,48 @@ +// 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 table + +import ( + iceberg "github.com/apache/iceberg-go" + "github.com/apache/iceberg-go/internal" +) + +type dataFileStatsRefer interface { + DataFileStatsRef(internal.DataFileRef) ( + valueCounts map[int]int64, + nullCounts map[int]int64, + nanCounts map[int]int64, + lowerBounds map[int][]byte, + upperBounds map[int][]byte, + ) +} + +func dataFileStats(file iceberg.DataFile) ( + valueCounts map[int]int64, + nullCounts map[int]int64, + nanCounts map[int]int64, + lowerBounds map[int][]byte, + upperBounds map[int][]byte, +) { + if ref, ok := file.(dataFileStatsRefer); ok { + return ref.DataFileStatsRef(internal.DataFileRef{}) + } + + return file.ValueCounts(), file.NullValueCounts(), file.NaNValueCounts(), + file.LowerBoundValues(), file.UpperBoundValues() +} diff --git a/table/data_file_stats_ref_test.go b/table/data_file_stats_ref_test.go new file mode 100644 index 000000000..f143ef2ab --- /dev/null +++ b/table/data_file_stats_ref_test.go @@ -0,0 +1,121 @@ +// 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 table + +import ( + "testing" + + iceberg "github.com/apache/iceberg-go" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type publicStatsDataFile struct { + iceberg.DataFile + getterCalls int +} + +func (f *publicStatsDataFile) ValueCounts() map[int]int64 { + f.getterCalls++ + + return f.DataFile.ValueCounts() +} + +func (f *publicStatsDataFile) NullValueCounts() map[int]int64 { + f.getterCalls++ + + return f.DataFile.NullValueCounts() +} + +func (f *publicStatsDataFile) NaNValueCounts() map[int]int64 { + f.getterCalls++ + + return f.DataFile.NaNValueCounts() +} + +func (f *publicStatsDataFile) LowerBoundValues() map[int][]byte { + f.getterCalls++ + + return f.DataFile.LowerBoundValues() +} + +func (f *publicStatsDataFile) UpperBoundValues() map[int][]byte { + f.getterCalls++ + + return f.DataFile.UpperBoundValues() +} + +func testDataFileWithStats(t *testing.T) iceberg.DataFile { + t.Helper() + + spec := iceberg.NewPartitionSpec(iceberg.PartitionField{ + SourceIDs: []int{1}, + FieldID: 1000, + Name: "part", + Transform: iceberg.IdentityTransform{}, + }) + builder, err := iceberg.NewDataFileBuilder( + spec, + iceberg.EntryContentData, + "s3://bucket/file.parquet", + iceberg.ParquetFile, + map[int]any{1000: "partition"}, + nil, + nil, + 2, + 10, + ) + require.NoError(t, err) + + return builder. + ValueCounts(map[int]int64{1: 2}). + NullValueCounts(map[int]int64{1: 0}). + NaNValueCounts(map[int]int64{1: 0}). + LowerBoundValues(map[int][]byte{1: {1, 2}}). + UpperBoundValues(map[int][]byte{1: {3, 4}}). + Build() +} + +func TestDataFileStatsUsesBorrowedView(t *testing.T) { + file := testDataFileWithStats(t) + require.Implements(t, (*dataFileStatsRefer)(nil), file) + + valueCounts, nullCounts, nanCounts, lowerBounds, upperBounds := dataFileStats(file) + assert.Equal(t, map[int]int64{1: 2}, valueCounts) + assert.Equal(t, map[int]int64{1: 0}, nullCounts) + assert.Equal(t, map[int]int64{1: 0}, nanCounts) + assert.Equal(t, map[int][]byte{1: {1, 2}}, lowerBounds) + assert.Equal(t, map[int][]byte{1: {3, 4}}, upperBounds) + + assert.Zero(t, testing.AllocsPerRun(100, func() { + dataFileStats(file) + })) +} + +func TestDataFileStatsFallsBackToPublicGetters(t *testing.T) { + file := &publicStatsDataFile{DataFile: testDataFileWithStats(t)} + + valueCounts, nullCounts, nanCounts, lowerBounds, upperBounds := dataFileStats(file) + assert.Equal(t, map[int]int64{1: 2}, valueCounts) + assert.Equal(t, map[int]int64{1: 0}, nullCounts) + assert.Equal(t, map[int]int64{1: 0}, nanCounts) + assert.Equal(t, map[int][]byte{1: {1, 2}}, lowerBounds) + assert.Equal(t, map[int][]byte{1: {3, 4}}, upperBounds) + assert.Equal(t, 5, file.getterCalls) +} diff --git a/table/evaluators.go b/table/evaluators.go index e914dde83..347838977 100644 --- a/table/evaluators.go +++ b/table/evaluators.go @@ -774,9 +774,7 @@ func (m *inclusiveMetricsEval) Eval(file iceberg.DataFile) (bool, error) { expr: m.expr, } - ev.valueCounts, ev.nullCounts = file.ValueCounts(), file.NullValueCounts() - ev.nanCounts = file.NaNValueCounts() - ev.lowerBounds, ev.upperBounds = file.LowerBoundValues(), file.UpperBoundValues() + ev.valueCounts, ev.nullCounts, ev.nanCounts, ev.lowerBounds, ev.upperBounds = dataFileStats(file) return iceberg.VisitExpr(m.expr, &ev) } @@ -1245,9 +1243,7 @@ func (m *strictMetricsEval) Eval(file iceberg.DataFile) (bool, error) { expr: m.expr, } - ev.valueCounts, ev.nullCounts = file.ValueCounts(), file.NullValueCounts() - ev.nanCounts = file.NaNValueCounts() - ev.lowerBounds, ev.upperBounds = file.LowerBoundValues(), file.UpperBoundValues() + ev.valueCounts, ev.nullCounts, ev.nanCounts, ev.lowerBounds, ev.upperBounds = dataFileStats(file) return iceberg.VisitExpr(m.expr, &ev) } From ecbe15bbee330e0d6c725229f259b3d3b662f955 Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Thu, 30 Jul 2026 16:13:38 +0200 Subject: [PATCH 4/5] perf(manifest): benchmark borrowed file statistics --- data_file_refs.go | 3 +- manifest.go | 3 ++ table/data_file_stats_ref_test.go | 46 +++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/data_file_refs.go b/data_file_refs.go index bf6b06ce9..9e652e548 100644 --- a/data_file_refs.go +++ b/data_file_refs.go @@ -21,7 +21,8 @@ import "github.com/apache/iceberg-go/internal" // DataFileStatsRef returns statistics maps owned by the data file. The token // restricts this zero-copy accessor to trusted in-module callers; the public -// DataFile getters continue returning defensive copies. +// DataFile getters continue returning defensive copies. Callers must treat the +// returned maps and the byte slices stored in the bounds maps as read-only. func (d *dataFile) DataFileStatsRef(_ internal.DataFileRef) ( valueCounts map[int]int64, nullCounts map[int]int64, diff --git a/manifest.go b/manifest.go index e7ae33766..418db206b 100644 --- a/manifest.go +++ b/manifest.go @@ -2453,6 +2453,9 @@ type DataFileBuilder struct { // before calling [DataFileBuilder.Build] to construct the object. // The fieldIDToFixedSize argument is retained for source compatibility and is // ignored; manifest schemas determine decimal fixed widths during encoding. +// Byte-slice partition values are cloned. Other partition values must be the +// immutable scalar or literal values produced by Iceberg's manifest decoder; +// mutable caller-defined values are retained by reference. func NewDataFileBuilder( spec PartitionSpec, content ManifestEntryContent, diff --git a/table/data_file_stats_ref_test.go b/table/data_file_stats_ref_test.go index f143ef2ab..d3f42bba7 100644 --- a/table/data_file_stats_ref_test.go +++ b/table/data_file_stats_ref_test.go @@ -119,3 +119,49 @@ func TestDataFileStatsFallsBackToPublicGetters(t *testing.T) { assert.Equal(t, map[int][]byte{1: {3, 4}}, upperBounds) assert.Equal(t, 5, file.getterCalls) } + +func BenchmarkInclusiveMetricsEvalDataFileStats(b *testing.B) { + lower, err := iceberg.Int64Literal(1).MarshalBinary() + require.NoError(b, err) + upper, err := iceberg.Int64Literal(10).MarshalBinary() + require.NoError(b, err) + + spec := iceberg.NewPartitionSpec(iceberg.PartitionField{ + SourceIDs: []int{1}, FieldID: 1000, Name: "part", Transform: iceberg.IdentityTransform{}, + }) + builder, err := iceberg.NewDataFileBuilder(spec, iceberg.EntryContentData, + "s3://bucket/file.parquet", iceberg.ParquetFile, map[int]any{1000: int64(1)}, nil, nil, 10, 100) + require.NoError(b, err) + file := builder. + ValueCounts(map[int]int64{1: 10}). + NullValueCounts(map[int]int64{1: 0}). + LowerBoundValues(map[int][]byte{1: lower}). + UpperBoundValues(map[int][]byte{1: upper}). + Build() + schema := iceberg.NewSchema(1, iceberg.NestedField{ + ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int64, + }) + eval, err := newInclusiveMetricsEvaluator(schema, + iceberg.LessThan(iceberg.Reference("id"), int64(20)), true, true) + require.NoError(b, err) + + b.Run("borrowed stats", func(b *testing.B) { + b.ReportAllocs() + for range b.N { + _, err := eval(file) + if err != nil { + b.Fatal(err) + } + } + }) + b.Run("public defensive copies", func(b *testing.B) { + wrapped := &publicStatsDataFile{DataFile: file} + b.ReportAllocs() + for range b.N { + _, err := eval(wrapped) + if err != nil { + b.Fatal(err) + } + } + }) +} From 3ee1c2b3cdc6f199fb7895c2634d95fba69360fc Mon Sep 17 00:00:00 2001 From: Minh Vu Date: Sun, 2 Aug 2026 01:19:15 +0200 Subject: [PATCH 5/5] perf(manifest): avoid duplicate partition value clone --- manifest.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/manifest.go b/manifest.go index 418db206b..2372f1cfd 100644 --- a/manifest.go +++ b/manifest.go @@ -2504,9 +2504,6 @@ func NewDataFileBuilder( fieldNameToID := make(map[string]int) for _, p := range spec.fields { if pData, ok := fieldIDToPartitionData[p.FieldID]; ok { - if bytes, ok := pData.([]byte); ok { - pData = slices.Clone(bytes) - } partitionData[p.Name] = pData fieldNameToID[p.Name] = p.FieldID }