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")
                }

Reply via email to