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


##########
table/updates.go:
##########
@@ -140,6 +188,9 @@ func (u *Updates) UnmarshalJSON(data []byte) error {
                default:

Review Comment:
   Non-blocking: Consider validating a missing or null `action` explicitly 
before this switch. Both currently become `unknown update action`, while a 
field-specific diagnostic would match the clarity introduced for the action 
payloads.



##########
table/updates_test.go:
##########
@@ -564,6 +564,53 @@ func TestUnmarshalUpdatesReplacesExistingSlice(t 
*testing.T) {
        assert.Empty(t, updates)
 }
 
+func TestUnmarshalUpdatesRejectsMissingRequiredFields(t *testing.T) {

Review Comment:
   Non-blocking: Consider independently omitting `type` and `snapshot-id` from 
`set-snapshot-ref`, covering null scalar/map/array fields, and confirming 
explicitly empty collections remain valid. Positive cases for `add-schema` 
without `last-column-id` and `set-snapshot-ref` without retention fields, plus 
receiver preservation after required-field failure, would complete the contract.



-- 
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