You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ze...@apache.org on 2026/08/07 16:34:42 UTC
(iceberg-go) branch main updated: fix(schema): validate schema JSON shape (#1661)
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 248dfade fix(schema): validate schema JSON shape (#1661)
248dfade is described below
commit 248dfadeb2ba2e35516d8de8072947f143f78025
Author: Minh Vu <vu...@gmail.com>
AuthorDate: Fri Aug 7 18:34:37 2026 +0200
fix(schema): validate schema JSON shape (#1661)
* fix(schema): validate JSON envelope
* fix(schema): serialize empty fields as an array
* test(schema): cover JSON envelope validation
---
schema.go | 29 ++++++++++++++---
schema_test.go | 71 +++++++++++++++++++++++++++++++++++++++++
table/metadata_internal_test.go | 6 ++--
3 files changed, 99 insertions(+), 7 deletions(-)
diff --git a/schema.go b/schema.go
index 1a917fb8..a5f923f4 100644
--- a/schema.go
+++ b/schema.go
@@ -18,6 +18,7 @@
package iceberg
import (
+ "bytes"
"encoding/json"
"fmt"
"iter"
@@ -279,19 +280,35 @@ func (s *Schema) UnmarshalJSON(b []byte) error {
type Alias Schema
var decoded Schema
aux := struct {
- Fields []NestedField `json:"fields"`
+ Type *string `json:"type"`
+ Fields json.RawMessage `json:"fields"`
*Alias
}{Alias: (*Alias)(&decoded)}
if err := json.Unmarshal(b, &aux); err != nil {
return err
}
+ if aux.Type == nil {
+ return fmt.Errorf("%w: schema type is required", ErrInvalidSchema)
+ }
+ if *aux.Type != "struct" {
+ return fmt.Errorf("%w: schema type must be struct, got %q", ErrInvalidSchema, *aux.Type)
+ }
+ fieldsJSON := bytes.TrimSpace(aux.Fields)
+ if len(fieldsJSON) == 0 || bytes.Equal(fieldsJSON, []byte("null")) {
+ return fmt.Errorf("%w: schema fields are required", ErrInvalidSchema)
+ }
- if err := checkDuplicateFieldIDs(nil, aux.Fields); err != nil {
+ var fields []NestedField
+ if err := json.Unmarshal(fieldsJSON, &fields); err != nil {
+ return fmt.Errorf("%w: invalid schema fields: %w", ErrInvalidSchema, err)
+ }
+
+ if err := checkDuplicateFieldIDs(nil, fields); err != nil {
return err
}
- decoded.fields = aux.Fields
+ decoded.fields = fields
if decoded.IdentifierFieldIDs == nil {
decoded.IdentifierFieldIDs = []int{}
}
@@ -314,6 +331,10 @@ func (s *Schema) MarshalJSON() ([]byte, error) {
if ids == nil {
ids = []int{}
}
+ fields := s.fields
+ if fields == nil {
+ fields = []NestedField{}
+ }
type Alias Schema
@@ -324,7 +345,7 @@ func (s *Schema) MarshalJSON() ([]byte, error) {
Type string `json:"type"`
Fields []NestedField `json:"fields"`
*Alias
- }{Type: "struct", Fields: s.fields, Alias: &aliasCopy})
+ }{Type: "struct", Fields: fields, Alias: &aliasCopy})
}
func cloneFields(fields []NestedField) []NestedField {
diff --git a/schema_test.go b/schema_test.go
index 1aaedea9..a3d716f8 100644
--- a/schema_test.go
+++ b/schema_test.go
@@ -570,6 +570,55 @@ func TestUnmarshalSchema(t *testing.T) {
assert.True(t, tableSchemaSimple.Equals(&schema))
}
+func TestUnmarshalSchemaRejectsInvalidTopLevelShape(t *testing.T) {
+ for _, tt := range []struct {
+ name string
+ data string
+ }{
+ {name: "missing type", data: `{"fields": []}`},
+ {name: "null type", data: `{"type": null, "fields": []}`},
+ {name: "non-string type", data: `{"type": 1, "fields": []}`},
+ {name: "non-struct type", data: `{"type": "list", "fields": []}`},
+ {name: "missing fields", data: `{"type": "struct"}`},
+ {name: "null fields", data: `{"type": "struct", "fields": null}`},
+ {name: "scalar fields", data: `{"type": "struct", "fields": {}}`},
+ {name: "string fields", data: `{"type": "struct", "fields": "not-an-array"}`},
+ {name: "malformed field", data: `{"type": "struct", "fields": [{"id": 1, "name": "bad", "type": 1}]}`},
+ {name: "malformed json", data: `{"type": "struct", "fields": [`},
+ } {
+ t.Run(tt.name, func(t *testing.T) {
+ schema := iceberg.NewSchema(7,
+ iceberg.NestedField{ID: 1, Name: "old", Type: iceberg.PrimitiveTypes.String})
+ err := json.Unmarshal([]byte(tt.data), schema)
+ require.Error(t, err)
+ assert.Equal(t, 7, schema.ID)
+ assert.Equal(t, "old", schema.Field(0).Name)
+ })
+ }
+}
+
+func TestUnmarshalSchemaAcceptsLegacyV1Envelope(t *testing.T) {
+ var schema iceberg.Schema
+ require.NoError(t, json.Unmarshal([]byte(`{
+ "type": "struct",
+ "fields": [{"id": 1, "name": "id", "type": "long", "required": true}]
+ }`), &schema))
+
+ assert.Zero(t, schema.ID)
+ assert.Empty(t, schema.IdentifierFieldIDs)
+}
+
+func TestMarshalSchemaWritesEmptyFieldsArray(t *testing.T) {
+ data, err := json.Marshal(iceberg.NewSchema(0))
+ require.NoError(t, err)
+ assert.JSONEq(t, `{
+ "type": "struct",
+ "fields": [],
+ "schema-id": 0,
+ "identifier-field-ids": []
+ }`, string(data))
+}
+
func TestUnmarshalSchemaReplacesExistingState(t *testing.T) {
schema := iceberg.NewSchemaWithIdentifiers(7, []int{1},
iceberg.NestedField{ID: 1, Name: "old", Type: iceberg.PrimitiveTypes.String},
@@ -649,6 +698,28 @@ func TestUnmarshalSchemaPreservesExistingStateOnError(t *testing.T) {
assert.False(t, schema.FieldHasOptionalParent(1))
}
+func TestUnmarshalSchemaPreservesExistingStateOnFieldDecodeError(t *testing.T) {
+ schema := iceberg.NewSchemaWithIdentifiers(7, []int{1},
+ iceberg.NestedField{ID: 1, Name: "old", Type: iceberg.PrimitiveTypes.String},
+ )
+ _, ok := schema.FindFieldByID(1)
+ require.True(t, ok)
+
+ err := json.Unmarshal([]byte(`{
+ "type": "struct",
+ "fields": [{"id": 2, "name": "new", "type": 1}]
+ }`), schema)
+ require.Error(t, err)
+
+ assert.Equal(t, 7, schema.ID)
+ assert.Equal(t, []int{1}, schema.IdentifierFieldIDs)
+ assert.Equal(t, "old", schema.Field(0).Name)
+ _, ok = schema.FindFieldByID(1)
+ assert.True(t, ok)
+ _, ok = schema.FindFieldByID(2)
+ assert.False(t, ok)
+}
+
func TestUnmarshalSchemaRejectsDuplicateFieldIDs(t *testing.T) {
tests := []struct {
name string
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index 69cc5d57..e38b3123 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -1326,7 +1326,7 @@ func TestMetadataV1Validation(t *testing.T) {
"table-uuid": "d20125c8-7284-442c-9aea-15fee620737c",
"location": "s3://bucket/test/location",
"last-updated-ms": 1602638573874,
- "schema": {},
+ "schema": {"type": "struct", "fields": []},
"partition-spec": [],
"properties": {},
"current-snapshot-id": -1,
@@ -1340,7 +1340,7 @@ func TestMetadataV1Validation(t *testing.T) {
"location": "s3://bucket/test/location",
"last-updated-ms": 1602638573874,
"last-column-id": 5,
- "schema": {},
+ "schema": {"type": "struct", "fields": []},
"partition-spec": [],
"properties": {},
"current-snapshot-id": -1,
@@ -1354,7 +1354,7 @@ func TestMetadataV1Validation(t *testing.T) {
"location": "s3://bucket/test/location",
"last-updated-ms": 1602638573874,
"last-column-id": 0,
- "schema": {},
+ "schema": {"type": "struct", "fields": []},
"partition-spec": [],
"properties": {},
"current-snapshot-id": -1,