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 858379a65 refactor(puffin)!: unify statistics blob types (#2295)
858379a65 is described below
commit 858379a657387a2a327b8a025398779a741ae10a
Author: Minh Vu <[email protected]>
AuthorDate: Sun Oct 25 12:53:09 2026 +0100
refactor(puffin)!: unify statistics blob types (#2295)
---
puffin/blob_type_test.go | 77 +++++++++++++++++++++++++++++++++++++++++
puffin/puffin.go | 34 ++++++++++++++++++
puffin/puffin_test.go | 29 ++++++++++++++++
table/metadata_getters_test.go | 3 +-
table/metadata_internal_test.go | 5 +--
table/statistics.go | 49 ++------------------------
table/statistics_test.go | 8 ++---
website/src/releases.md | 1 +
8 files changed, 153 insertions(+), 53 deletions(-)
diff --git a/puffin/blob_type_test.go b/puffin/blob_type_test.go
new file mode 100644
index 000000000..21295b2e8
--- /dev/null
+++ b/puffin/blob_type_test.go
@@ -0,0 +1,77 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package puffin_test
+
+import (
+ "encoding/json"
+ "testing"
+
+ "github.com/apache/iceberg-go/puffin"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+func TestBlobTypeIsKnown(t *testing.T) {
+ assert.True(t, puffin.BlobTypeDataSketchesTheta.IsKnown())
+ assert.True(t, puffin.BlobTypeDeletionVector.IsKnown())
+ assert.False(t, puffin.BlobType("vendor-sketch-v1").IsKnown())
+ assert.False(t, puffin.BlobType("").IsKnown())
+}
+
+func TestBlobTypeUnmarshalErrorPreservesPreviousValue(t *testing.T) {
+ for _, input := range []string{`""`, "null", "123", `{}`} {
+ t.Run(input, func(t *testing.T) {
+ got := puffin.BlobTypeDeletionVector
+ err := json.Unmarshal([]byte(input), &got)
+ require.Error(t, err)
+ assert.Equal(t, puffin.BlobTypeDeletionVector, got)
+ })
+ }
+}
+
+func TestBlobTypeJSON(t *testing.T) {
+ for _, tc := range []struct {
+ name string
+ input string
+ want puffin.BlobType
+ wantErr bool
+ }{
+ {name: "theta", input: `"apache-datasketches-theta-v1"`, want:
puffin.BlobTypeDataSketchesTheta},
+ {name: "deletion vector", input: `"deletion-vector-v1"`, want:
puffin.BlobTypeDeletionVector},
+ {name: "vendor type", input: `"vendor-sketch-v1"`, want:
puffin.BlobType("vendor-sketch-v1")},
+ {name: "empty", input: `""`, wantErr: true},
+ {name: "null", input: "null", wantErr: true},
+ {name: "non-string", input: "123", wantErr: true},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ var got puffin.BlobType
+ err := json.Unmarshal([]byte(tc.input), &got)
+ if tc.wantErr {
+ require.Error(t, err)
+
+ return
+ }
+
+ require.NoError(t, err)
+ assert.Equal(t, tc.want, got)
+ encoded, err := json.Marshal(got)
+ require.NoError(t, err)
+ assert.JSONEq(t, tc.input, string(encoded))
+ })
+ }
+}
diff --git a/puffin/puffin.go b/puffin/puffin.go
index c4b88303e..1a8466139 100644
--- a/puffin/puffin.go
+++ b/puffin/puffin.go
@@ -28,6 +28,11 @@
// See the specification at https://iceberg.apache.org/puffin-spec/
package puffin
+import (
+ "encoding/json"
+ "fmt"
+)
+
var magic = [4]byte{'P', 'F', 'A', '1'}
const (
@@ -74,6 +79,8 @@ type Footer struct {
Properties map[string]string `json:"properties,omitempty"`
}
+// BlobType identifies the format of a Puffin blob. Unknown non-empty names
+// are preserved so that callers can round-trip vendor-defined blob types.
type BlobType string
const (
@@ -85,3 +92,30 @@ const (
// Iceberg spec. Requires snapshot-id and sequence-number to be -1.
BlobTypeDeletionVector BlobType = "deletion-vector-v1"
)
+
+// IsKnown reports whether the type is one of the standard blob types
+// defined by the Puffin spec. Unknown non-empty types are still valid
metadata.
+func (bt BlobType) IsKnown() bool {
+ switch bt {
+ case BlobTypeDataSketchesTheta, BlobTypeDeletionVector:
+ return true
+ default:
+ return false
+ }
+}
+
+// UnmarshalJSON rejects empty and null blob types while preserving unknown
+// non-empty type names for forward compatibility.
+func (bt *BlobType) UnmarshalJSON(data []byte) error {
+ var name string
+ if err := json.Unmarshal(data, &name); err != nil {
+ return err
+ }
+ if name == "" {
+ return fmt.Errorf("invalid blob type: %s", name)
+ }
+
+ *bt = BlobType(name)
+
+ return nil
+}
diff --git a/puffin/puffin_test.go b/puffin/puffin_test.go
index d06041e90..aad597296 100644
--- a/puffin/puffin_test.go
+++ b/puffin/puffin_test.go
@@ -830,6 +830,35 @@ func TestReaderRejectsTrailingFooterData(t *testing.T) {
}
}
+// TestReaderRejectsMissingBlobTypes checks footer decoding and validation,
+// including when the malformed blob follows an otherwise valid one.
+func TestReaderRejectsMissingBlobTypes(t *testing.T) {
+ t.Parallel()
+
+ tests := []struct {
+ name string
+ typeJSON string
+ wantErr string
+ }{
+ {name: "empty", typeJSON: `"type":""`, wantErr: "invalid blob
type"},
+ {name: "null", typeJSON: `"type":null`, wantErr: "invalid blob
type"},
+ {name: "absent", wantErr: "puffin: blob 1: type is required"},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ t.Parallel()
+ second := `{"fields":[],"offset":4,"length":0}`
+ if tt.typeJSON != "" {
+ second = `{` + tt.typeJSON +
`,"fields":[],"offset":4,"length":0}`
+ }
+ footer :=
`{"blobs":[{"type":"vendor-sketch-v1","fields":[],"offset":4,"length":0},` +
second + `]}`
+ _, err :=
puffin.NewReader(bytes.NewReader(fileWithFooterPayload([]byte(footer))))
+ require.ErrorContains(t, err, tt.wantErr)
+ })
+ }
+}
+
func TestReaderReadsLZ4CompressedFooter(t *testing.T) {
data := fileWithCompressedFooterPayload(t,
[]byte(`{"blobs":[],"properties":{"source":"lz4"}}`))
diff --git a/table/metadata_getters_test.go b/table/metadata_getters_test.go
index 99d007aa0..93a7be6cc 100644
--- a/table/metadata_getters_test.go
+++ b/table/metadata_getters_test.go
@@ -24,6 +24,7 @@ import (
"github.com/apache/iceberg-go"
iceinternal "github.com/apache/iceberg-go/internal"
+ "github.com/apache/iceberg-go/puffin"
"github.com/stretchr/testify/require"
)
@@ -69,7 +70,7 @@ func TestMetadataGettersReturnDefensiveCopies(t *testing.T) {
StatisticsPath: "stats.puffin",
KeyMetadata: &keyMetadata,
BlobMetadata: []BlobMetadata{{
- Type: BlobTypeApacheDatasketchesThetaV1,
+ Type: puffin.BlobTypeDataSketchesTheta,
Fields: []int32{1},
Properties: map[string]string{"source": "test"},
}},
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index 9c9e3d3c2..d8db65ec9 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -30,6 +30,7 @@ import (
"testing"
"github.com/apache/iceberg-go"
+ "github.com/apache/iceberg-go/puffin"
"github.com/davecgh/go-spew/spew"
"github.com/google/go-cmp/cmp"
"github.com/google/uuid"
@@ -1725,7 +1726,7 @@ func TestMetadataV1Serialize(t *testing.T) {
KeyMetadata: nil,
BlobMetadata: []BlobMetadata{
{
- Type:
BlobTypeApacheDatasketchesThetaV1,
+ Type:
puffin.BlobTypeDataSketchesTheta,
SnapshotID:
1234567890,
SequenceNumber: 1,
Fields:
[]int32{1, 2},
@@ -1817,7 +1818,7 @@ func TestMetadataV2Serialize(t *testing.T) {
FileFooterSizeInBytes: 1024,
BlobMetadata: []BlobMetadata{
{
- Type:
BlobTypeDeletionVectorV1,
+ Type:
puffin.BlobTypeDeletionVector,
SnapshotID:
9876543210,
SequenceNumber: 5,
Fields:
[]int32{1},
diff --git a/table/statistics.go b/table/statistics.go
index 645e119d4..46a429c4d 100644
--- a/table/statistics.go
+++ b/table/statistics.go
@@ -17,51 +17,7 @@
package table
-import (
- "encoding/json"
- "fmt"
-)
-
-// BlobType is the type of blob in a Puffin file.
-type BlobType string
-
-const (
- BlobTypeApacheDatasketchesThetaV1 BlobType =
"apache-datasketches-theta-v1"
- BlobTypeDeletionVectorV1 BlobType = "deletion-vector-v1"
-)
-
-// IsKnown reports whether bt is one of the standard blob types understood by
-// this package. Unknown non-empty wire values may still be preserved for
-// forward compatibility.
-func (bt BlobType) IsKnown() bool {
- switch bt {
- case BlobTypeApacheDatasketchesThetaV1, BlobTypeDeletionVectorV1:
- return true
- default:
- return false
- }
-}
-
-// IsValid reports whether bt is non-nil and names a standard blob type.
-// Unknown non-empty values can be decoded and preserved, but are not valid for
-// operations that require a known blob type.
-func (bt *BlobType) IsValid() bool {
- return bt != nil && bt.IsKnown()
-}
-
-func (bt *BlobType) UnmarshalJSON(data []byte) error {
- var s string
- if err := json.Unmarshal(data, &s); err != nil {
- return err
- }
-
- *bt = BlobType(s)
- if s == "" {
- return fmt.Errorf("invalid blob type: %s", s)
- }
-
- return nil
-}
+import "github.com/apache/iceberg-go/puffin"
// StatisticsFile represents a statistics file in the Puffin format, that can
be used to read table data more
// efficiently.
@@ -78,8 +34,9 @@ type StatisticsFile struct {
}
// BlobMetadata is the metadata of a statistics or indices blob.
+// Its type uses the same Puffin blob identifiers as the file reader and
writer.
type BlobMetadata struct {
- Type BlobType `json:"type"`
+ Type puffin.BlobType `json:"type"`
SnapshotID int64 `json:"snapshot-id"`
SequenceNumber int64 `json:"sequence-number"`
Fields []int32 `json:"fields"`
diff --git a/table/statistics_test.go b/table/statistics_test.go
index d5ae594bd..d9796bde6 100644
--- a/table/statistics_test.go
+++ b/table/statistics_test.go
@@ -20,6 +20,7 @@ import (
"encoding/json"
"testing"
+ "github.com/apache/iceberg-go/puffin"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -32,7 +33,7 @@ func TestMetadataPreservesUnknownStatisticsBlobType(t
*testing.T) {
SnapshotID: 3055729675574597004,
StatisticsPath: "s3://bucket/stats/vendor.puffin",
BlobMetadata: []BlobMetadata{{
- Type: BlobType("vendor-sketch-v1"),
+ Type: puffin.BlobType("vendor-sketch-v1"),
SnapshotID: 3055729675574597004,
SequenceNumber: 2,
Fields: []int32{1},
@@ -51,12 +52,11 @@ func TestMetadataPreservesUnknownStatisticsBlobType(t
*testing.T) {
}
require.Len(t, stats, 1)
require.Len(t, stats[0].BlobMetadata, 1)
- assert.Equal(t, BlobType("vendor-sketch-v1"),
stats[0].BlobMetadata[0].Type)
+ assert.Equal(t, puffin.BlobType("vendor-sketch-v1"),
stats[0].BlobMetadata[0].Type)
assert.False(t, stats[0].BlobMetadata[0].Type.IsKnown())
- assert.False(t, stats[0].BlobMetadata[0].Type.IsValid())
}
-func TestBlobTypeRejectsNullAndEmptyValues(t *testing.T) {
+func TestBlobMetadataRejectsNullAndEmptyType(t *testing.T) {
for _, value := range []string{"null", `""`} {
t.Run(value, func(t *testing.T) {
var blob BlobMetadata
diff --git a/website/src/releases.md b/website/src/releases.md
index 074e1453f..e0d398b74 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
+- **Unify statistics blob types with Puffin.** Replace `table.BlobType` with
`puffin.BlobType` when constructing `table.BlobMetadata`. Replace
`table.BlobTypeApacheDatasketchesThetaV1` with
`puffin.BlobTypeDataSketchesTheta` and `table.BlobTypeDeletionVectorV1` with
`puffin.BlobTypeDeletionVector`. Use `puffin.BlobType.IsKnown()` to identify
the types defined by the Puffin specification; the redundant
`table.BlobType.IsValid()` method is removed. Unknown non-empty blob type names
still r [...]
- **View representation field uses the Go SQL initialism.** Replace
`view.Representation.Sql` with `view.Representation.SQL`. Serialized view
metadata still uses the `sql` JSON field.
- **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.
