feat(arrow): read manifest entries into Arrow with arrow-avro

[mbutrovich]

Which issue does this PR close?

• Part of [EPIC] Fast manifest reads for PyIceberg through pyiceberg-core #3262 (Task 3).

What changes are included in this PR?

Adds iceberg::arrow::read_manifest_entries, which decodes a manifest file into a RecordBatch with arrow-avro, so pyiceberg-core can return Arrow instead of one Python object per entry. Manifest lists and the binding follow in separate PRs.

• The schema is the spec's manifest_entry with the latest data_file fields and their field IDs, so every format version reads into the same schema apart from data_file.partition.
• Fields are matched to the manifest's Avro schema by field ID. v1 manifests read with the spec's v2+ read behavior, and null snapshot IDs, sequence numbers, and first row IDs are inherited from the manifest's ManifestFile.
• arrow-avro 59 can't decode a record with no fields, which every unpartitioned manifest has (fixed in fix(arrow-avro): decode records with no fields arrow-rs#10771). Until Bump to Arrow 60 #3257 moves to arrow 60, the reader leaves the empty partition out of the Avro reader schema and builds it itself.

On a 10,000-entry PyIceberg manifest with 64 KB blocks, it takes 11.6 ms against 37.2 ms for Manifest::parse_avro, which stops at Rust structs, in a release build on an Apple M5 Max.

Are these changes tested?

Yes, with unit tests in arrow/manifest.rs and a doctest.

AI Disclosure

I wrote this PR with help from Claude but understand and support the core approach, implementation, and test coverage.

[comphead]

Tests, clippy and the public-api.txt snapshot pass on the PR head, and first row ID inheritance matches Java's ManifestReader.idAssigner and the in-repo assign_first_row_ids.

Beyond the inline comments, arrow-avro is built with snappy and zstd, but the only external fixture is deflate. One small test per codec would cover the feature flags. I did not reproduce the benchmark numbers.

[comphead]

Sequence number inheritance only fills ADDED rows. Java's InheritableMetadataFactory and ManifestEntry::inherit_data also fill any null when the manifest list entry's sequence number is 0 (sequenceNumber == 0 || status == ADDED). Here, a v2-layout manifest listed with sequence number 0 returns nulls for EXISTING and DELETED entries, where the Manifest path returns 0. Adding || manifest_file.sequence_number == INITIAL_SEQUENCE_NUMBER to the mask would match.

The ADDED-only restriction is also untested. Replacing Some(&added) with None here leaves all 11 tests green (I ran it), because test_snapshot_id_and_sequence_number_inheritance only uses an EXISTING entry with explicit values. Could you add an EXISTING entry with null sequence numbers that must stay null (add_existing_entry, as in first_row_id_manifest), plus the sequence number 0 case?

[mbutrovich]

Sequence number inheritance only fills ADDED rows. Java's InheritableMetadataFactory and ManifestEntry::inherit_data also fill any null when the manifest list entry's sequence number is 0 (sequenceNumber == 0 || status == ADDED). Here, a v2-layout manifest listed with sequence number 0 returns nulls for EXISTING and DELETED entries, where the Manifest path returns 0. Adding || manifest_file.sequence_number == INITIAL_SEQUENCE_NUMBER to the mask would match.

I'd like to keep the ADDED-only mask, because it follows the spec's sequence number inheritance rules. The manifest_entry fields say both sequence numbers are "Inherited when null and status is 1 (added)", and the entry rules say "If the entry status is 0 (existing) or 2 (deleted), the entry must include both sequence numbers explicitly." The spec asks for 0 only when reading v1 manifests: "When reading v1 manifests with no sequence number column, sequence numbers for all files must default to 0." This reader fills the missing column with 0 in that case, and test_v1_manifest_reads_with_v2_defaults covers it. A v2-layout manifest with a null sequence number on an existing or deleted entry doesn't follow the spec, so I'd rather not add Java's fallback for it here. Java's V2Metadata and PyIceberg's ManifestWriterV2 both reject a null sequence number on a non-added entry, so I don't expect these manifests from either writer. Do you know of another writer that produces them?

The ADDED-only restriction is also untested. Replacing Some(&added) with None here leaves all 11 tests green (I ran it), because test_snapshot_id_and_sequence_number_inheritance only uses an EXISTING entry with explicit values. Could you add an EXISTING entry with null sequence numbers that must stay null (add_existing_entry, as in first_row_id_manifest), plus the sequence number 0 case?

Thanks, I added it. test_snapshot_id_and_sequence_number_inheritance now has an added entry that inherits, a deleted entry with explicit values, and an existing entry whose null sequence numbers stay null. ManifestWriter rejects an existing entry without sequence numbers, including through add_existing_entry, so the test nulls them with rewrite_manifest. I left out the sequence number 0 case for the reason above.

[mbutrovich]

@comphead, thanks for the review.

Beyond the inline comments, arrow-avro is built with snappy and zstd, but the only external fixture is deflate. One small test per codec would cover the feature flags.

I added test_compressed_manifests, which re-encodes a manifest with deflate, snappy and zstd and compares each read with the read of the uncompressed manifest. ManifestWriter writes uncompressed manifests, so the test re-encodes with apache-avro instead of adding fixture files.