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 7581291e8 refactor: encode nested types without aliasing their
exported fields (#2361)
7581291e8 is described below
commit 7581291e82650407697232213ab8551c1e0b957e
Author: Revanth <[email protected]>
AuthorDate: Wed Nov 4 11:11:50 2026 -0600
refactor: encode nested types without aliasing their exported fields (#2361)
* refactor: encode nested types without aliasing their exported fields
* refactor: name every JSON key in Schema and NestedField decoding
---
nested_type_json_internal_test.go | 72 ++++++++++++++++++++++++++
schema.go | 40 +++++++--------
types.go | 86 ++++++++++++++++++-------------
types_test.go | 105 ++++++++++++++++++++++++++++++++++++++
4 files changed, 246 insertions(+), 57 deletions(-)
diff --git a/nested_type_json_internal_test.go
b/nested_type_json_internal_test.go
new file mode 100644
index 000000000..30eca8487
--- /dev/null
+++ b/nested_type_json_internal_test.go
@@ -0,0 +1,72 @@
+// 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 iceberg
+
+import (
+ "encoding/json"
+ "reflect"
+ "testing"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+)
+
+// TestNestedTypeJSONRoundTripsEveryField fails when a field is added to one of
+// these types without a value in its fixture, and when the JSON methods drop a
+// field. It walks unexported fields too, so it keeps working after #2062.
+func TestNestedTypeJSONRoundTripsEveryField(t *testing.T) {
+ t.Run("StructType", func(t *testing.T) {
+ requireJSONRoundTripsEveryField(t, StructType{FieldList:
[]NestedField{
+ {ID: 1, Name: "a", Type: PrimitiveTypes.Int32,
Required: true},
+ }})
+ })
+ t.Run("ListType", func(t *testing.T) {
+ requireJSONRoundTripsEveryField(t, ListType{
+ ElementID: 1, Element: PrimitiveTypes.String,
ElementRequired: true,
+ })
+ })
+ t.Run("MapType", func(t *testing.T) {
+ requireJSONRoundTripsEveryField(t, MapType{
+ KeyID: 1, KeyType: PrimitiveTypes.String,
+ ValueID: 2, ValueType: PrimitiveTypes.Int32,
ValueRequired: true,
+ })
+ })
+ t.Run("NestedField", func(t *testing.T) {
+ requireJSONRoundTripsEveryField(t, NestedField{
+ ID: 1, Name: "a", Type: PrimitiveTypes.String,
Required: true,
+ Doc: "doc", InitialDefault: "initial", WriteDefault:
"write",
+ })
+ })
+}
+
+func requireJSONRoundTripsEveryField[T any](t *testing.T, value T) {
+ t.Helper()
+
+ v := reflect.ValueOf(value)
+ for i := range v.NumField() {
+ require.Falsef(t, v.Field(i).IsZero(), "populate %s.%s in the
fixture",
+ v.Type().Name(), v.Type().Field(i).Name)
+ }
+
+ data, err := json.Marshal(&value)
+ require.NoError(t, err)
+
+ var decoded T
+ require.NoError(t, json.Unmarshal(data, &decoded))
+ assert.Equal(t, value, decoded)
+}
diff --git a/schema.go b/schema.go
index 94c8fc0e4..e518e3a5e 100644
--- a/schema.go
+++ b/schema.go
@@ -295,13 +295,12 @@ func (s *Schema) FieldIDs() []int {
}
func (s *Schema) UnmarshalJSON(b []byte) error {
- type Alias Schema
- var decoded Schema
aux := struct {
- Type *string `json:"type"`
- Fields json.RawMessage `json:"fields"`
- *Alias
- }{Alias: (*Alias)(&decoded)}
+ Type *string `json:"type"`
+ Fields json.RawMessage `json:"fields"`
+ ID int `json:"schema-id"`
+ IdentifierFieldIDs []int `json:"identifier-field-ids"`
+ }{}
if err := json.Unmarshal(b, &aux); err != nil {
return err
@@ -326,14 +325,14 @@ func (s *Schema) UnmarshalJSON(b []byte) error {
return err
}
- decoded.fields = fields
- if decoded.IdentifierFieldIDs == nil {
- decoded.IdentifierFieldIDs = []int{}
+ ids := aux.IdentifierFieldIDs
+ if ids == nil {
+ ids = []int{}
}
- s.ID = decoded.ID
- s.IdentifierFieldIDs = decoded.IdentifierFieldIDs
- s.fields = decoded.fields
+ s.ID = aux.ID
+ s.IdentifierFieldIDs = ids
+ s.fields = fields
s.idToName.Store(nil)
s.idToField.Store(nil)
s.nameToID.Store(nil)
@@ -354,17 +353,14 @@ func (s *Schema) MarshalJSON() ([]byte, error) {
fields = []NestedField{}
}
- type Alias Schema
-
- // Keep this literal in sync with every JSON-marshaled Schema field.
Copying
- // the whole Schema would also copy its lazy atomic caches after they
are used.
- aliasCopy := Alias{ID: s.ID, IdentifierFieldIDs: ids}
-
+ // Name every serialized field instead of encoding through an alias of
+ // Schema, so the wire format does not depend on which fields are
exported.
return json.Marshal(struct {
- Type string `json:"type"`
- Fields []NestedField `json:"fields"`
- *Alias
- }{Type: "struct", Fields: fields, Alias: &aliasCopy})
+ Type string `json:"type"`
+ Fields []NestedField `json:"fields"`
+ ID int `json:"schema-id"`
+ IdentifierFieldIDs []int `json:"identifier-field-ids"`
+ }{Type: "struct", Fields: fields, ID: s.ID, IdentifierFieldIDs: ids})
}
func cloneFields(fields []NestedField) []NestedField {
diff --git a/types.go b/types.go
index adabde404..0e780fbdc 100644
--- a/types.go
+++ b/types.go
@@ -374,42 +374,41 @@ func defaultValueToJSON(typ Type, value any) any {
}
func (n *NestedField) UnmarshalJSON(b []byte) error {
- type Alias NestedField
- var next NestedField
aux := struct {
- ID *int `json:"id"`
- Required *bool `json:"required"`
- Type typeIFace `json:"type"`
- *Alias
- }{
- Alias: (*Alias)(&next),
- }
+ ID *int `json:"id"`
+ Name string `json:"name"`
+ Required *bool `json:"required"`
+ Type typeIFace `json:"type"`
+ Doc string `json:"doc"`
+ InitialDefault any `json:"initial-default"`
+ WriteDefault any `json:"write-default"`
+ }{}
if err := json.Unmarshal(b, &aux); err != nil {
return err
}
- next.Type = aux.Type.Type
-
if aux.ID == nil {
return fmt.Errorf("%w: field is missing required 'id' key in
JSON", ErrInvalidSchema)
}
- next.ID = *aux.ID
- if next.Name == "" {
+ if aux.Name == "" {
return fmt.Errorf("%w: field is missing required 'name' key in
JSON", ErrInvalidSchema)
}
- if next.Type == nil {
- return fmt.Errorf("%w: field %q is missing required 'type' key
in JSON", ErrInvalidSchema, next.Name)
+ if aux.Type.Type == nil {
+ return fmt.Errorf("%w: field %q is missing required 'type' key
in JSON", ErrInvalidSchema, aux.Name)
}
if aux.Required == nil {
return fmt.Errorf("%w: field is missing required 'required' key
in JSON", ErrInvalidSchema)
}
- next.Required = *aux.Required
- *n = next
+ *n = NestedField{
+ Type: aux.Type.Type,
+ ID: *aux.ID, Name: aux.Name, Required: *aux.Required, Doc:
aux.Doc,
+ InitialDefault: aux.InitialDefault, WriteDefault:
aux.WriteDefault,
+ }
return nil
}
@@ -431,13 +430,28 @@ func (s *StructType) Equals(other Type) bool {
func (s *StructType) Fields() []NestedField { return s.FieldList }
-func (s *StructType) MarshalJSON() ([]byte, error) {
- type Alias StructType
+// The JSON methods of the nested types name every serialized field instead of
+// encoding through an alias of the type, so the wire format does not depend on
+// which Go fields are exported.
+func (s *StructType) MarshalJSON() ([]byte, error) {
return json.Marshal(struct {
- Type string `json:"type"`
- *Alias
- }{Type: s.Type(), Alias: (*Alias)(s)})
+ Type string `json:"type"`
+ Fields []NestedField `json:"fields"`
+ }{Type: s.Type(), Fields: s.FieldList})
+}
+
+func (s *StructType) UnmarshalJSON(b []byte) error {
+ aux := struct {
+ Fields []NestedField `json:"fields"`
+ }{}
+ if err := json.Unmarshal(b, &aux); err != nil {
+ return err
+ }
+
+ s.FieldList = aux.Fields
+
+ return nil
}
func (*StructType) Type() string { return "struct" }
@@ -474,13 +488,15 @@ type ListType struct {
}
func (l *ListType) MarshalJSON() ([]byte, error) {
- type Alias ListType
-
return json.Marshal(struct {
- Type string `json:"type"`
- *Alias
- Element *typeIFace `json:"element"`
- }{Type: l.Type(), Alias: (*Alias)(l), Element: &typeIFace{l.Element}})
+ Type string `json:"type"`
+ ElementID int `json:"element-id"`
+ ElementRequired bool `json:"element-required"`
+ Element *typeIFace `json:"element"`
+ }{
+ Type: l.Type(), ElementID: l.ElementID, ElementRequired:
l.ElementRequired,
+ Element: &typeIFace{l.Element},
+ })
}
func (l *ListType) Equals(other Type) bool {
@@ -546,15 +562,15 @@ type MapType struct {
}
func (m *MapType) MarshalJSON() ([]byte, error) {
- type Alias MapType
-
return json.Marshal(struct {
- Type string `json:"type"`
- *Alias
- KeyType *typeIFace `json:"key"`
- ValueType *typeIFace `json:"value"`
+ Type string `json:"type"`
+ KeyID int `json:"key-id"`
+ ValueID int `json:"value-id"`
+ ValueRequired bool `json:"value-required"`
+ KeyType *typeIFace `json:"key"`
+ ValueType *typeIFace `json:"value"`
}{
- Type: m.Type(), Alias: (*Alias)(m),
+ Type: m.Type(), KeyID: m.KeyID, ValueID: m.ValueID,
ValueRequired: m.ValueRequired,
KeyType: &typeIFace{m.KeyType},
ValueType: &typeIFace{m.ValueType},
})
diff --git a/types_test.go b/types_test.go
index eb3a453cd..0224942c1 100644
--- a/types_test.go
+++ b/types_test.go
@@ -304,6 +304,111 @@ func TestMapTypeUnmarshalRequiresValue(t *testing.T) {
assert.ErrorContains(t, err, "missing required 'value' key")
}
+// nestedTypeWireFixture nests every composite type inside the others, with
+// required and optional children, docs and defaults on nested fields.
+func nestedTypeWireFixture() *iceberg.StructType {
+ return &iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 1, Name: "id", Type: iceberg.PrimitiveTypes.Int64,
Required: true},
+ {ID: 2, Name: "tags", Doc: "labels", Type: &iceberg.ListType{
+ ElementID: 3, Element: iceberg.PrimitiveTypes.String,
ElementRequired: false,
+ }},
+ {ID: 4, Name: "attrs", Required: true, Type: &iceberg.MapType{
+ KeyID: 5, KeyType: iceberg.PrimitiveTypes.String,
+ ValueID: 6, ValueType: &iceberg.StructType{FieldList:
[]iceberg.NestedField{
+ {ID: 7, Name: "score", Type:
iceberg.PrimitiveTypes.Float64, Required: true},
+ {ID: 8, Name: "note", Type:
iceberg.PrimitiveTypes.String, InitialDefault: "n/a", WriteDefault: "none"},
+ }},
+ ValueRequired: true,
+ }},
+ {ID: 9, Name: "points", Type: &iceberg.ListType{
+ ElementID: 10, ElementRequired: true, Element:
&iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 11, Name: "x", Type:
iceberg.PrimitiveTypes.Float32, Required: true},
+ {ID: 12, Name: "y", Type:
iceberg.PrimitiveTypes.Float32, Required: true},
+ }},
+ }},
+ {ID: 13, Name: "flags", Type: &iceberg.MapType{
+ KeyID: 14, KeyType: iceberg.PrimitiveTypes.Int32,
+ ValueID: 15, ValueType: &iceberg.ListType{
+ ElementID: 16, Element:
iceberg.PrimitiveTypes.Bool, ElementRequired: false,
+ },
+ ValueRequired: false,
+ }},
+ {ID: 17, Name: "active", Type: iceberg.PrimitiveTypes.Bool,
InitialDefault: true},
+ }}
+}
+
+const nestedTypeWireJSON = `{"type":"struct","fields":[` +
+ `{"type":"long","id":1,"name":"id","required":true},` +
+
`{"type":{"type":"list","element-id":3,"element-required":false,"element":"string"},`
+
+ `"id":2,"name":"tags","required":false,"doc":"labels"},` +
+
`{"type":{"type":"map","key-id":5,"value-id":6,"value-required":true,"key":"string",`
+
+ `"value":{"type":"struct","fields":[` +
+ `{"type":"double","id":7,"name":"score","required":true},` +
+
`{"type":"string","id":8,"name":"note","required":false,"initial-default":"n/a","write-default":"none"}]}},`
+
+ `"id":4,"name":"attrs","required":true},` +
+ `{"type":{"type":"list","element-id":10,"element-required":true,` +
+ `"element":{"type":"struct","fields":[` +
+ `{"type":"float","id":11,"name":"x","required":true},` +
+ `{"type":"float","id":12,"name":"y","required":true}]}},` +
+ `"id":9,"name":"points","required":false},` +
+
`{"type":{"type":"map","key-id":14,"value-id":15,"value-required":false,"key":"int",`
+
+
`"value":{"type":"list","element-id":16,"element-required":false,"element":"boolean"}},`
+
+ `"id":13,"name":"flags","required":false},` +
+
`{"type":"boolean","id":17,"name":"active","required":false,"initial-default":true}]}`
+
+// TestNestedTypeJSONWireFormat pins the exact bytes, key order included, that
+// nested types encode to, and that decoding them rebuilds an equal type.
+func TestNestedTypeJSONWireFormat(t *testing.T) {
+ typ := nestedTypeWireFixture()
+
+ out, err := json.Marshal(typ)
+ require.NoError(t, err)
+ assert.Equal(t, nestedTypeWireJSON, string(out))
+
+ var decoded iceberg.StructType
+ require.NoError(t, json.Unmarshal([]byte(nestedTypeWireJSON), &decoded))
+ assert.True(t, typ.Equals(&decoded), "decoded: %s", &decoded)
+
+ again, err := json.Marshal(&decoded)
+ require.NoError(t, err)
+ assert.Equal(t, nestedTypeWireJSON, string(again))
+
+ t.Run("in schema", func(t *testing.T) {
+ sc := iceberg.NewSchema(0, typ.FieldList...)
+ out, err := json.Marshal(sc)
+ require.NoError(t, err)
+
+ var decoded iceberg.Schema
+ require.NoError(t, json.Unmarshal(out, &decoded))
+ assert.True(t, sc.Equals(&decoded))
+ })
+}
+
+// TestStructTypeUnmarshalMissingFieldsIsEmpty decodes into a populated type:
+// like the other nested types, a decode replaces the whole value.
+func TestStructTypeUnmarshalMissingFieldsIsEmpty(t *testing.T) {
+ typ := iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 1, Name: "stale", Type: iceberg.PrimitiveTypes.String},
+ }}
+ require.NoError(t, json.Unmarshal([]byte(`{"type":"struct"}`), &typ))
+ assert.Empty(t, typ.FieldList)
+}
+
+func TestStructTypeUnmarshalErrorLeavesTypeUnchanged(t *testing.T) {
+ kept := func() iceberg.StructType {
+ return iceberg.StructType{FieldList: []iceberg.NestedField{
+ {ID: 1, Name: "kept", Type:
iceberg.PrimitiveTypes.String},
+ }}
+ }
+ typ := kept()
+
+ err := json.Unmarshal([]byte(`{"type":"struct","fields":[`+
+ `{"id":2,"name":"replaced","type":"int","required":true},`+
+ `{"id":3,"name":"bad","required":false}]}`), &typ)
+ assert.ErrorIs(t, err, iceberg.ErrInvalidSchema)
+ assert.Equal(t, kept(), typ)
+}
+
var NonParameterizedTypes = []iceberg.Type{
iceberg.PrimitiveTypes.Bool,
iceberg.PrimitiveTypes.Int32,
