Skip to content

[fix](iceberg) Project the row filter before the delete-manifest prune in the cached scan plan - #67406

Open
raghav-reglobe wants to merge 1 commit into
apache:masterfrom
raghav-reglobe:fix-iceberg-cached-plan-delete-manifest-projection
Open

[fix](iceberg) Project the row filter before the delete-manifest prune in the cached scan plan#67406
raghav-reglobe wants to merge 1 commit into
apache:masterfrom
raghav-reglobe:fix-iceberg-cached-plan-delete-manifest-projection

Conversation

@raghav-reglobe

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: N/A

Related PR: #64304 (where the plugin-world port of this path landed; the data-manifest side got the projection, the delete-manifest side did not)

Problem Summary:

cacheBackedFileScanTasks — the manifest-cache planning path shared by the synchronous, streaming, and COUNT(*) plans — prunes delete manifests with:

ManifestEvaluator.forPartitionFilter(filterExpr, spec, caseSensitive).eval(manifest)

passing the raw row filter. forPartitionFilter binds against the partition struct, so this works by accident on identity-only specs (the partition field keeps the source column name) and throws on any spec with a transform — e.g. (identity(flag), month(ts)) stores the field as ts_month: int — whenever the filter references the transform's source column. The catch in planFileScanTask then aborts the whole cached plan into the SDK fallback, once per query:

[IcebergScanPlanProvider.planFileScanTask():2468] Iceberg plan with manifest cache failed, falling back to SDK scan: Cannot find field 'ts' in struct: struct<1000: flag: optional boolean, 1001: ts_month: optional int>
org.apache.iceberg.exceptions.ValidationException: Cannot find field 'ts' in struct: ...
    at org.apache.iceberg.expressions.NamedReference.bind(NamedReference.java:45)
    ...

Net effect on v2 tables with delete files and a time-transform partition spec: every filtered query silently loses the manifest cache (planning latency + repeated catalog/storage manifest reads) and logs a WARN with a full stack trace. On one production FE where most queries filter on the transform's source column we measured ~1,100 such stacks per hour; the fallback also means the manifest cache is effectively dead for the hottest query shape on those tables.

The data-manifest side of the very same method (getMatchingManifest) already projects the filter into partition space before building its evaluator. This PR does the same for delete manifests — Projections.inclusive(spec, caseSensitive).project(rowFilter) — via a small package-private helper so the behavior is unit-testable. An inclusive projection maps predicates on non-partition columns to alwaysTrue(), so pruning semantics are unchanged; the projected transform predicates now actually prune delete manifests instead of failing to bind.

Release note

Fix Iceberg manifest-cache planning falling back to the SDK scan (with a per-query WARN + stack trace) for every filtered query on tables whose partition spec contains a transform (e.g. month(ts)) and that carry delete files.

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.

    IcebergScanPlanProviderDeleteManifestPruneTest (real InMemoryCatalog v2 table, (identity(flag), month(ts)) spec, one position-delete file): pins that the raw row filter still fails to bind (the projection stays load-bearing), the projected evaluator keeps an overlapping month and prunes a far month, the identity leg still prunes, and a residual-only filter keeps the manifest.

  • Behavior changed:

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

    • No.
    • Yes.

Check List (For Reviewer who merged this PR)

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

…e in the cached scan plan

cacheBackedFileScanTasks (the manifest-cache planning path shared by the
synchronous, streaming and COUNT(*) plans) prunes DELETE manifests with

    ManifestEvaluator.forPartitionFilter(filterExpr, spec, caseSensitive)

passing the raw ROW filter. forPartitionFilter binds against the partition
struct, so this works by accident on identity-only specs (the partition
field keeps the source column name) and throws ValidationException
("Cannot find field 'ts' in struct: struct<... ts_month: int>") on any
spec with a transform — e.g. (identity(flag), month(ts)) — whenever the
filter references the transform's source column. The catch then aborts
the WHOLE cached plan into the SDK fallback, logging a WARN with a stack
trace per query:

    [IcebergScanPlanProvider.planFileScanTask()] Iceberg plan with
    manifest cache failed, falling back to SDK scan: Cannot find field
    'ts' in struct: struct<1000: flag: optional boolean, 1001: ts_month:
    optional int>

So on v2 tables with delete files and a time-transform partition spec,
every filtered query silently loses the manifest cache (planning latency
+ repeated catalog/storage manifest reads) and spams the warn log — we
measured ~1,100 such stacks per hour on one production FE where most
queries filter on the transform's source column.

The data-manifest side of the very same method (getMatchingManifest)
already projects the filter into partition space before building its
evaluator. Do the same for delete manifests: project with
Projections.inclusive(spec, caseSensitive) — predicates on non-partition
columns project to alwaysTrue(), so pruning semantics are unchanged. The
evaluator construction moves into a small package-private helper so the
behavior is unit-testable.

The new test builds a real InMemoryCatalog v2 table partitioned by
(identity(flag), month(ts)) with a position-delete file and pins: the raw
row filter still fails to bind (the projection stays load-bearing), the
projected evaluator keeps an overlapping month and prunes a far month,
the identity leg still prunes, and a residual-only filter keeps the
manifest.

Signed-off-by: Raghvendra Singh <raghav@cashify.in>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@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?

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