Write deletion vectors as valid Puffin files (add file magic + footer)

[raghav-reglobe]

Problem

IcebergDelete::WriteDeletionVectorFile writes a deletion vector by serializing the
deletion-vector-v1 blob and writing only that blob to the file, while labelling it
file_format = puffin with content_offset = 0 and file_size_bytes == blob size.

That is not a valid Puffin file. The Puffin spec
requires the layout Magic Blob₁ … Blobₙ Footer, with the 4-byte magic
0x50 0x46 0x41 0x31 (PFA1) at the start of the file and a Footer
(Magic, FooterPayload JSON, FooterPayloadSize, Flags, Magic) at the end.
The file produced today has neither the leading magic nor the footer — it is a bare blob.

It works in practice only because every reader (duckdb-iceberg included) locates the
vector via the manifest's content_offset / content_size_in_bytes and never parses the
Puffin container. A spec-compliant Puffin reader — or any tool that enumerates blobs via
the footer, validates the file standalone, or lacks the manifest offsets — cannot read
these files.

Fix

Add IcebergDeletionVectorData::ToPuffinFile, which wraps the blob in a proper container:

Magic(PFA1) | Blob (at offset 4) | Footer{ Magic | FooterPayload(JSON) | FooterPayloadSize(LE i32) | Flags(4) | Magic }

The FooterPayload is a FileMetadata describing the single deletion-vector-v1 blob.
Per the spec, for a deletion vector snapshot-id and sequence-number are -1, the blob
carries the required referenced-data-file and cardinality properties, and it is
uncompressed (no compression-codec, flags = 0).

content_offset is now 4 (the blob begins right after the leading magic). The read path
is unchanged — it is driven by the manifest's content_offset, so existing deletion-vector
files written with content_offset = 0 keep reading correctly. No migration needed.

Validation

• Footer framing (leading/footer/trailing magic, little-endian FooterPayloadSize, flags)
  is byte-identical to an Iceberg-Java-generated Puffin file
  (empty-puffin-uncompressed.bin): PFA1 | … | <size LE> | 0x00000000 | PFA1. The blob
  metadata field order matches Iceberg-Java
  (type, fields, snapshot-id, sequence-number, offset, length, properties).
• The deletion-vector-v1 blob is unchanged (ToBlob), so it remains byte-identical to
  the Iceberg reference vector.
• Both a strict footer-driven reader and the manifest-offset reader recover the same blob.

Testing

test/.../delete/test_deletion_vector_is_valid_puffin.test: after a v3 DELETE, resolves
the deletion-vector file via iceberg_metadata (content = 'POSITION_DELETES') and asserts
the file begins and ends with PFA1 — i.e. it is a valid Puffin container, not a bare
blob. Skipped on Nessie (v3 fixture limitation, like the other v3 delete tests).

[Tishj]

You'll need to resurrect this to get the read path working and actually verify Puffin correctness there: 123b14a

[Tishj]

My two cents is that they shouldn't use "puffin" for this, they even add content_offset and content_size_in_bytes to make it readable without parsing any of the puffin metadata bloat surrounding the relevant bytes. But here we are.

The only reason we add the .puffin suffix is because I was scared that other readers would outright reject our deletion vectors, as the main way to recognize a deletion vector currently is by checking the file format for "puffin". (presence of content_offset and content_size_in_bytes could be seen as indicators as well, maybe)

[raghav-reglobe]

Yeah, fair point — since reads jump straight to the blob via content_offset/content_size_in_bytes, the Puffin wrapper is mostly ceremony for anything going through the manifest. I'm not thrilled about it either, but as long as we're labelling these .puffin, it seemed worth making them actually valid.

What pushed me here is that not every reader goes through those offsets. pyiceberg's deletion-vector reader, for example, validates the file as a standalone Puffin and chokes on the bare blob with Incorrect magic bytes expected PFA1 — so anything that reads it as a real Puffin (or just doesn't have the manifest offsets on hand) breaks.

And good call on the test — checking the first/last 4 bytes wasn't really proving much. I've reworked it to actually parse the file as a Puffin: leading + footer-start + trailing magic, parse the footer FileMetadata JSON, then pull the deletion-vector-v1 blob back out using the footer's own offset/length (and confirm the 0xD1D33964 magic). No production read-path code for it — it just exercises the container in the test.

Tangentially related: while validating this I noticed delete manifests get written with content="data" in their Avro metadata, which trips the same kind of strict reader. Opened #1097 for that.

[raghav-reglobe]

@Tishj let me know, how we can take this forward.

[Tishj]

This is missing verification in the read path, even if it's just in an #ifdef DEBUG block, I would like there to be code that verifies that the puffin file is correct when scanning, which is why I sent that commit and saying it should be resurrected

EDIT:
Actually, let's not only do this in the debug path
I think it's good to also verify the footer of the puffin file to make sure that the payload is uncompressed, as is required by the spec

[raghav-reglobe]

Done — pushed a commit resurrecting the footer parse as a verification step in the read path (ScanPuffinFile):

• Always-on: validates the footer's trailing magic and that the FooterPayload is uncompressed (per your edit — so a reader walking the footer can parse the FileMetadata without a decompressor).
• #ifdef DEBUG: parses the whole container (leading/footer magic + the FileMetadata) and asserts the single blob's offset/length agree with the manifest content_offset/content_size_in_bytes used by the fast path.

It reuses the already-open CachingFileHandle (one small extra footer read), so the blob fast-path via the offsets is untouched. Verified with a debug build + the catalog tests are green.