zeroshade commented on code in PR #1060:
URL: https://github.com/apache/iceberg-go/pull/1060#discussion_r3228271081
##########
puffin/puffin_writer.go:
##########
@@ -137,6 +138,28 @@ func (w *Writer) AddBlob(input BlobMetadataInput, data
[]byte) (BlobMetadata, er
if input.SequenceNumber != -1 {
return BlobMetadata{}, errors.New("puffin:
deletion-vector-v1 requires sequence-number to be -1")
}
+ // Properties cardinality and referenced-data-file are
spec-mandated
+ // for deletion-vector-v1; the reader in table/dv does not
enforce
+ // them today, so we hold the line here to keep Go-emitted files
+ // always conformant.
+ if input.Properties["cardinality"] == "" {
+ return BlobMetadata{}, errors.New("puffin:
deletion-vector-v1 requires a cardinality property")
+ }
+ // Reject non-numeric or negative values at write time too —
+ // otherwise a writer could emit "cardinality": "abc" that the
+ // reader hard-rejects when it parses the property. Symmetric
+ // with strconv.ParseInt + non-negative check on the read side.
+ card, err := strconv.ParseInt(input.Properties["cardinality"],
10, 64)
Review Comment:
Could get two birds with one stone by using `ParseUint` instead which would
error on a negative value. So you could just do something like
```go
if _, err := strconv.ParseUint(input.Properties["cardinality"], 10, 64); err
!= nil {
// got something other than a non-negative integer
}
```
--
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]
