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 18baae267 fix(table): validate last-updated-ms presence (#1591)
18baae267 is described below
commit 18baae267848679afb7f693c82b97e6b43be9a8a
Author: Minh Vu <[email protected]>
AuthorDate: Fri Jul 31 16:21:13 2026 +0200
fix(table): validate last-updated-ms presence (#1591)
## What changed
Require `last-updated-ms` to be present and non-null while decoding
metadata versions 1, 2, and 3. Numeric values remain unrestricted, so
Unix epoch zero and existing negative fixture values are preserved.
Add coverage for missing, null, epoch-zero, and negative values across
all metadata format versions.
## Why
The decoder previously used a numeric sentinel for presence validation.
That made a valid number indistinguishable from an absent field and
either accepted missing metadata or rejected explicitly encoded values.
## Testing
- `go test ./table ./table/internal -count=1 -timeout=180s`
- `go test ./... -count=1 -timeout=300s`
---------
Signed-off-by: Minh Vu <[email protected]>
---
cmd/iceberg/output_test.go | 11 ++++--
table/metadata.go | 24 +++++++++----
table/metadata_internal_test.go | 77 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+), 9 deletions(-)
diff --git a/cmd/iceberg/output_test.go b/cmd/iceberg/output_test.go
index 4d149b90d..cf8604be5 100644
--- a/cmd/iceberg/output_test.go
+++ b/cmd/iceberg/output_test.go
@@ -28,6 +28,7 @@ import (
"github.com/apache/iceberg-go/table"
"github.com/pterm/pterm"
"github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
)
func Test_textOutput_DescribeTable(t *testing.T) {
@@ -150,6 +151,7 @@ read.split.target.size | 134217728
{"type": "struct", "schema-id": 0, "fields": [{"id": 1, "name": "x",
"required": true, "type": "long"}]},
{
"type": "struct",
+ "schema-id": 1,
"fields": [
{"id": 1, "name": "x", "required": true, "type": "long"}
]
@@ -201,7 +203,8 @@ read.split.target.size | 134217728
pterm.SetDefaultOutput(&buf)
pterm.DisableColor()
- meta, _ :=
table.ParseMetadataBytes([]byte(tt.args.meta))
+ meta, err :=
table.ParseMetadataBytes([]byte(tt.args.meta))
+ require.NoError(t, err)
tbl := table.New([]string{"t"}, meta, "", nil, nil)
buf.Reset()
@@ -361,6 +364,7 @@ func Test_jsonOutput_DescribeTable(t *testing.T) {
{"type": "struct", "schema-id": 0, "fields": [{"id": 1, "name": "x",
"required": true, "type": "long"}]},
{
"type": "struct",
+ "schema-id": 1,
"fields": [
{"id": 1, "name": "x", "required": true, "type": "long"}
]
@@ -384,7 +388,7 @@ func Test_jsonOutput_DescribeTable(t *testing.T) {
"refs": { }
}`,
},
- expected:
`{"metadata":{"last-sequence-number":0,"format-version":2,"table-uuid":"9c12d441-03fe-4693-9a96-a0705ddf69c1","location":"s3://bucket/test/location","last-updated-ms":1602638573590,"last-column-id":3,"schemas":[{"type":"struct","fields":[{"type":"long","id":1,"name":"x","required":true}],"schema-id":0,"identifier-field-ids":[]},{"type":"struct","fields":[{"type":"long","id":1,"name":"x","required":true}],"schema-id":0,"identifier-field-ids":[]}],"current-schema-id":0,"partit
[...]
+ expected:
`{"metadata":{"last-sequence-number":0,"format-version":2,"table-uuid":"9c12d441-03fe-4693-9a96-a0705ddf69c1","location":"s3://bucket/test/location","last-updated-ms":1602638573590,"last-column-id":3,"schemas":[{"type":"struct","fields":[{"type":"long","id":1,"name":"x","required":true}],"schema-id":0,"identifier-field-ids":[]},{"type":"struct","fields":[{"type":"long","id":1,"name":"x","required":true}],"schema-id":1,"identifier-field-ids":[]}],"current-schema-id":0,"partit
[...]
},
}
for _, tt := range tests {
@@ -397,7 +401,8 @@ func Test_jsonOutput_DescribeTable(t *testing.T) {
os.Stdout = oldStdout
}()
- meta, _ :=
table.ParseMetadataBytes([]byte(tt.args.meta))
+ meta, err :=
table.ParseMetadataBytes([]byte(tt.args.meta))
+ require.NoError(t, err)
tbl := table.New([]string{"t"}, meta, "", nil, nil)
jsonOutput{}.DescribeTable(tbl)
diff --git a/table/metadata.go b/table/metadata.go
index 7b7a18749..439f4be5b 100644
--- a/table/metadata.go
+++ b/table/metadata.go
@@ -1509,7 +1509,7 @@ func ParseMetadataBytes(b []byte) (Metadata, error) {
FormatVersion int `json:"format-version"`
}{}
if err := json.Unmarshal(b, &ver); err != nil {
- return nil, err
+ return nil, fmt.Errorf("%w: %w", ErrInvalidMetadata, err)
}
var ret Metadata
@@ -1529,12 +1529,19 @@ func ParseMetadataBytes(b []byte) (Metadata, error) {
return nil, err
}
- return ret, json.Unmarshal(normalized, ret)
+ if err := json.Unmarshal(normalized, ret); err != nil {
+ return nil, fmt.Errorf("%w: %w", ErrInvalidMetadata, err)
+ }
+
+ return ret, nil
}
func assignMissingPartitionFieldIDs(b []byte) ([]byte, error) {
var metadata map[string]json.RawMessage
if err := json.Unmarshal(b, &metadata); err != nil {
+ return nil, fmt.Errorf("%w: %w", ErrInvalidMetadata, err)
+ }
+ if err := requireLastUpdatedMS(metadata); err != nil {
return nil, err
}
@@ -1656,7 +1663,6 @@ type commonMetadata struct {
func initCommonMetadataForDeserialization() commonMetadata {
return commonMetadata{
- LastUpdatedMS: -1,
LastColumnId: -1,
CurrentSchemaID: -1,
DefaultSpecID: -1,
@@ -1666,6 +1672,15 @@ func initCommonMetadataForDeserialization()
commonMetadata {
}
}
+func requireLastUpdatedMS(fields map[string]json.RawMessage) error {
+ value, ok := fields["last-updated-ms"]
+ if !ok || string(value) == "null" {
+ return fmt.Errorf("%w: last-updated-ms is absent or null",
ErrInvalidMetadata)
+ }
+
+ return nil
+}
+
func (c *commonMetadata) Ref() SnapshotRef {
return cloneSnapshotRef(c.SnapshotRefs[MainBranch])
}
@@ -2240,9 +2255,6 @@ func (c *commonMetadata) constructRefs() {
func (c *commonMetadata) validate() error {
switch {
- case c.LastUpdatedMS == 0:
- // last-updated-ms is required
- return fmt.Errorf("%w: missing last-updated-ms",
ErrInvalidMetadata)
case c.LastColumnId < 0:
// last-column-id is required
return fmt.Errorf("%w: missing last-column-id",
ErrInvalidMetadata)
diff --git a/table/metadata_internal_test.go b/table/metadata_internal_test.go
index 087bf6d73..88534b669 100644
--- a/table/metadata_internal_test.go
+++ b/table/metadata_internal_test.go
@@ -310,6 +310,82 @@ func TestMetadataV3Parsing(t *testing.T) {
assert.Equal(t, int64(2000), *secondSnapshot.FirstRowID)
}
+func TestLastUpdatedMSPresence(t *testing.T) {
+ tests := []struct {
+ name string
+ data string
+ }{
+ {name: "v1", data: ExampleTableMetadataV1},
+ {name: "v2", data: ExampleTableMetadataV2},
+ {name: "v3", data: ExampleTableMetadataV3},
+ }
+
+ for _, tt := range tests {
+ t.Run(tt.name, func(t *testing.T) {
+ var metadata map[string]any
+ require.NoError(t, json.Unmarshal([]byte(tt.data),
&metadata))
+
+ for _, key := range []string{
+ "snapshots", "snapshot-log", "metadata-log",
"current-snapshot-id",
+ "refs", "statistics", "partition-statistics",
+ } {
+ delete(metadata, key)
+ }
+
+ t.Run("missing", func(t *testing.T) {
+ metadata := maps.Clone(metadata)
+ delete(metadata, "last-updated-ms")
+ assertMissingLastUpdatedMS(t, metadata)
+ })
+
+ t.Run("null", func(t *testing.T) {
+ metadata := maps.Clone(metadata)
+ metadata["last-updated-ms"] = nil
+ assertMissingLastUpdatedMS(t, metadata)
+ })
+
+ t.Run("epoch", func(t *testing.T) {
+ metadata := maps.Clone(metadata)
+ metadata["last-updated-ms"] = float64(0)
+ raw, err := json.Marshal(metadata)
+ require.NoError(t, err)
+ parsed, err := ParseMetadataBytes(raw)
+ require.NoError(t, err)
+ assert.Zero(t, parsed.LastUpdatedMillis())
+ })
+
+ t.Run("negative", func(t *testing.T) {
+ metadata := maps.Clone(metadata)
+ metadata["last-updated-ms"] = float64(-1)
+ raw, err := json.Marshal(metadata)
+ require.NoError(t, err)
+ parsed, err := ParseMetadataBytes(raw)
+ require.NoError(t, err)
+ assert.Equal(t, int64(-1),
parsed.LastUpdatedMillis())
+ })
+
+ t.Run("wrong type", func(t *testing.T) {
+ metadata := maps.Clone(metadata)
+ metadata["last-updated-ms"] = "2024-01-01"
+ raw, err := json.Marshal(metadata)
+ require.NoError(t, err)
+ _, err = ParseMetadataBytes(raw)
+ require.ErrorIs(t, err, ErrInvalidMetadata)
+ })
+ })
+ }
+}
+
+func assertMissingLastUpdatedMS(t *testing.T, metadata map[string]any) {
+ t.Helper()
+
+ raw, err := json.Marshal(metadata)
+ require.NoError(t, err)
+ _, err = ParseMetadataBytes(raw)
+ require.ErrorIs(t, err, ErrInvalidMetadata)
+ assert.ErrorContains(t, err, "last-updated-ms is absent or null")
+}
+
func TestMetadataEqualsIncludesStatistics(t *testing.T) {
builder := builderWithoutChanges(2)
base, err := builder.Build()
@@ -1422,6 +1498,7 @@ func TestTableMetadataV2MissingSchemas(t *testing.T) {
func TestAssignMissingPartitionFieldIDsAcrossSpecs(t *testing.T) {
input := []byte(`{
"format-version": 2,
+ "last-updated-ms": 0,
"last-partition-id": 1003,
"partition-specs": [
{
