feat(writer): add PositionDeleteFileWriter

[laskoviymishka]

Which issue does this PR close?

Closes #340.

What changes are included in this PR?

This adds PositionDeleteFileWriter under writer/base_writer, next to the data
and equality-delete writers. We already had a writer for equality deletes but
nothing that writes position delete files, so I modelled this one on
equality_delete_writer.rs.

It writes the two required columns, file_path (string, field id 2147483546) and
pos (long, 2147483545), and sets the content type to PositionDeletes on close.
position_delete_schema() / position_delete_arrow_schema() hand back the
canonical schema, built from the field definitions already in metadata_columns
so the reserved ids aren't duplicated here.

write() checks the batch is exactly those two required, non-null, correctly
typed columns before handing it to the parquet writer, so a bad batch fails with a
clear message instead of a confusing error further down. close() carries the
partition key onto the data file and leaves sort_order_id null.

One thing worth calling out: this writer doesn't sort its input. Position deletes
have to be sorted by file_path then pos, and for now that's on the caller (the
write() docs say so). I'd rather land the plain writer first and put sorting on
top of it, so a sorting writer, referenced_data_file, and a higher-level
DeltaWriter (#2218) are follow-ups instead of being crammed in here.

Are these changes tested?

Yes, 12 unit tests: the schema shape, a parquet round trip for single and multiple
writes (write it, read it back, compare), partition and spec-id propagation on
close, and the rejection cases: wrong column count, wrong or missing field ids on
either column, wrong types (including LargeUtf8, which is what a lot of Arrow
producers give you), nullable columns, and writing after close. cargo test -p iceberg, clippy, and fmt are all clean.

AI assistance disclosure

Drafted with help from Claude Code, then reviewed and tested by me before opening.

[anoopj]

I took a pass. Overall, the implementation looks mostly good to me. Some feedback:

[anoopj]

Could we add a note that position delete files are a V2 construct? Per the spec they are deprecated in v3 and writers must not add new position delete files to v3 tables because v3s add DVs instead.. This base writer correctly has no format-version gate, so gating belongs at the transaction/commit layer.

Consider adding something like "position delete files apply to v2 tables; v3 replaces them with deletion vectors, so callers must not route v3 writes here".

[anoopj]

Can we add a test/assertion that reads the written file's parquet schema and asserts the two column ids?

[anoopj]

Can this be pub crate instead? position_delete_schema needs to be public though

[anoopj]

Consider adding test coverage for this?

[laskoviymishka]

Thanks for the review! Pushed a follow-up commit addressing all four:

• Added a note to the module docs that position delete files are a v2 construct — v3 replaces them with deletion vectors and forbids new position delete files, so callers must not route v3 writes here, and format-version gating stays at the transaction/commit layer rather than this base writer.
• Made position_delete_arrow_schema crate-internal. Since only the tests consume it today, I gated it (and its backing static) under #[cfg(test)] to avoid a dead-code warning; I'll widen it to pub(crate) when a higher-level writer actually needs the Arrow form. position_delete_schema stays public.
• Added a test that reads the written file's Parquet schema and asserts the two reserved column field ids (2147483546 / 2147483545) survive the write.
• Added a test for the field_id parse-error branch (metadata present but not an integer).

Also merged latest main in. Let me know if you'd prefer position_delete_arrow_schema kept as pub(crate) regardless — happy to switch.

[anoopj]

LGTM. There is a CI failure though:

  error: `unparseable` should be `unparsable`
      ╭▸ ./crates/iceberg/src/writer/base_writer/position_delete_writer.rs:593:50
      │
  593 │     async fn test_position_delete_writer_rejects_unparseable_field_id() -> Result<()> {

It's a bit weird that the checker thinks unparseable is a typo. (both are correct forms)