Skip to content

feat(scan): apply Iceberg v3 deletion vectors on read - #77

Closed
antonireus wants to merge 2 commits into
mainfrom
feat/deletion-vector-application
Closed

feat(scan): apply Iceberg v3 deletion vectors on read#77
antonireus wants to merge 2 commits into
mainfrom
feat/deletion-vector-application

Conversation

@antonireus

Copy link
Copy Markdown
Collaborator

Summary

Applies Apache Iceberg v3 deletion vectors (deletion-vector-v1 roaring-bitmap Puffin blobs) on read, closing the deletion-vector half of #11 (Closes #12). Databricks-UniForm v3 tables and v2→v3-migrated tables now return correct post-delete rows instead of silently pre-delete rows.

A decoded DV feeds the same RowSelection/ParquetAccessPlan union point the positional-delete path uses, so DVs compose with projection/filter/LIMIT/pruning for free, and a shard may mix positional-delete-backed and DV-backed data files.

Follow-up to #72 (positional deletes); reconciled onto main after #71 (broadcast join) merged.

What changed

  • deletion-vector-v1 decoder (scan/deletion_vectors.rs): BE length + D1 D3 39 64 magic + portable roaring vector + BE CRC-32, validated against the declared cardinality. Fails loud (credential-redacted) on any magic/CRC/cardinality/truncation mismatch. Uses the existing roaring dep + crc32fast (verified as the correct CRC-32/ISO-HDLC variant).
  • Normalized interned per-shard wire {deleteFiles, dataFiles} (scan/spec.rs): each physical delete file/container interned once per shard; df-indexed refs carry optional offset/length for blob-addressed DVs. Retires FileEntryWire/DeleteFileRef/DeleteFileContentType. JoinSpec.files migrated to the same shape (reconciliation with JOIN pushdown Phase 1: broadcast inner equi-join (BL-001) #71's broadcast-join block).
  • Adapter DV producer (adapter/pushdown.rs): manifest walk sources DV coordinates (plan_files drops them); classify_manifest_file accepts Puffin position deletes, still rejects equality/ORC/Avro; fails loud when a DV references a data file matching no snapshot file.
  • Scan-side application (scan/puffin.rs, positional_deletes.rs): per-data-file dispatch on delete type; Puffin container opened once per shard and reused; blob's referenced-data-file cross-checked against the applied data file.
  • Fixtures + E2E (scripts/spark-fixtures/, tests/): positive mor_dv, mixed positional+DV mor_mixed, ORC-unsupported fixture; new deletion-vector E2E matrix; positional fail-loud test retargeted to ORC.
  • Bumps lakehouse-engine 0.24.0 → 0.25.0.

A note on the interesting bug

Host tests passed but E2E against a real Spark v3 fixture caught a genuine integration bug: iceberg-rust 0.10 does surface the Puffin DV file in FileScanTask.deletes (the plan assumed it didn't), so the container was emitted as both a mis-typed POS_DEL (Parquet) ref and a DV ref — the POS_DEL ref then tried to parse the Puffin footer as Parquet ("Corrupt footer"). Fixed by excluding manifest-collected DV container paths from the positional refs; the manifest walk stays authoritative for DVs. (FileScanTaskDeleteFile exposes no file-format field, so path-exclusion is the robust fix.)

Verification

  • make cross-musl-udf-build → exit 0 (release .so, v0.25.0)
  • cargo test → 457 lib + all integration, 0 failures
  • cargo clippy --all-targets + cargo fmt --check → clean
  • make test-e2e (live Exasol + MinIO + Iceberg REST + Spark) → 83 passed, 0 failed (capability 8, count_distinct 6, deletion_vectors 9, join 6, positional_deletes 11, scan 43)
  • Code review: no correctness bugs; 5 quality/safety findings + 1 E2E-caught bug fixed.

Spec deltas recorded into the permanent library (spec(record) commit); ADR-092..096 promoted.

🤖 Generated with Claude Code

antonireus and others added 2 commits July 7, 2026 16:53
Decode deletion-vector-v1 Puffin blobs (roaring bitmap) and feed the
decoded positions into the same RowSelection/ParquetAccessPlan union point
the positional-delete path uses, so DVs compose with projection/filter/
LIMIT/pruning identically. Databricks-UniForm v3 tables now return correct
post-delete rows instead of silently pre-delete rows.

- New deletion-vector-v1 decoder (BE length/magic/CRC, portable roaring,
  cardinality validation, fail-loud redacted errors).
- Normalized interned per-shard wire {deleteFiles, dataFiles}: each physical
  delete file/container interned once, df-indexed refs carry optional
  offset/length for blob-addressed DVs. Retires FileEntryWire/DeleteFileRef/
  DeleteFileContentType. JoinSpec.files reuses the same shape.
- Manifest walk sources DV coordinates (plan_files drops them); classify
  now accepts Puffin position deletes, still rejects equality/ORC/Avro.
- Per-data-file dispatch on delete type; Puffin container opened once per
  shard and reused across its data files; referenced-data-file cross-checked.
- Exclude DV Puffin containers from positional refs: iceberg-rust 0.10 also
  surfaces the DV file in FileScanTask.deletes, so the manifest walk stays
  authoritative and the container is applied once (as a DV), not twice.
- Fail loud when a DV references a data file matching no snapshot file.
- Spark fixtures: positive mor_dv, mixed positional+DV mor_mixed, ORC
  unsupported; new deletion-vector E2E matrix; retarget positional fail-loud
  test to ORC.

Bumps lakehouse-engine to 0.25.0.

Closes #12

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Merge the 7 feature deltas into the permanent spec library and archive the
plan to specs/_recorded/2026-07-07-add-deletion-vector-application.

- NEW: datafusion-scan/scan-execution-deletion-vectors,
  packaging/deletion-vector-fixtures, packaging/e2e-harness-deletion-vectors
- CHANGED: vs-adapter/pushdown-file-pruning,
  datafusion-scan/scan-execution-spec-reconstitution,
  datafusion-scan/scan-execution-positional-deletes,
  packaging/e2e-harness-positional-deletes
- Promoted ADR-092..096 (DV decoder, shared union point, manifest-sourced DV
  refs incl. the FileScanTask.deletes E2E correction, fail-loud validation,
  normalized interned wire).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@antonireus
antonireus marked this pull request as draft July 7, 2026 22:01
@antonireus

Copy link
Copy Markdown
Collaborator Author

Review synthesis — deletion-vector planning & format detection

Focused review of how the planning phase acquires positional deletes + DVs and how format is
detected
. Overall the design is clean and unusually well-documented; the items below are recorded
for follow-up, not merge blockers. Detailed analysis of the performance item (#4) is in the next
comment.

How it works today

  • Two sources for the two mechanisms. Parquet positional deletes come from plan_files()
    (FileScanTask.deletes, in plan_files_from_table). v3 deletion vectors come from a separate
    raw manifest walk
    (gate_and_collect_deletion_vectors), because plan_files drops the Puffin
    discriminator and the DV blob coordinates.
  • Format detection is at the manifest level (classify_manifest_file) — a 3×4
    DataContentType × DataFileFormat match. (PositionDeletes, Puffin) ⇒ DV; every unsupported
    (content, format) combination fails loud before any scan SQL is built. This is the correct seam:
    by the time you reach plan_files, a Puffin DV is indistinguishable from a Parquet positional
    delete.
  • The two sources are reconciled: attach_deletion_vectors unions DVs onto the planned data
    files; positional_delete_refs excludes DV container paths (the iceberg-0.10 "surfaced in both"
    bug).

Findings (ranked)

  1. (correctness risk) Silent DV-drop hinges on an undocumented cross-API path-form invariant.
    attach_deletion_vectors has a "pruned ⇒ skip silently" branch keyed on all_data_files. That
    set and the DV referenced_data_file both come from the manifest walk (DataFile.file_path()),
    while the planned files come from plan_files (FileScanTask.data_file_path()). If those two
    iceberg APIs ever emit the same file in different path forms (e.g. s3:// vs s3a://), a
    scanned file's DV would fail the files lookup, match all_data_files, and be silently
    dropped ⇒ pre-delete rows. Note the asymmetry: the unmatched case fails loud, but the pruned
    case trusts path forms silently. E2E is green today, so the forms align in practice — but nothing
    enforces or documents the invariant, and this PR exists precisely because these two APIs disagree
    about DV files.
  2. (maintainability) ~400 lines of cohesive delete-planning logic live inside a ~13.9k-line
    pushdown.rs.
    UnsupportedDeleteMechanism, classify_manifest_file,
    gate_and_collect_deletion_vectors, extract_deletion_vector_ref, attach_deletion_vectors,
    positional_delete_refs, plan_files_from_table, DeletionVectorRef form a self-contained unit
    (in: Table + filter; out: Vec<FileEntry>) that belongs in its own module
    (adapter/delete_planning.rs). Pure move, no logic change. Highest-leverage cleanup.
  3. (robustness, low) The DV-container path exclusion has no guard, unlike its sibling.
    positional_delete_refs drops DV containers by exact path match; a miss (same path-form
    fragility as GROUP BY pushdown + vs-expression SQL-comprehension crate (0.4.0) #1) reintroduces the "Corrupt footer" crash. Self-announcing (crash, not silent
    wrong data), so lower severity — but inconsistent with attach_deletion_vectors, which fails
    loud. Same root cause as GROUP BY pushdown + vs-expression SQL-comprehension crate (0.4.0) #1.
  4. (performance) Double manifest traversal per query. The gate reads the manifest list + every
    manifest; plan_files then reads the manifest list + manifests again. Two S3 metadata passes,
    worst for the common no-deletes case. See the dedicated comment below.
  5. (minor) The support matrix is encoded twiceclassify_manifest_file (plan-time) and
    ensure_applicable_delete (read-time) hand-synced. Intentional defense-in-depth; just note a
    future third mechanism must be added in both places.

@antonireus

Copy link
Copy Markdown
Collaborator Author

Deep dive: the double manifest read (finding #4) — problem, root cause, directions

Recording the exploration so the analysis isn't lost. No decision made yet — this lays out the
problem, why it exists in iceberg-rust 0.10, and the possible directions with their trade-offs.

Problem

Planning does two metadata passes:

  1. gate_and_collect_deletion_vectors reads the manifest list and loads every manifest
    (unpruned) — to fail loud on unsupported mechanisms and to extract DV coordinates.
  2. plan_files_from_table independently reads the manifest list and manifests again (pruned) —
    for data files + positional-delete association.

Two S3 metadata passes per query, in tension with the "resolve metadata once per query" rule. It's
worst for the common no-deletes case, where the gate still walks every data manifest only to
learn "all Parquet, no deletes."

Root cause (iceberg-rust 0.10, tag v0.10.0-rc.2)

FileScanTask does expose data_file_format, and FileScanTaskDeleteFile does expose
file_type (content type) — but DeleteFileContext::from (crates/iceberg/src/scan/task.rs) keeps
only 5 fields and drops file_format, content_offset, content_size_in_bytes, and
referenced_data_file. Those four are exactly what DVs need, and they survive only at the manifest
DataFile level. Also, Table::object_cache() and ObjectCache::get_manifest are pub(crate), so
we can't route our gate walk through iceberg's Moka cache to make the second read cheap — the double
read is two genuine S3 fetches.

Secondary observation (scan side)

The scan already carries offset/length to the UDF but still reads the Puffin footer via
PuffinReader (file_metadata() + blob()), using the coordinates only to pick a blob out of
the parsed footer, not to avoid it. So today we pay for both the planning manifest walk and
a scan-side footer read, and fully exploit neither shortcut.

Possible directions

A) In-repo, planning: delete-manifest-only gate.
ManifestFile.content is readable straight off the manifest list; DVs (and all delete files) live
only in delete-content manifests. Skip data manifests in the gate, and move non-Parquet data-file
rejection to the plan_files task mapping (FileScanTask.data_file_format).

  • No-deletes tables: gate reads the manifest list only, loads nothing — the extra pass
    effectively disappears.
  • DV/positional tables: gate reads only the (few) delete manifests.
  • Interacts with finding GROUP BY pushdown + vs-expression SQL-comprehension crate (0.4.0) #1: it drops all_data_files, so replace the pruned-vs-mismatch DV
    distinction with filter-presence reasoning (filter_json.is_none() ⇒ no pruning ⇒ unmatched DV
    must fail loud; Some ⇒ pruning active ⇒ skip). Arguably tighter than all_data_files, which
    (same manifest source as referenced_data_file) can't detect the path-form mismatch GROUP BY pushdown + vs-expression SQL-comprehension crate (0.4.0) #1 is about.

B) In-repo, scan: direct-access read (footer-free).
Carry cardinality (DataFile.record_count()) on the wire — or drop the cardinality cross-check and
rely on CRC + magic — then read the blob with a ranged FileRead::read(offset..offset+length) and
decode, skipping PuffinReader/footer entirely. Achievable today, independent of upstream, since
we already carry the coordinates.

C) Upstream contribution: add fields to FileScanTaskDeleteFile.

  • Minimal (file_format only): unblocks single-pass planning, but the scan must still read
    the footer (locate the blob by its footer referenced-data-file property).
  • Full (file_format + content_offset + content_size_in_bytes + referenced_data_file,
    ideally record_count): unblocks single-pass planning and footer-free scan.
    referenced_data_file is required to guard against iceberg 0.10's partition-level
    over-attachment
    (it isn't DV-aware, so one DV can attach to sibling files in the partition;
    #2681's matchDVToData is the upstream fix).

Where the four data points come from decides which levers you get:

Source of the fields Planning Scan
Manifest walk (today) double read can be footer-free (not currently exploited)
file_format only, upstream single pass must read footer
all four, upstream on FileScanTaskDeleteFile single pass footer-free direct read

This is the argument for the four-field ask over file_format-only: the coordinates aren't
needed for planning correctness, they're what makes the scan footer-free.

Upstream status (don't wait on it)

  • EPIC #2411 (v3 support) lists "DV read
    support" and "Scanner integration" as open. Umbrella: #630.
  • PR #2681 "apply V3 deletion vectors on
    read" — its commit "carry file_format + referenced_data_file" on scan/task.rs is exactly this
    fix, and it reads the blob via content_offset/content_size_in_bytes (direct access). But it
    is stacked on #2678
    #2203 (RowDelta write side), all OPEN,
    CONFLICTING, 0 reviews. PR #2414 (DV blob
    decoder, ≈ our deletion_vectors.rs) stalled since June. Realistically not landing in a pinnable
    release soon.
  • Clean future exit: when #2681 (or just its field-carrying slice) lands, we can delete our
    manifest walk and deletion_vectors.rs/puffin.rs and let plan_files do it in one pass.
    Worth // TODO(iceberg-rust#2681) markers now so the removal is signposted.

On the table (not decided)

A) the delete-manifest-only gate removes the double read for the common case now; (B) the
direct-access scan removes the footer read; (C) a narrow four-field upstream PR would collapse both
into a genuine single pass. (A) pairs naturally with the module extraction in review finding #2.

@antonireus antonireus closed this Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Read Iceberg v3 deletion vectors (Puffin)

1 participant