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 c3eda1d41 refactor(puffin)!: rename Writer.AddProperties to
SetProperties before v1 (#2273)
c3eda1d41 is described below
commit c3eda1d41d59b617e2e528ad0e8136bf177665b3
Author: Minh Vu <[email protected]>
AuthorDate: Fri Oct 23 21:44:56 2026 +0200
refactor(puffin)!: rename Writer.AddProperties to SetProperties before v1
(#2273)
refactor(puffin)!: rename Writer.AddProperties before v1
Align the Puffin writer property API with its GoDoc and other metadata
setters.
Preserve merge semantics and cover repeated property updates.
Signed-off-by: Minh Vu <[email protected]>
Co-authored-by: Matt Topol <[email protected]>
---
puffin/puffin_test.go | 35 ++++++++++++++++++++++++++---------
puffin/puffin_writer.go | 5 +++--
website/src/releases.md | 2 ++
3 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/puffin/puffin_test.go b/puffin/puffin_test.go
index 879eee3ad..d06041e90 100644
--- a/puffin/puffin_test.go
+++ b/puffin/puffin_test.go
@@ -133,7 +133,7 @@ func TestWriterRejectsOperationsAfterPartialWriteFailure(t
*testing.T) {
_, err = writer.AddBlob(defaultBlobInput(),
[]byte("second blob payload"))
assert.ErrorIs(t, err, writeErr)
assert.ErrorIs(t, writer.Finish(), writeErr)
- assert.ErrorIs(t,
writer.AddProperties(map[string]string{"key": "value"}), writeErr)
+ assert.ErrorIs(t,
writer.SetProperties(map[string]string{"key": "value"}), writeErr)
assert.ErrorIs(t, writer.ClearProperties(), writeErr)
assert.ErrorIs(t, writer.SetCreatedBy("test"), writeErr)
assert.Equal(t, callsAfterFailure, output.calls)
@@ -154,7 +154,7 @@ func TestWriterRejectsOperationsAfterShortWrite(t
*testing.T) {
_, err = writer.AddBlob(defaultBlobInput(), []byte("second blob
payload"))
assert.ErrorContains(t, err, "short write")
assert.ErrorContains(t, writer.Finish(), "short write")
- assert.ErrorContains(t, writer.AddProperties(map[string]string{"key":
"value"}), "short write")
+ assert.ErrorContains(t, writer.SetProperties(map[string]string{"key":
"value"}), "short write")
assert.ErrorContains(t, writer.ClearProperties(), "short write")
assert.ErrorContains(t, writer.SetCreatedBy("test"), "short write")
assert.Equal(t, callsAfterFailure, output.calls)
@@ -314,7 +314,7 @@ func TestRoundTrip(t *testing.T) {
blob2Data := []byte("another blob with different content")
w, buf := newWriter()
- w.AddProperties(map[string]string{"test-property": "test-value"})
+ w.SetProperties(map[string]string{"test-property": "test-value"})
meta1, err := w.AddBlob(puffin.BlobMetadataInput{
Type: puffin.BlobTypeDataSketchesTheta,
@@ -398,7 +398,7 @@ func TestAddBlobCopiesMetadataInputs(t *testing.T) {
func TestReaderReturnsMetadataCopies(t *testing.T) {
w, buf := newWriter()
- require.NoError(t, w.AddProperties(map[string]string{"file":
"original"}))
+ require.NoError(t, w.SetProperties(map[string]string{"file":
"original"}))
_, err := w.AddBlob(puffin.BlobMetadataInput{
Type: puffin.BlobTypeDataSketchesTheta,
Fields: []int32{1},
@@ -690,11 +690,28 @@ func TestSetCreatedBy(t *testing.T) {
})
}
+// TestSetPropertiesMergesAndOverwrites verifies that property updates are
merged
+// rather than replacing the entire footer property map.
+func TestSetPropertiesMergesAndOverwrites(t *testing.T) {
+ w, buf := newWriter()
+ initial := map[string]string{"first": "one", "shared": "old"}
+ require.NoError(t, w.SetProperties(initial))
+ initial["first"] = "changed"
+ require.NoError(t, w.SetProperties(map[string]string{"shared": "new",
"second": "two"}))
+ require.NoError(t, w.SetProperties(nil))
+ require.NoError(t, w.Finish())
+
+ props := newReader(t, buf).Properties()
+ assert.Equal(t, "one", props["first"])
+ assert.Equal(t, "new", props["shared"])
+ assert.Equal(t, "two", props["second"])
+}
+
// TestClearProperties verifies the ClearProperties method.
// Allows resetting properties before finishing if initial values were wrong.
func TestClearProperties(t *testing.T) {
w, buf := newWriter()
- w.AddProperties(map[string]string{"key": "value"})
+ w.SetProperties(map[string]string{"key": "value"})
require.NoError(t, w.ClearProperties())
w.Finish()
@@ -709,12 +726,12 @@ func TestClearProperties(t *testing.T) {
})
}
-// TestAddPropertiesAfterFinish verifies AddProperties rejects calls after
finish.
-// Enforces writer state machine - properties cannot be added after footer is
written.
-func TestAddPropertiesAfterFinish(t *testing.T) {
+// TestSetPropertiesAfterFinish verifies SetProperties rejects calls after
finish.
+// Enforces writer state machine - properties cannot be changed after footer
is written.
+func TestSetPropertiesAfterFinish(t *testing.T) {
w, _ := newWriter()
w.Finish()
- assert.ErrorContains(t, w.AddProperties(map[string]string{"k": "v"}),
"finalized")
+ assert.ErrorContains(t, w.SetProperties(map[string]string{"k": "v"}),
"finalized")
}
// TestReaderInvalidFile verifies that Reader rejects invalid/corrupt files.
diff --git a/puffin/puffin_writer.go b/puffin/puffin_writer.go
index bd6b59726..a5836662f 100644
--- a/puffin/puffin_writer.go
+++ b/puffin/puffin_writer.go
@@ -91,8 +91,9 @@ func NewWriter(w io.Writer) (*Writer, error) {
}
// SetProperties merges the provided properties into the file-level properties
-// written to the footer. Can be called multiple times before Finish.
-func (w *Writer) AddProperties(props map[string]string) error {
+// written to the footer. Repeated calls overwrite matching keys; use
+// ClearProperties to remove previously set custom properties.
+func (w *Writer) SetProperties(props map[string]string) error {
if w.done {
return errors.New("puffin: cannot set properties: writer
already finalized")
}
diff --git a/website/src/releases.md b/website/src/releases.md
index d2aade3a6..0874a7ad2 100644
--- a/website/src/releases.md
+++ b/website/src/releases.md
@@ -44,6 +44,8 @@ Per-release notes (highlights, breaking changes,
contributors) are published on
- **Transform formatting requires the result type.**
`Transform.ToHumanStr(value)` is removed. Use
`Transform.ToHumanStrType(resultType, value)` to format partition values,
including timestamps with time zones.
+- **Puffin writer property setter renamed.** `puffin.Writer.AddProperties` is
now `puffin.Writer.SetProperties`. Repeated calls still merge keys; use
`ClearProperties` to clear custom properties before finishing.
+
- **Orphan file cleanup moved to `table/maintenance`.** The orphan-cleanup and
file-purge API is no longer on `table.Table`; it now lives in the
`table/maintenance` package as free functions that take the table as their
first argument (matching `table/compaction`). For example,
`tbl.DeleteOrphanFiles(ctx, ...)` becomes `maintenance.DeleteOrphanFiles(ctx,
tbl, ...)`. Because `table/maintenance` imports `table` (and not the reverse),
this cannot be offered as a deprecation shim on `table.T [...]
- Functions (now free functions taking `*table.Table`):
`DeleteOrphanFiles`, `PlanOrphanFiles`, `ExecuteOrphanCleanup`, `PurgeFiles`.
- Types: `OrphanCleanupOption`, `OrphanCleanupResult`, `OrphanFile`,
`OrphanCleanupPlan`, `PrefixMismatchMode`.
