Skip to content

branch-4.1: [fix](iceberg) Resolve dropped equality-delete fields in manifest-cache planning - #68433

Open
hubgeter wants to merge 1 commit into
apache:branch-4.1from
hubgeter:fix/iceberg-manifest-cache-dropped-equality-key-branch-4.1
Open

hubgeter wants to merge 1 commit into
apache:branch-4.1from
hubgeter:fix/iceberg-manifest-cache-dropped-equality-key-branch-4.1

Conversation

@hubgeter

@hubgeter hubgeter commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #67479, #67687

Problem Summary:

#67479 lets DeleteFileIndex resolve equality-delete fields that were later dropped, by looking them up in historical schemas. It covers the default planner but misses the manifest-cache planner in IcebergScanNode, which still passes only specsById. When the manifest cache is enabled, planning fails with Cannot find field for ID N and falls back to the SDK scan: results are correct, but each query logs a WARN and skips the cache.

The master port (#67687) already covers this path. This PR applies the same one-line change to branch-4.1: pass icebergTable.schemas() to the index.

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

    Manual test: with the manifest cache enabled, EXPLAIN VERBOSE shows failures=0 instead of 1, and the fallback WARN is gone.

  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

…manifest-cache planning

An equality delete can reference a field that a later schema change
dropped. Iceberg 1.11.0 resolves such field IDs against every
historical schema, and apache#67479 synced Doris's DeleteFileIndex with it,
but the manifest-cache planner builds the index from specsById alone.
Those specs are bound to the current schema, so planning fails with

  IllegalArgumentException: Cannot find field for ID 1

and falls back to the SDK scan. Results stay correct, but every query
logs a WARN with a stack trace and the manifest cache is bypassed for
the table.

Pass the table's schemas to the index, as DataTableScan does and as
master's connector already does.
@hubgeter
hubgeter requested a review from yiguolei as a code owner September 23, 2026 08:04
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hubgeter

Copy link
Copy Markdown
Contributor Author

run buildall

@hubgeter

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Static review completed on exact head 76375a30e5a009a6b6af2707ec83aa64d2a22d31 against base 12a53cd879c68c2fa6eeffb0dd5713c1b514cee0.

No new valuable findings.

  • Correctness and lifecycle: reviewed all three changed files and traced manifest-cache planning through the frozen Iceberg table generation, SDK fallback, cached file copies, delete sequence/spec/partition applicability, and dropped-field schema resolution. The new schema map matches Iceberg's normal planner input for the retained-history case addressed by this patch.
  • Compatibility and boundaries: the builder visibility widening is compatible with the existing vendored API and is required by the cross-package caller. The patch changes no persisted format, configuration/session behavior, FE/BE protocol, memory ownership, or nullability contract. No distinct performance or observability issue was substantiated.
  • Tests and validation: the added regression is a meaningful parent-code negative and checks that the equality delete attaches only to the older data file. Validation was static only as required; no build or test was run. git diff --check passed. Author and CI claims were not treated as independent validation.
  • Scope and completeness: there was no additional user focus and no pre-existing inline thread to address. Two complete review rounds plus a final changed-file and unresolved-candidate sweep covered the production, test, API, cache, fallback, schema-evolution, and cleanup paths. A broader cleanup-pruned-schema lifecycle was investigated and dismissed as an inline finding because it fails identically at the parent revision; it should be tracked separately rather than attributed to this backport.

The review is complete for the supplied exact-head bundle, with zero new inline comments by design.

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.

2 participants