This is an automated email from the ASF dual-hosted git repository.
laskoviymishka 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 83f062352 fix(puffin): constrain DV cardinality to int64 (#1587)
83f062352 is described below
commit 83f062352a33bf3f2fd80c6a8228ba387035ddb5
Author: Minh Vu <[email protected]>
AuthorDate: Thu Jul 30 10:02:12 2026 +0200
fix(puffin): constrain DV cardinality to int64 (#1587)
## What changed
Validate deletion-vector cardinality properties as non-negative signed
64-bit values. Add coverage for the maximum accepted value and the first
value above the signed range.
## Why
The writer accepted the full `uint64` range while the deletion-vector
reader and manifest record counts use `int64`. This allowed writing
Puffin files that the reader could not parse.
## Testing
- `go test ./puffin`
Signed-off-by: Minh Vu <[email protected]>
---
puffin/puffin_test.go | 23 +++++++++++++++++++++++
puffin/puffin_writer.go | 14 ++++++++------
2 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/puffin/puffin_test.go b/puffin/puffin_test.go
index 866233281..689fb1948 100644
--- a/puffin/puffin_test.go
+++ b/puffin/puffin_test.go
@@ -22,6 +22,7 @@ import (
"math"
"os"
"path"
+ "strconv"
"testing"
"github.com/apache/iceberg-go/puffin"
@@ -347,6 +348,28 @@ func TestWriterValidation(t *testing.T) {
assert.ErrorContains(t, err, "not a valid non-negative integer")
})
+ t.Run("deletion vector maximum cardinality", func(t *testing.T) {
+ w, _ := newWriter()
+ _, err := w.AddBlob(puffin.BlobMetadataInput{
+ Type: puffin.BlobTypeDeletionVector, SnapshotID: -1,
SequenceNumber: -1, Fields: []int32{},
+ Properties: map[string]string{
+ "cardinality": strconv.FormatInt(math.MaxInt64,
10), "referenced-data-file": "data/x.parquet",
+ },
+ }, []byte("x"))
+ require.NoError(t, err)
+ })
+
+ t.Run("deletion vector cardinality above signed range", func(t
*testing.T) {
+ w, _ := newWriter()
+ _, err := w.AddBlob(puffin.BlobMetadataInput{
+ Type: puffin.BlobTypeDeletionVector, SnapshotID: -1,
SequenceNumber: -1, Fields: []int32{},
+ Properties: map[string]string{
+ "cardinality": "9223372036854775808",
"referenced-data-file": "data/x.parquet",
+ },
+ }, []byte("x"))
+ assert.ErrorContains(t, err, "not a valid non-negative integer")
+ })
+
// deletion vector missing referenced-data-file property: spec-mandated.
t.Run("deletion vector missing referenced-data-file property", func(t
*testing.T) {
w, _ := newWriter()
diff --git a/puffin/puffin_writer.go b/puffin/puffin_writer.go
index 8fb6a261e..45da4195a 100644
--- a/puffin/puffin_writer.go
+++ b/puffin/puffin_writer.go
@@ -158,15 +158,17 @@ func (w *Writer) AddBlob(input BlobMetadataInput, data
[]byte) (BlobMetadata, er
if properties["cardinality"] == "" {
return BlobMetadata{}, errors.New("puffin:
deletion-vector-v1 requires a cardinality property")
}
- // Reject non-numeric or negative values at write time too —
otherwise
- // a writer could emit "cardinality": "abc" or "-1" that the
reader
- // hard-rejects later. ParseUint covers both: "-1" fails as
invalid
- // syntax (the minus is rejected before any value is parsed), so
- // non-numeric and negative collapse into one error path.
- if _, err := strconv.ParseUint(properties["cardinality"], 10,
64); err != nil {
+ // Parse the same signed range used by deletion-vector readers
and
+ // manifest record counts so every emitted value is readable.
+ cardinality, err := strconv.ParseInt(properties["cardinality"],
10, 64)
+ if err != nil {
return BlobMetadata{}, fmt.Errorf("puffin:
deletion-vector-v1 cardinality property %q is not a valid non-negative integer:
%w",
properties["cardinality"], err)
}
+ if cardinality < 0 {
+ return BlobMetadata{}, fmt.Errorf("puffin:
deletion-vector-v1 cardinality property %q is not a valid non-negative integer",
+ properties["cardinality"])
+ }
if properties["referenced-data-file"] == "" {
return BlobMetadata{}, errors.New("puffin:
deletion-vector-v1 requires a referenced-data-file property")
}