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 8f8b994aa fix(table): snapshot summary counts and sizes for deletion
vectors (#2352)
8f8b994aa is described below
commit 8f8b994aadeca0299aec7ad8343b0812987660c8
Author: Akriti <[email protected]>
AuthorDate: Wed Nov 4 21:36:45 2026 +0530
fix(table): snapshot summary counts and sizes for deletion vectors (#2352)
* dev
* test(table): address review feedback on DV summary tests
* fix: calculate dv sizes
* docs: add compatibility note for DV snapshot summaries
* docs: refine DV compatibility note
---
table/commit_metrics.go | 8 ++++---
table/commit_metrics_test.go | 10 +++++---
table/dv_rewrite_test.go | 3 ++-
table/row_delta_test.go | 4 ++++
table/snapshots.go | 32 ++++++++++++++++++++++----
table/snapshots_internal_test.go | 49 ++++++++++++++++++++++++++++++++++++++++
website/src/releases.md | 1 +
7 files changed, 96 insertions(+), 11 deletions(-)
diff --git a/table/commit_metrics.go b/table/commit_metrics.go
index 2db0b74c8..0cd1abd6a 100644
--- a/table/commit_metrics.go
+++ b/table/commit_metrics.go
@@ -81,9 +81,8 @@ func summaryCounter(props iceberg.Properties, key string,
unit metrics.Unit) *me
// snapshotSummary): attempts and total-duration come from the commit itself,
// and every other metric is read from the snapshot summary under its spec key
// and emitted under Java's commit-report field name so dashboards line up
-// across implementations. Metrics whose summary key iceberg-go does not yet
-// populate (DVs) are absent and therefore omitted — exactly as Java's
-// counterFrom omits summary keys it does not find.
+// across implementations. A metric whose summary key is absent is omitted,
+// exactly as Java's counterFrom omits summary keys it does not find.
func buildCommitReport(tableName string, snap *Snapshot, attempts int64, dur
time.Duration) metrics.CommitReport {
var (
snapshotID int64
@@ -127,6 +126,9 @@ func buildCommitReport(tableName string, snap *Snapshot,
attempts int64, dur tim
AddedPositionalDeleteFiles:
count(addedPosDeleteFilesKey),
RemovedPositionalDeleteFiles:
count(removedPosDeleteFilesKey),
+ AddedDVs: count(addedDVsKey),
+ RemovedDVs: count(removedDVsKey),
+
AddedRecords: count(addedRecordsKey),
RemovedRecords: count(deletedRecordsKey),
TotalRecords: count(totalRecordsKey),
diff --git a/table/commit_metrics_test.go b/table/commit_metrics_test.go
index d298a2fdb..c283774c3 100644
--- a/table/commit_metrics_test.go
+++ b/table/commit_metrics_test.go
@@ -59,6 +59,8 @@ func TestBuildCommitReport(t *testing.T) {
totalFileSizeKey: "5000000",
addedPosDeletesKey: "2",
addedPosDeleteFilesKey: "1",
+ addedDVsKey: "7",
+ removedDVsKey: "8",
addedEqDeletesKey: "3",
addedEqDeleteFilesKey: "1",
manifestsCreatedKey: "6",
@@ -91,6 +93,10 @@ func TestBuildCommitReport(t *testing.T) {
assert.Equal(t, int64(12345), m.AddedRecords.Value)
require.NotNil(t, m.TotalRecords)
assert.Equal(t, int64(20000), m.TotalRecords.Value)
+ require.NotNil(t, m.AddedDVs)
+ assert.Equal(t, int64(7), m.AddedDVs.Value)
+ require.NotNil(t, m.RemovedDVs)
+ assert.Equal(t, int64(8), m.RemovedDVs.Value)
// Name-translated mappings (iceberg-go summary key -> Java report
name).
require.NotNil(t, m.RemovedDataFiles, "deleted-data-files ->
removed-data-files")
@@ -123,10 +129,8 @@ func TestBuildCommitReport(t *testing.T) {
require.NotNil(t, m.ManifestEntriesProcessed, "entries-processed ->
manifest-entries-processed")
assert.Equal(t, int64(9), m.ManifestEntriesProcessed.Value)
- // Keys absent from the summary are omitted, not zeroed; metrics with no
- // iceberg-go summary key (DVs) stay unset.
+ // Keys absent from the summary are omitted, not zeroed.
assert.Nil(t, m.RemovedDeleteFiles)
- assert.Nil(t, m.AddedDVs)
}
func TestBuildCommitReportIncludesEnvironmentContext(t *testing.T) {
diff --git a/table/dv_rewrite_test.go b/table/dv_rewrite_test.go
index dbae9dc8c..ed6cb806e 100644
--- a/table/dv_rewrite_test.go
+++ b/table/dv_rewrite_test.go
@@ -586,7 +586,8 @@ func TestRemovingDataFileRemovesItsUnnamedDV(t *testing.T) {
snap := tbl.CurrentSnapshot()
live, removed := snapshotDeleteEntryFiles(t,
snap, iceio.LocalFS{})
assert.Contains(t, pathRefPairs(removed),
[2]string{dvB.FilePath(), dataB.FilePath()})
- assert.Equal(t, "1",
snap.Summary.Properties["removed-position-delete-files"])
+ assert.Equal(t, "1",
snap.Summary.Properties["removed-dvs"])
+ assert.NotContains(t, snap.Summary.Properties,
"removed-position-delete-files")
assert.Equal(t, strconv.Itoa(len(live)),
snap.Summary.Properties["total-delete-files"])
})
}
diff --git a/table/row_delta_test.go b/table/row_delta_test.go
index e17c877df..f0e23a033 100644
--- a/table/row_delta_test.go
+++ b/table/row_delta_test.go
@@ -890,6 +890,10 @@ func TestRowDeltaDVSupersessionSingleSnapshot(t
*testing.T) {
assert.Equal(t, table.OpDelete, snap.Summary.Operation)
assert.Equal(t, "1", snap.Summary.Properties["added-delete-files"])
assert.Equal(t, "1", snap.Summary.Properties["removed-delete-files"])
+ assert.Equal(t, "1", snap.Summary.Properties["added-dvs"])
+ assert.Equal(t, "1", snap.Summary.Properties["removed-dvs"])
+ assert.NotContains(t, snap.Summary.Properties,
"added-position-delete-files")
+ assert.NotContains(t, snap.Summary.Properties,
"removed-position-delete-files")
fs := iceio.LocalFS{}
liveDVs, removedDVs := snapshotDVEntries(t, snap, fs)
diff --git a/table/snapshots.go b/table/snapshots.go
index 7e49aaad3..eb10667cf 100644
--- a/table/snapshots.go
+++ b/table/snapshots.go
@@ -70,6 +70,7 @@ const (
addedPosDeleteFilesKey = "added-position-delete-files"
addedRecordsKey = "added-records"
addedEqDeleteFilesKey = "added-equality-delete-files"
+ addedDVsKey = "added-dvs"
deletedDataFilesKey = "deleted-data-files"
deletedRecordsKey = "deleted-records"
removedDeleteFilesKey = "removed-delete-files"
@@ -78,6 +79,7 @@ const (
removedFileSizeKey = "removed-files-size"
removedPosDeletesKey = "removed-position-deletes"
removedPosDeleteFilesKey = "removed-position-delete-files"
+ removedDVsKey = "removed-dvs"
totalEqDeletesKey = "total-equality-deletes"
totalPosDeletesKey = "total-position-deletes"
totalDataFilesKey = "total-data-files"
@@ -97,6 +99,8 @@ type updateMetrics struct {
removedEqDeleteFiles int64
addedPosDeleteFiles int64
removedPosDeleteFiles int64
+ addedDVs int64
+ removedDVs int64
addedDeleteFiles int64
removedDeleteFiles int64
addedRecords int64
@@ -107,15 +111,29 @@ type updateMetrics struct {
removedEqDeletes int64
}
+// contentSizeInBytes returns the bytes a file contributes to the table. A DV
+// shares its Puffin file with other DVs, so it counts only its own blob.
+func contentSizeInBytes(df iceberg.DataFile) int64 {
+ if IsDeletionVector(df) && df.ContentSizeInBytes() != nil {
+ return *df.ContentSizeInBytes()
+ }
+
+ return df.FileSizeBytes()
+}
+
func (m *updateMetrics) addDataFile(df iceberg.DataFile) error {
- m.addedFileSize += df.FileSizeBytes()
+ m.addedFileSize += contentSizeInBytes(df)
switch df.ContentType() {
case iceberg.EntryContentData:
m.addedDataFiles++
m.addedRecords += df.Count()
case iceberg.EntryContentPosDeletes:
m.addedDeleteFiles++
- m.addedPosDeleteFiles++
+ if IsDeletionVector(df) {
+ m.addedDVs++
+ } else {
+ m.addedPosDeleteFiles++
+ }
m.addedPosDeletes += df.Count()
case iceberg.EntryContentEqDeletes:
m.addedDeleteFiles++
@@ -129,14 +147,18 @@ func (m *updateMetrics) addDataFile(df iceberg.DataFile)
error {
}
func (m *updateMetrics) removeFile(df iceberg.DataFile) error {
- m.removedFileSize += df.FileSizeBytes()
+ m.removedFileSize += contentSizeInBytes(df)
switch df.ContentType() {
case iceberg.EntryContentData:
m.removedDataFiles++
m.deletedRecords += df.Count()
case iceberg.EntryContentPosDeletes:
m.removedDeleteFiles++
- m.removedPosDeleteFiles++
+ if IsDeletionVector(df) {
+ m.removedDVs++
+ } else {
+ m.removedPosDeleteFiles++
+ }
m.removedPosDeletes += df.Count()
case iceberg.EntryContentEqDeletes:
m.removedDeleteFiles++
@@ -165,6 +187,8 @@ func (m *updateMetrics) toProps() iceberg.Properties {
setWhenPositive(props, removedEqDeleteFilesKey, m.removedEqDeleteFiles)
setWhenPositive(props, addedPosDeleteFilesKey, m.addedPosDeleteFiles)
setWhenPositive(props, removedPosDeleteFilesKey,
m.removedPosDeleteFiles)
+ setWhenPositive(props, addedDVsKey, m.addedDVs)
+ setWhenPositive(props, removedDVsKey, m.removedDVs)
setWhenPositive(props, addedDeleteFilesKey, m.addedDeleteFiles)
setWhenPositive(props, removedDeleteFilesKey, m.removedDeleteFiles)
setWhenPositive(props, addedRecordsKey, m.addedRecords)
diff --git a/table/snapshots_internal_test.go b/table/snapshots_internal_test.go
index b35d5f70a..67cf50b1a 100644
--- a/table/snapshots_internal_test.go
+++ b/table/snapshots_internal_test.go
@@ -294,3 +294,52 @@ func TestUpdateSnapshotSummariesPreservesLargeTotals(t
*testing.T) {
assert.Equal(t, "4000000000", result.Properties[totalRecordsKey])
assert.Equal(t, "9000000000", result.Properties[totalFileSizeKey])
}
+
+func TestSnapshotSummaryCollectorDeletionVectors(t *testing.T) {
+ var ssc SnapshotSummaryCollector
+ spec := *iceberg.UnpartitionedSpec
+
+ // Two DVs that share one Puffin file, as DVWriter writes them.
+ newDV := func(ref string, count, offset, size int64) iceberg.DataFile {
+ return must(iceberg.NewDataFileBuilder(spec,
iceberg.EntryContentPosDeletes,
+ "/path/to/batch.puffin", iceberg.PuffinFile, nil, nil,
nil, count, 1000)).
+ ReferencedDataFile(ref).
+ ContentOffset(offset).
+ ContentSizeInBytes(size).
+ Build()
+ }
+ dvA := newDV("/path/to/data-a.parquet", 2, 4, 300)
+ dvB := newDV("/path/to/data-b.parquet", 4, 304, 400)
+
+ oldDV := must(iceberg.NewDataFileBuilder(spec,
iceberg.EntryContentPosDeletes,
+ "/path/to/old.puffin", iceberg.PuffinFile, nil, nil, nil, 1,
600)).
+ ReferencedDataFile("/path/to/data-a.parquet").
+ ContentOffset(4).
+ ContentSizeInBytes(250).
+ Build()
+
+ posDelete := must(iceberg.NewDataFileBuilder(spec,
iceberg.EntryContentPosDeletes,
+ "/path/to/pos-delete.parquet", iceberg.ParquetFile, nil, nil,
nil, 3, 800)).Build()
+
+ require.NoError(t, ssc.addFile(dvA, tableSchemaSimple, spec))
+ require.NoError(t, ssc.addFile(dvB, tableSchemaSimple, spec))
+ require.NoError(t, ssc.addFile(posDelete, tableSchemaSimple, spec))
+ require.NoError(t, ssc.removeFile(oldDV, tableSchemaSimple, spec))
+
+ props := ssc.build()
+
+ // DVs are counted on their own...
+ assert.Equal(t, "2", props["added-dvs"])
+ assert.Equal(t, "1", props["removed-dvs"])
+ // ...and are not counted as position delete files.
+ assert.Equal(t, "1", props["added-position-delete-files"])
+ assert.NotContains(t, props, "removed-position-delete-files")
+ // DVs still count as delete files and as position deletes.
+ assert.Equal(t, "3", props["added-delete-files"])
+ assert.Equal(t, "1", props["removed-delete-files"])
+ assert.Equal(t, "9", props["added-position-deletes"]) // 2 + 4 + 3
+ assert.Equal(t, "1", props["removed-position-deletes"])
+ // A DV counts only its own blob, not the whole shared Puffin file.
+ assert.Equal(t, "1500", props["added-files-size"]) // 300 + 400 + 800
+ assert.Equal(t, "250", props["removed-files-size"])
+}
diff --git a/website/src/releases.md b/website/src/releases.md
index 0dc5a7c51..75490333d 100644
--- a/website/src/releases.md
+++ b/website/src/releases.md
@@ -92,6 +92,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
- **Data-file replacement methods renamed.** Use `ReplaceDataFilesByPath` for
string paths. Use `ReplaceDataFiles` for prebuilt `DataFile` values instead of
`ReplaceDataFilesWithDataFiles`.
- `maintenance.PurgeFiles` (previously `Table.PurgeFiles`) now calls
`IO.Remove` concurrently, with up to 32 calls in flight, when the filesystem
does not implement `BulkRemovableIO`. Custom IO implementations, including
those registered with `io.Register`, must make `Remove` safe for concurrent use
and synchronize any shared mutable state. The bulk deletion path is unchanged.
+- **Snapshot summaries report deletion vectors separately.** On v3 tables, DVs
are counted under `added-dvs` and `removed-dvs` instead of
`added-position-delete-files` and `removed-position-delete-files`.
`added-files-size` and `removed-files-size` use each DV's
`content_size_in_bytes` instead of its whole Puffin file, so they go down for
commits with DVs. `total-files-size` is updated from these values. Partition
summaries, commit reports and OTel metrics change the same way. Existing s [...]
- **Partition and sort source accessors renamed.** Replace
`PartitionField.SourceID()` and `SortField.SourceID()` with `FirstSourceID()`.
Their behavior and serialized fields are unchanged.
