feat: materialize _last_updated_sequence_number metadata column

[anoopj]

Which issue does this PR close?

Refs #2879

What changes are included in this PR?

Reading the _last_updated_sequence_number metadata column is not supported currently. This PR fixes it.

Note: A file may physically carry a per-row column (e.g. written by Iceberg Java when carrying rows forward across a rewrite). The spec reads such non-null per-row values unmodified; that per-row coalesce is not implemented yet, so such files are rejected with FeatureUnsupported.

The per-row coalesce, and the _row_id column, follow in later PRs.

Are these changes tested?

unit tests

[laskoviymishka]

The primitive_type_to_arrow_type_with_ree extraction is the right call — routing both the value path and the null path through one Arrow type is exactly what keeps mixed files in a scan from producing incompatible batch schemas, and test_..._mixed_files_share_schema proves it. I also like that the per-row coalesce deferral is documented in the guard rather than left implicit.

I'd hold this before merging though. The physical-column contract this establishes — how we detect a stored _last_updated_sequence_number and what we do when we find one — is exactly what the follow-up coalesce PR builds on top of, so I'd rather settle it now than inherit it.

Main concern is the FeatureUnsupported hard-error on files that physically carry the column. Java's RewriteDataFiles writes this per-row when it carries rows forward across a rewrite (ExtractRowLineage), so a standard Java-compacted v3 table will have the physical column — and projecting it here turns a previously-readable table into a hard error, where Java's LastUpdatedSeqVectorReader just coalesces. Rejecting loudly beats silently mis-coalescing, agreed, but a hard error is still a regression relative to "the column just wasn't projected before." I'd lean toward passing the physical column through unchanged as the interim (correct for non-null rows, null rows stay null) rather than erroring, if the full coalesce is genuinely follow-up scope.

Things I'd like to settle in this PR before the follow-ups build on it:

• close the two bypass routes in the physical-column guard (positional-fallback field IDs, and the swallowed parse error) so fail-loud actually holds
• decide hard-error vs pass-through for a physically-present column, and make it explicit either way
• carry the type in ColumnConstant::Null (Null(DataType)) so the schema and column-source paths stop re-deriving it independently
• resolve (Some(first_row_id), None) deliberately — error on a malformed entry, or document that null is intentional
• assert logical column values in the tests rather than REE run counts
• reword the "spec gates both" comment to "Java gates both"

None of this is a rewrite — the structure is sound and the scoping is sensible. It's mostly making the base layer airtight so the coalesce PR has something solid to land on. Once those are settled, happy to take another pass.

[laskoviymishka]

This guard is the safety net for deferring the coalesce — but there are two ways a file that physically carries the column gets past it and silently overwritten with the derived value, which is the exact thing the guard is here to prevent.

First, .parse::<i32>().ok() swallows a malformed field-id string, so a column whose field-id metadata doesn't parse reads as absent. Second, and more likely to bite: when a Parquet file has no embedded field IDs and no name mapping, ArrowReader assigns positional fallback IDs (1, 2, 3…), which never equal i32::MAX - 108 — so a file physically carrying the column with positional IDs sails straight through. build_field_id_to_arrow_schema_map already propagates the parse error rather than .ok()-ing it, so there's a precedent to match.

I'd also match on the column name RESERVED_COL_NAME_LAST_UPDATED_SEQUENCE_NUMBER in the positional-fallback case so the guard is actually airtight before the coalesce lands on top of it. wdyt?

[laskoviymishka]

I get the intent — reject loudly rather than silently mis-coalesce — and I think that instinct beats a silent overwrite. What gives me pause is interop: Java's RewriteDataFiles writes this column per-row when it carries rows forward across a rewrite (ExtractRowLineage), so a standard Java-compacted v3 table will physically have the column, and projecting it here turns a previously-readable table into a hard error, where Java's LastUpdatedSeqVectorReader just coalesces.

A couple of ways to thread it, wdyt: implement the coalesce now (per-row non-null wins, fall back to the derived value where null), or, if that's genuinely follow-up scope, pass the physical column through unchanged instead of erroring. Pass-through is correct for the non-null rows and leaves null rows null — a visible gap, but non-destructive and readable, versus unreadable.

Not asking for the full coalesce in this PR — mainly flagging that a hard error is a regression relative to "the column just wasn't projected before," and I'd lean toward pass-through as the interim.

[laskoviymishka]

The (Some(_), None) case — first_row_id present but no data sequence number — folds into the null gate here, and I see the downstream test pins that deliberately. After manifest inheritance a committed entry should always have a sequence number, so that state reads to me more like a malformed manifest than a legitimate null.

I'd lean toward DataInvalid (or at least a tracing::warn!) there rather than a silent all-null column, but if matching Java's dual gate is the intent, a one-line note saying so is enough. wdyt?

[laskoviymishka]

Small accuracy thing on this comment: the spec text only says _last_updated_sequence_number is assigned the manifest entry's sequence number on read — it doesn't itself gate that column on first_row_id. It's Java (VectorizedArrowReader.lastUpdated returns nulls when baseRowId == null) that gates both. I'd reword to "Java gates both" so a future reader doesn't go hunting for spec text that isn't there — a link to a spec-clarification issue if one exists would be even better.

[laskoviymishka]

These assert on REE internals — values().len() == 1, a single run — rather than the logical column. A batch that happened to span two runs would fail this with no actual regression, and conversely a single-run shape doesn't prove every row got the right value.

I'd assert the logical contents instead: materialize the column and check each of the 3 rows equals the expected constant (or is null, for the null-gate tests). Same pattern shows up in the sibling tests, so it's a one-time cleanup across all four.

[laskoviymishka]

Scalar(Datum) carries type and value together, but Null carries neither — so both the schema path (record_batch_transformer.rs:531) and the column-source path (:709) independently re-derive the Arrow type via get_metadata_field + null_metadata_column_arrow_type. Two callsites deriving the same type separately is how they quietly drift apart later.

I'd carry it in the variant — Null(DataType), computed once in with_null_metadata_column and read back in both arms. That kills the duplication and makes it structurally impossible for the two paths to disagree; it also hands with_null_metadata_column the type at the callsite, which is the metadata-only precondition the name is already implying. wdyt?

[laskoviymishka]

The literal 1 here is silently coupled to the fixture's sequence number — if setup_v3_manifest_files ever writes a different sequence number, this passes or fails for a reason that has nothing to do with the code under test. I'd tie it back to the fixture explicitly (read the expected value from the manifest entry) or at least comment where the 1 comes from.