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 1fdb33f9c refactor(maintenance)!: remove duplicate orphan paths from
cleanup result (#2282)
1fdb33f9c is described below
commit 1fdb33f9c1b50a64a7a4561889fc77b14f8d2de8
Author: Minh Vu <[email protected]>
AuthorDate: Sat Oct 24 00:42:07 2026 +0200
refactor(maintenance)!: remove duplicate orphan paths from cleanup result
(#2282)
Signed-off-by: Minh Vu <[email protected]>
Co-authored-by: Matt Topol <[email protected]>
---
cmd/iceberg/clean_orphan_files.go | 5 ++---
cmd/iceberg/clean_orphan_files_test.go | 6 ------
table/maintenance/orphan_cleanup.go | 18 +++++++-----------
table/maintenance/orphan_cleanup_integration_test.go | 8 ++++----
table/maintenance/orphan_cleanup_test.go | 11 ++++++-----
website/src/releases.md | 1 +
6 files changed, 20 insertions(+), 29 deletions(-)
diff --git a/cmd/iceberg/clean_orphan_files.go
b/cmd/iceberg/clean_orphan_files.go
index a7fd6e483..b2b28a270 100644
--- a/cmd/iceberg/clean_orphan_files.go
+++ b/cmd/iceberg/clean_orphan_files.go
@@ -55,9 +55,8 @@ func runCleanOrphanFiles(ctx context.Context, output Output,
cat catalog.Catalog
planFiles := plan.Files()
result := maintenance.OrphanCleanupResult{
- OrphanFileLocations: planFiles,
- OrphanFiles: plan.OrphanFiles(),
- TotalSizeBytes: plan.TotalSizeBytes(),
+ OrphanFiles: plan.OrphanFiles(),
+ TotalSizeBytes: plan.TotalSizeBytes(),
}
cliResult := buildCleanOrphanFilesResult(tbl, result, cmd.DryRun)
diff --git a/cmd/iceberg/clean_orphan_files_test.go
b/cmd/iceberg/clean_orphan_files_test.go
index f7ad8b5fb..ea1aecc22 100644
--- a/cmd/iceberg/clean_orphan_files_test.go
+++ b/cmd/iceberg/clean_orphan_files_test.go
@@ -129,7 +129,6 @@ func TestBuildCleanOrphanFilesResultDryRun(t *testing.T) {
tbl := table.New([]string{"db", "orphans"}, meta, "", nil, nil)
orphanResult := maintenance.OrphanCleanupResult{
- OrphanFileLocations: []string{"s3://bucket/data/file1.parquet",
"s3://bucket/data/file2.parquet"},
OrphanFiles: []maintenance.OrphanFile{
{Path: "s3://bucket/data/file1.parquet", SizeBytes:
1024},
{Path: "s3://bucket/data/file2.parquet", SizeBytes:
2048},
@@ -177,7 +176,6 @@ func TestBuildCleanOrphanFilesResultDeleted(t *testing.T) {
tbl := table.New([]string{"db", "orphans"}, meta, "", nil, nil)
orphanResult := maintenance.OrphanCleanupResult{
- OrphanFileLocations: []string{"s3://bucket/data/file1.parquet"},
OrphanFiles: []maintenance.OrphanFile{
{Path: "s3://bucket/data/file1.parquet", SizeBytes:
2048},
},
@@ -222,10 +220,6 @@ func
TestBuildCleanOrphanFilesResultDeletedFiltersToDeletedFiles(t *testing.T) {
tbl := table.New([]string{"db", "orphans"}, meta, "", nil, nil)
orphanResult := maintenance.OrphanCleanupResult{
- OrphanFileLocations: []string{
- "s3://bucket/data/file1.parquet",
- "s3://bucket/data/file2.parquet",
- },
OrphanFiles: []maintenance.OrphanFile{
{Path: "s3://bucket/data/file1.parquet", SizeBytes:
1024},
{Path: "s3://bucket/data/file2.parquet", SizeBytes:
2048},
diff --git a/table/maintenance/orphan_cleanup.go
b/table/maintenance/orphan_cleanup.go
index b4a964b95..e56c58219 100644
--- a/table/maintenance/orphan_cleanup.go
+++ b/table/maintenance/orphan_cleanup.go
@@ -227,11 +227,9 @@ func flattenURIEquivalences(equivalences
map[string]string) map[string]string {
return flattened
}
+// OrphanCleanupResult reports the files found and deleted by orphan cleanup.
type OrphanCleanupResult struct {
- // OrphanFileLocations is retained for backward compatibility with
callers
- // that consume only orphan paths. Prefer OrphanFiles for canonical
path+size data.
- OrphanFileLocations []string
- // OrphanFiles is the canonical richer orphan result, carrying both
path and size.
+ // OrphanFiles includes the path and size of each identified orphan.
OrphanFiles []OrphanFile
DeletedFiles []string
// TotalSizeBytes is the combined size of orphan files only, not all
scanned files.
@@ -276,9 +274,8 @@ func (p OrphanCleanupPlan) Cutoff() time.Time {
func (p OrphanCleanupPlan) result() OrphanCleanupResult {
return OrphanCleanupResult{
- OrphanFileLocations: p.Files(),
- OrphanFiles: p.OrphanFiles(),
- TotalSizeBytes: p.totalSizeBytes,
+ OrphanFiles: p.OrphanFiles(),
+ TotalSizeBytes: p.totalSizeBytes,
}
}
@@ -483,10 +480,9 @@ func executeOrphanCleanup(ctx context.Context, tbl
*table.Table, plan OrphanClea
}
return OrphanCleanupResult{
- OrphanFileLocations: orphanFiles,
- OrphanFiles: plan.OrphanFiles(),
- DeletedFiles: deletedFiles,
- TotalSizeBytes: plan.totalSizeBytes,
+ OrphanFiles: plan.OrphanFiles(),
+ DeletedFiles: deletedFiles,
+ TotalSizeBytes: plan.totalSizeBytes,
}, nil
}
diff --git a/table/maintenance/orphan_cleanup_integration_test.go
b/table/maintenance/orphan_cleanup_integration_test.go
index b72358461..268adcdda 100644
--- a/table/maintenance/orphan_cleanup_integration_test.go
+++ b/table/maintenance/orphan_cleanup_integration_test.go
@@ -230,7 +230,7 @@ func (s *OrphanCleanupIntegrationSuite)
TestOrphanCleanupDryRun() {
s.Require().NoError(err)
s.Require().NotNil(result)
- s.GreaterOrEqual(len(result.OrphanFileLocations), 3, "Should identify
at least 3 orphan files")
+ s.GreaterOrEqual(len(result.OrphanFiles), 3, "Should identify at least
3 orphan files")
s.Empty(result.DeletedFiles, "Dry run should not delete any files")
for _, orphanFile := range orphanFiles {
@@ -239,8 +239,8 @@ func (s *OrphanCleanupIntegrationSuite)
TestOrphanCleanupDryRun() {
for _, orphanFile := range orphanFiles {
found := false
- for _, identified := range result.OrphanFileLocations {
- if strings.Contains(identified,
filepath.Base(orphanFile)) {
+ for _, identified := range result.OrphanFiles {
+ if strings.Contains(identified.Path,
filepath.Base(orphanFile)) {
found = true
break
}
@@ -277,7 +277,7 @@ func (s *OrphanCleanupIntegrationSuite)
TestOrphanCleanupActualDeletion() {
s.Require().NoError(err)
s.Require().NotNil(result)
- s.GreaterOrEqual(len(result.OrphanFileLocations), 2, "Should identify
at least 2 orphan files")
+ s.GreaterOrEqual(len(result.OrphanFiles), 2, "Should identify at least
2 orphan files")
s.GreaterOrEqual(len(result.DeletedFiles), 2, "Should delete at least 2
orphan files")
for _, orphanFile := range orphanFiles {
diff --git a/table/maintenance/orphan_cleanup_test.go
b/table/maintenance/orphan_cleanup_test.go
index 6709ad95d..941036f2e 100644
--- a/table/maintenance/orphan_cleanup_test.go
+++ b/table/maintenance/orphan_cleanup_test.go
@@ -2028,11 +2028,12 @@ func TestDeleteOrphanFilesPrefixMismatchModes(t
*testing.T) {
assert.Empty(t, deleted)
case PrefixMismatchIgnore:
require.NoError(t, err)
- assert.Empty(t,
result.OrphanFileLocations)
+ assert.Empty(t,
result.OrphanFiles)
assert.Empty(t, deleted)
case PrefixMismatchDelete:
require.NoError(t, err)
- assert.Equal(t,
[]string{tt.listedPath}, result.OrphanFileLocations)
+ require.Len(t,
result.OrphanFiles, 1)
+ assert.Equal(t, tt.listedPath,
result.OrphanFiles[0].Path)
assert.Equal(t,
[]string{tt.listedPath}, deleted)
}
})
@@ -2086,7 +2087,7 @@ func
TestDeleteOrphanFilesDryRunKeepsMixedCaseWindowsReference(t *testing.T) {
WithCleanupDryRun(true),
)
require.NoError(t, err)
- assert.Empty(t, result.OrphanFileLocations)
+ assert.Empty(t, result.OrphanFiles)
assert.Empty(t, result.DeletedFiles)
}
@@ -2161,7 +2162,7 @@ func
TestDeleteOrphanFilesDryRunKeepsPortableWindowsReferences(t *testing.T) {
WithCleanupDryRun(true),
)
require.NoError(t, err)
- assert.Empty(t, result.OrphanFileLocations)
+ assert.Empty(t, result.OrphanFiles)
assert.Empty(t, result.DeletedFiles)
})
}
@@ -2204,7 +2205,7 @@ func TestDeleteOrphanFilesDryRunKeepsOpaqueVersionHint(t
*testing.T) {
)
require.NoError(t, err)
assert.Equal(t, tableLocation, fsys.root)
- assert.Empty(t, result.OrphanFileLocations)
+ assert.Empty(t, result.OrphanFiles)
assert.Empty(t, result.DeletedFiles)
}
diff --git a/website/src/releases.md b/website/src/releases.md
index 3c81ebc4d..435ec5148 100644
--- a/website/src/releases.md
+++ b/website/src/releases.md
@@ -42,6 +42,7 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
### Unreleased compatibility notes
+- **Orphan cleanup results now expose only `OrphanFiles`.** Replace
`result.OrphanFileLocations` with `result.OrphanFiles` (each entry has `Path`
and `SizeBytes`). `OrphanCleanupPlan.Files()` still provides path-only lists
during planning.
- **Catalog identifier helper renamed.** `catalog.TableNameFromIdent` is
removed. Use `catalog.ObjectNameFromIdent` for tables, views, and functions.
- **Substrait expression conversion no longer takes case sensitivity.**
`substrait.ConvertExpr(schema, expression, caseSensitive)` becomes
`substrait.ConvertExpr(schema, expression)`. Case sensitivity remains part of
expression binding.
