tanmayrauth commented on code in PR #1762:
URL: https://github.com/apache/iceberg-go/pull/1762#discussion_r3763682824
##########
manifest.go:
##########
@@ -999,17 +999,66 @@ func ReadManifest(m ManifestFile, f io.Reader,
discardDeleted bool) ([]ManifestE
return results, nil
}
+// Manifest-list entry field IDs used to infer the format version from an
+// embedded writer schema. Per the Iceberg spec, content and sequence_number
+// are required fields for v2+ manifest lists and first_row_id is a v3 field;
+// none of them exist in v1.
+const (
+ fieldIDManifestSequenceNumber = 515
+ fieldIDManifestContent = 517
+ fieldIDManifestFirstRowID = 520
+)
+
+// schemaFieldID returns the Iceberg "field-id" property of an Avro schema
+// field, if present. Props hold decoded JSON, so numbers arrive as float64.
+func schemaFieldID(f avro.SchemaField) (int, bool) {
+ if id, ok := f.Props["field-id"].(float64); ok {
Review Comment:
The name fallback here answers the open question from the issue (writers
that don't annotate ids), and it's the right call to keep since DuckDB does
embed
them, so it's just belt-and-suspenders. One robustness gap though: this
only accepts `float64`, while the existing field-id reader in this same file
(`manifest.go:476`, `switch v := field.Props["field-id"].(type)`) handles both
`int` and `float64`. For the read path here the writer schema is JSON-parsed
from the OCF header, so numbers come through as `float64` and the DuckDB
fixture proves it works — no live bug. But the two field-id readers now
disagree on accepted types, and that sibling `int` case exists because an
in-memory-built schema yields `int` props. If inference is ever handed such a
schema, `schemaFieldID` returns false and silently degrades to name-matching,
and a writer that annotates only field-ids (non-spec names) then misfires back
to the v1 fallback this PR removes. Worth mirroring the sibling type switch to
accept `int` too — ideally one shared helper both sites call so they can't
drift again.
--
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]
