fix(scan): validate deletion vector delete tasks

[linhongyu510]

Summary

• validate deletion vector invariants when FileScanTaskDeleteFile is built
• migrate delete-file fixtures and callers to the validated builder result shape
• remove duplicate single-task deletion-vector structural checks from the loader and delete-file index

Testing

• cargo test -p iceberg scan::task::tests::test_delete_file_builder --lib
• cargo test -p iceberg --lib delete_file_loader
• cargo test -p iceberg --lib row_filter
• cargo test -p iceberg --lib caching_delete_file_loader
• cargo test -p iceberg --lib delete_file_index
• cargo test -p iceberg --lib
• cargo clippy -p iceberg --all-targets --all-features -- -D warnings
• cargo fmt --all -- --check

Closes #3135.

[linhongyu510]

CI follow-up pushed in 8ff9d12: removed the internal design note that failed the Apache license-header check, made the helper private, and updated the validated builder public API baseline. Verified locally with 15 targeted scan tests, rustfmt, clippy -D warnings, LICENSE/NOTICE checks, and cargo-public-api 0.51.0 output comparison.

[linhongyu510]

Addressed the review in 0228cd4: all FileScanTaskDeleteFile fields are private, with read-only getters used by internal callers; the public API baseline is updated. Verified 66 focused tests, workspace fmt, Clippy with -D warnings, public API checks, and git diff --check.

[blackmwk]

1. I think this check should also apply to non deletion vector?
2. Why not make offset usize?

[blackmwk]

See above

[blackmwk]

I think this should also apply to non deletion vector? Also why not remove the Option

[mbutrovich]

I noticed this PR after opening #3180. I'll leave mine open for now as a reference, but we can try to get #3145 over the finish line first.

The two converge on the same mechanism (build_method(into = Result<..>) + validate() + is_deletion_vector()), so the differences are small. Two things that might be worth pulling across either way:

• The #3035 naming nit (r3919759548), renaming pos_deletes_by_path to pos_deletes_by_referenced_data_file, isn't covered here. It's independent of #3135 and could go separately.
• Making the fields private is a bigger breaking change for downstream than the validation itself. Would it be worth calling out explicitly in the description, so it shows up in the 0.11 release notes?

[mbutrovich]

I may be misreading the ordering here, but is the "validated" premise true at this point? PopulatedDeleteFileIndex::new consumes DeleteFileContext { manifest_entry, .. } fed straight from manifest scanning, and the conversion to FileScanTaskDeleteFile (and so build()) happens later, in get_deletes_for_data_file. If that's right, a manifest with a Puffin position-delete entry missing referenced_data_file would panic here, where it previously returned DataInvalid.

Relatedly, test_deletion_vector_missing_referenced_data_file_is_rejected is removed in this PR. Was that deliberate, or fallout from the check moving?

One option: keep this one check returning DataInvalid, since the index needs the value as its map key and so can't defer it, and move only content_offset / content_size_in_bytes to the builder. Curious whether you considered that and preferred the expect.

[mbutrovich]

Same question about the guarantee, for a different reason: FileScanTaskDeleteFile derives plain Deserialize with no #[serde(try_from = ..)], unlike FileScanTask, which has one. So a task rebuilt from a serialized scan plan, the planner to worker path the key_metadata doc comment describes, never passes through build(). Does that leave these expects reachable on a malformed or hand-written plan?

Since the function already returns Result, keeping ok_or_else(..) here would cost little and degrade to DataInvalid instead of a panic. Alternatively, adding #[serde(try_from = ..)] to route deserialization through the same validation would make the expects sound, which seems like the stronger fix if it's not too much scope for this PR.