zeroshade commented on code in PR #1059:
URL: https://github.com/apache/iceberg-go/pull/1059#discussion_r3228242761


##########
table/metadata.go:
##########
@@ -2006,6 +2007,145 @@ func (m *metadataV3) UnmarshalJSON(b []byte) error {
        return m.validate()
 }
 
+// MarshalJSON serializes v3 metadata with the spec-mandated emission of
+// `current-snapshot-id: null` for empty tables, rather than dropping the
+// key via the underlying `omitempty` tag. Java reference behaviour per
+// apache/iceberg#12335; v1/v2 keep the omitempty path since their fixtures
+// use the -1 sentinel for "no current snapshot".
+//
+// `last-partition-id` is required for v2+ per spec — the existing parse-
+// time validation enforces this on read; the symmetric check below
+// rejects writes of malformed in-memory state (builder/parser paths
+// always populate it, so this only fires when code constructs metadataV3
+// directly and skips the builder).
+func (m *metadataV3) MarshalJSON() ([]byte, error) {
+       if m.LastPartitionID == nil {
+               return nil, fmt.Errorf("%w: last-partition-id must be set for 
v3 metadata", ErrInvalidMetadata)
+       }
+
+       type Alias metadataV3

Review Comment:
   Rather than decoding and injecting, the preferred way to do this would be 
for Alias to embed metadataV3 and then overwrite the field definition to remove 
the omitempty etc. That would be more efficient and avoid having to decode what 
we just encoded. 
   
   i.e. something like
   
   ```go
   alias := &struct {
       *metadataV3
       CurrentSnapshotID *int64 `json:"current-snapshot-id"`
   }{ m, m.CurrentSnapshotID }
   ```
   
   That should do what you're looking for without needing the extra 
decode/encode/injection etc.
   
   You can see this pattern in other MarshalJSON functions we have defined



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to