Repository navigation
[fix](scan) Read all nested access paths before evaluating TopN filters on pruned STRUCT columns - #68110
Merged
yiguolei merged 1 commit intoSep 20, 2026
Conversation
…rs on pruned STRUCT columns
### What problem does this PR solve?
Issue Number: None
Problem Summary:
With `enable_prune_nested_column = true`, the planner splits a STRUCT column's
access paths into predicate paths (read before filtering) and lazy paths (read
only for surviving rows). SegmentIterator relies on this split when a common
expression references the column: it reads the column in the PREDICATE phase,
evaluates the expression, and recovers the lazy nested fields afterwards.
The TopN filter is a common expression that BE attaches to the scan at runtime,
after the planner computed the access paths. When ORDER BY and WHERE reference
different fields of the same STRUCT, for example
```sql
SELECT struct_element(s, 'a') FROM t
WHERE struct_element(s, 'b') IS NOT NULL
ORDER BY 1 NULLS LAST LIMIT 1;
```
the predicate paths only contain `s.b.NULL`, so `s.a` is a placeholder (NULL)
while the TopN filter `struct_element(s, 'a') <= current_top` is evaluated.
Every row of the later tablets is rejected and the global minimum is lost,
silently returning a wrong LIMIT result.
This PR marks columns referenced by runtime-generated common expressions
(TopN filters and runtime filters) and keeps them in the NORMAL read phase, so
all access paths are materialized before the expression is evaluated. The lazy
split is still used for planner-visible expressions.
### Release note
None
### Check List (For Author)
- Test:
- Unit Test: `SegmentIteratorRuntimeCommonExprTest` covers the lazy split
decision for planner expressions, TopN filters, and both on one column.
- Regression test: `topn_filter_nested_column_pruning` reproduces the wrong
result with TIMESTAMPTZ/INT/DATE STRUCT fields, ASC/DESC, NULLS FIRST/LAST,
selective predicates and multi-tablet scans.
- Behavior changed: No
- Does this need documentation: No
Claude-Session: https://claude.ai/code/session_01E3gDAafAXZELs6AHVfD9UG
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review outcome: no blocking findings. The change is focused and the runtime-generated TopN/runtime-filter path is handled correctly.
Critical checkpoint conclusions:
- Goal and correctness: the new per-column state distinguishes BE-runtime-generated common expressions from planner-visible common expressions. For STRUCT/ARRAY/MAP columns used by those expressions, suppressing lazy nested recovery makes both initial and row-id common-expression reads use NORMAL phase, so every FE-retained predicate and lazy-output descendant is materialized before the runtime expression evaluates. TopN and runtime-filter roots retain their identity through normalization/cloning, and virtual slots are expanded with the runtime flag preserved.
- Scope and clarity: the production change is confined to SegmentIterator state marking and lazy-materialization planning. The conservative eager read applies only to retained complex columns referenced by a runtime-generated common expression.
- Concurrency: no new shared mutable state or locking is introduced; the added flags are iterator-local initialization state. I traced adaptive scanner admission, nonempty-block/EOS resubmission, TopN bound publication, and the p0 one-BE topology. The regression serial variants deterministically publish a bucket-1 bound before scanning bucket 0, which contains the global extrema and exercises the old failure.
- Lifecycle and resources: the state is rebuilt and initialized with the existing SegmentIterator lifecycle, is monotonically ORed across expressions, and adds no ownership, cleanup, static-initialization, or memory-accounting concern.
- Configuration and compatibility: no configuration, protocol, persisted/storage format, public symbol, or rolling-upgrade contract changes are introduced.
- Parallel and special paths: TopN and runtime-filter wrappers, predicate/common overlap, mixed planner/runtime expressions, index-consumed expressions, virtual slots, meta paths, and STRUCT/ARRAY/MAP read-phase behavior were checked. No parallel path requiring a corresponding production change was found.
- Tests and expected results: the BE unit rejects the old lazy-planning decision and covers planner-only, TopN-only, and mixed-expression state. The ordered p0 regression covers different-field ASC/DESC cases across TIMESTAMPTZ, INT, and DATE, plus selective, same-field, multi-STRUCT, and default-parallel shapes; its expected output matches the inserted data. The LIMIT-2 and NULLS-FIRST labels provide extra coverage but are not relied on to prove the repaired branch. Test discovery was verified statically.
- Observability, transactions, and cross-layer variables: no additional observability is needed for this local read-planning decision. Transaction/persistence/data-write concerns and new FE-to-BE variables are not applicable.
- Performance: runtime-generated filters may read all FE-retained nested paths for the affected complex column instead of recovering lazy children later. That bounded extra I/O is the necessary correctness fallback because BE-generated predicate paths are unavailable to FE pruning.
- User focus: no additional review focus was provided.
Review status: complete for the supplied head, but static-only. Per the review environment constraints I did not build Doris or execute the BE/regression tests. git diff --check and the static build-hygiene gate passed; visible GitHub formatting, license, title, and secret/large-file checks were also green at review time.
Member
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 27899 ms |
Contributor
TPC-DS: Total hot run time: 153307 ms |
Contributor
ClickBench: Total hot run time: 23.94 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
mrhhsg
marked this pull request as ready for review
September 18, 2026 02:16
mrhhsg
requested review from
airborne12,
csun5285,
eldenmoon,
gavinchou,
liaoxin01 and
yiguolei
as code owners
September 18, 2026 02:16
Mryange
approved these changes
Sep 18, 2026
liaoxin01
approved these changes
Sep 18, 2026
csun5285
approved these changes
Sep 19, 2026
yiguolei
approved these changes
Sep 20, 2026
2 of 3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: None
Problem Summary:
With
enable_prune_nested_column = true, the planner splits a STRUCT column'saccess paths into predicate paths (read before filtering) and lazy paths (read
only for surviving rows). SegmentIterator relies on this split when a common
expression references the column: it reads the column in the PREDICATE phase,
evaluates the expression, and recovers the lazy nested fields afterwards.
The TopN filter is a common expression that BE attaches to the scan at runtime,
after the planner computed the access paths. When ORDER BY and WHERE reference
different fields of the same STRUCT, for example
the predicate paths only contain
s.b.NULL, sos.ais a placeholder (NULL)while the TopN filter
struct_element(s, 'a') <= current_topis evaluated.Every row of the later tablets is rejected and the global minimum is lost,
silently returning a wrong LIMIT result.
This PR marks columns referenced by runtime-generated common expressions
(TopN filters and runtime filters) and keeps them in the NORMAL read phase, so
all access paths are materialized before the expression is evaluated. The lazy
split is still used for planner-visible expressions.
Release note
None
Check List (For Author)
SegmentIteratorRuntimeCommonExprTestcovers the lazy splitdecision for planner expressions, TopN filters, and both on one column.
topn_filter_nested_column_pruningreproduces the wrongresult with TIMESTAMPTZ/INT/DATE STRUCT fields, ASC/DESC, NULLS FIRST/LAST,
selective predicates and multi-tablet scans.
https://claude.ai/code/session_01E3gDAafAXZELs6AHVfD9UG