Skip to content

fix: fall back when an Iceberg equality delete is keyed on a nested field - #7122

Merged
andygrove merged 1 commit into
apache:mainfrom
yunchipang:fix/iceberg-nested-equality-delete-fallback
Oct 20, 2026
Merged

andygrove merged 1 commit into
apache:mainfrom
yunchipang:fix/iceberg-nested-equality-delete-fallback

Conversation

@yunchipang

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #6782.

Rationale for this change

If an Iceberg equality delete is keyed on a nested field such as s.k, every native scan of the table fails with field not found. iceberg-rust only looks up top-level columns. The upstream fix is apache/iceberg-rust#3453. Until Comet picks it up, this case should fall back to Spark.

What changes are included in this PR?

  • CometScanRule: fall back when an equality delete's key is not a top-level column in any of the table's schemas.
  • CometEqualityDeletes: a test helper option to key a delete on a nested field.
  • iceberg.md: list this case as a fallback instead of a known failure.

How are these changes tested?

A new test in CometIcebergNativeSuite keys a delete on s.k. It checks that the result matches Spark and that the scan falls back. It fails without the fix. The existing equality delete tests still pass.

@github-actions github-actions Bot added bug Something isn't working area:scan Parquet scan / data reading area:Iceberg labels Oct 20, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No introduced P1/P2 issues found within this review.

Reviewed the entire four-file diff at 3a2a1f593555fdb5413a6eb774f1544c5959b801 against 3355602e268819750884aa0ce0dd47002c6ea580. The PR is not a draft and contains no stacked prerequisites. The snapshot and live discussion checks contained no reviews, issue comments, inline comments, or review threads.

Summary

  • Prior state and problem: A primitive equality-delete key nested inside a struct passed Comet’s validation, but the pinned iceberg-rust transformer could not resolve it and failed with field not found.
  • Design approach: Collect top-level field IDs from the current schema and schema history, then fall back when a resolved equality-delete key is absent from that set.
  • Correctness: Verified that Schema.columns() returns top-level fields while findField(id) resolves nested fields. The native loader’s top-level lookup confirms the need for this guard. Focused checks distinguished nested keys from top-level keys, including dropped fields and mixed deletes. No introduced P1/P2 correctness issue was identified.
  • Compatibility analysis: Checked Iceberg Java’s schema and Spark reader implementations in 1.5.2, 1.8.1, 1.10.0, and 1.11.0, covering the dependencies used by Comet’s Spark 3.4–4.2 profiles. The added calls use existing reflection helpers. Compared with latest release branch branch-1.1 at 89c729a9697467e8f8d89e3eadf1797a75e2328c: the affected validation previously admitted nested keys, so this is an intended fallback change.
  • Key design decisions: Checking field IDs avoids confusing same-named fields. Including historical schemas preserves native eligibility for dropped top-level keys. Existing unsupported-type and unresolved-ID checks remain effective.
  • Implementation sketch: Eleven production lines extend deleteFileTypesSupported. The test helper accepts explicit equality-field IDs while preserving its previous default. The regression writes a genuine nested-key delete, checks the result and fallback, and updates the user guide.
  • Performance: Scans with delete files gain one traversal of current and historical top-level columns and set lookups for equality keys. This adds planning work, including for positional-only deletes, but no per-row processing. No evidence-backed P1/P2 performance regression was identified. Planning overhead was not benchmarked.
  • Design: Extending the existing validation block is a straightforward way to keep an unsupported case out of native execution until the upstream reader supports it.
  • Abstraction & complexity: Existing reflection helpers provide the necessary schema inspection. The optional test-helper argument enables nested-key fixtures without introducing a production abstraction or changing existing callers.
  • Behavioral changes worth calling out: Nested-key equality deletes now use Spark’s reader. Top-level equality keys and positional deletes retain their previous eligibility. Spark/Iceberg’s existing limitation when the projection omits the nested equality key remains. The regression deliberately projects that key.
  • Suggested improvements: No additional change met the P1/P2 reporting bar. Full integration validation remains outstanding as described below.

Routed skills: review-comet-pr. No sibling skill applies to this scan-planning fallback. Repository AGENTS.md and the relevant contributor guidance were read.

Exact-head CI: Comet CI, Check PR Title, and CodeQL report action_required. The label job remains queued. There is no completed build or test verdict for this head.

Validation: Compiled the verbatim base/head delete-validation blocks with the current reflection implementation and modified test helper in a disposable harness. Eleven cases passed on each of Iceberg 1.10.0 and 1.11.0, with a base/head regression control. A real Parquet equality-delete reproduction on Spark 4.1.3/Iceberg 1.11.0 returned [90,420,4470]. Its planned delete files were accepted by the base guard and rejected by the head guard with the expected reason. git diff --check passed.

Limits: These focused checks did not execute the complete CometIcebergNativeSuite, the full Comet fallback plan, or the Spark/Iceberg integration matrix. No fresh native build or performance benchmark was run. Project source remained unchanged, and nothing was published to GitHub.

@andygrove andygrove left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix @yunchipang. This is a clean way to keep the nested-key case out of the native scan until iceberg-rust#3453 lands.

I compared against branch-1.1. The nested-key failure is the same there, so this is an intended change from a native field not found error to a fallback that returns Spark's answer. Top-level keys, dropped top-level keys and struct or variant keys behave as before. Checking the schema history for top-level ids is the right call. If reflection fails, the key looks nested and the scan falls back, so that failure mode is safe.

The test arithmetic checks out (90, 420 and 4470) and projecting the key is the right way to get a valid comparison with Spark.

One request: could we get the run-iceberg-tests label on this once the workflows are approved, so the Iceberg version matrix confirms it before merge?

@andygrove
andygrove enabled auto-merge October 20, 2026 01:55
@yunchipang

Copy link
Copy Markdown
Contributor Author

@andygrove thanks! i don't think i have the permission to add labels though. could you add run-iceberg-tests please? thanks again!

@andygrove
andygrove added this pull request to the merge queue Oct 20, 2026
Merged via the queue into apache:main with commit a48c82d Oct 20, 2026
44 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Iceberg area:scan Parquet scan / data reading bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Native Iceberg scan fails with field not found when an equality delete is keyed on a nested field

3 participants