laskoviymishka commented on code in PR #1060:
URL: https://github.com/apache/iceberg-go/pull/1060#discussion_r3237293569
##########
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:
Good catch, refactored this to `ParseUint`.
For `"-1"`, the leading `-` now fails as invalid syntax before any value is
parsed, so non-numeric and negative values go through the same error path. No
separate negative check needed.
I also cleaned up `dv/deletion_vector_test.go`: the old reader-side
“negative cardinality rejected” and “malformed property rejected” subtests
can’t build their fixtures anymore, because the writer now rejects them at
`AddBlob` time. I removed those and left a pointer to the writer-side coverage
in `puffin_test.go`.
The reader-side checks in `blobCardinality` still stay, since we can still
hit malformed files written by third-party writers.
--
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]
