Conversation
### What problem does this PR solve?
Issue Number: None
Problem Summary:
`SELECT lazy_col AS x, lazy_col AS y FROM t ORDER BY x LIMIT 1` failed with
"A expression contains slot not from children".
The TopN order key is the alias slot, so that alias has to be computed below the
TopN. However `MaterializeProbeVisitor` only protects the order key slot itself
(an order key slot is in `TopN.getInputSlots()`) and never resolves an identity
alias down to the column the alias reads. The probe of the other output
(`lazy_col AS y`) therefore resolved to the base column `lazy_col` and classified
it as lazily materialized, and `LazySlotPruning` removed `lazy_col` from the scan
while `lazy_col AS x` below the TopN still read it. The final `Validator` rejected
the resulting plan, so the query returned an error. When `fe_debug=true`, the
failure was caught inside `LazyMaterializeTopN` instead, which silently skipped
lazy materialization (the query succeeded but lost the optimization).
Fix: `LazyMaterializeTopN` now resolves the TopN order keys through the identity
alias chain of the Projects under the TopN and adds the resolved slots (and the
intermediate alias slots) to `requiredMaterializedSlots`. The probe then rejects
every lazy candidate backed by a column that an order key reads, so the plan stays
valid. The resolution stops at set operations, which the probe never materializes
through (lazy materialization does not cross set operations today; if that is ever
extended, the order keys have to be resolved per set operation branch).
Only the ordering column is forced to be materialized: other columns are still
fetched lazily, and plans that order by a plain column are unchanged.
Reproduction on master:
```sql
create table t(sort_col int, lazy_col int) duplicate key(sort_col)
distributed by hash(sort_col) buckets 1 properties('replication_num'='1');
set fe_debug = false;
select lazy_col as x, lazy_col as y from t order by x limit 1;
-- ERROR 1105: A expression contains slot not from children
-- Slot: lazy_col#1 Children Output:{0, 4}
```
### Release note
TopN lazy materialization no longer builds an invalid plan (no more
"A expression contains slot not from children") when a query orders by an alias of
a column. The ordering column is materialized during the scan, while other columns
keep using lazy materialization.
### Check List (For Author)
- Test
- [x] Regression test (regression-test/suites/query_p0/topn_lazy/order_by_alias)
- [x] Unit Test (TopnLazyMaterializeTest, LazyMaterializeTopNTest)
- Behavior changed: Yes. Queries that order by an alias no longer fail planning;
the column feeding the order key stays materialized.
- Does this need documentation: No
englefly
requested review from
924060929,
morrySnow and
starocean999
as code owners
September 15, 2026 10:59
Contributor
Author
|
run buildall |
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
FE Regression Coverage ReportIncrement line coverage |
Contributor
Author
|
run performance |
Contributor
TPC-H: Total hot run time: 16885 ms |
Contributor
TPC-DS: Total hot run time: 82644 ms |
Contributor
ClickBench: Total hot run time: 14.86 s |
Contributor
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.
Static review of exact head 4ea274aa90f586ad6fddf064d939672685d957be found one correctness blocker and one test-coverage gap.
The direct ORDER BY-alias case is fixed, but identity-alias bases consumed by descendant operators are still not protected: a reachable nested-TopN plan can prune a scan slot that a retained inner ordering alias reads, and the final validator rejects the query. The new index-mode regression cases also remain green on the old implementation and therefore do not prove that parallel path. Details and concrete repros are inline.
Critical checkpoint conclusions:
- Goal and correctness: incomplete because the descendant-consumer case still creates an invalid plan; the direct current-TopN case is sound.
- Scope and structure: the change is focused, and the postprocessor ordering through the final
Validatoris coherent. - Parallel paths and tests: traced default and index modes, local/merge and nested TopNs, Projects, joins, filters, pruning boundaries, and final validation. Direct non-index validity and unrelated-column laziness are covered; descendant consumers and a pre-fix-sensitive index shape are not.
- Compatibility and lifecycle: no FE/BE protocol, rolling-upgrade, persistence, transaction, configuration-definition, concurrency, locking, security, or resource-lifecycle change is involved.
- Performance and observability: the added traversal is linear over the local subtree and raises no material performance concern; no observability change is needed. The follow-up should close only identity-alias dependencies rather than force every descendant input eager.
- User focus: no additional focus was supplied, so the complete authoritative patch and all changed files were reviewed.
This was a static-only review as required: no source edits, builds, or tests were run. The live PR base/head and all comment surfaces were rechecked immediately before submission; existing exact-head CI reports FE UT, P0 regression, compile, coverage, and performance success, with the automated code-review status pending.
| private void collectOrderKeyColumns(PhysicalTopN<? extends Plan> topN, Set<Slot> requiredMaterializedSlots) { | ||
| Map<Slot, Slot> aliasToChild = new HashMap<>(); | ||
| collectIdentityAliasMap(topN.child(), aliasToChild); | ||
| for (OrderKey orderKey : topN.getOrderKeys()) { |
Contributor
There was a problem hiding this comment.
[P1] Protect alias bases consumed below this TopN Resolving only this TopN's order keys leaves the same invalid-plan bug for descendant TopNs. For example, select y from (select lazy_col AS x, lazy_col AS y, other_col AS z from topn_lazy_order_by_alias_tbl order by x limit 2) s order by z limit 1 preserves an outer TopN(z) -> ... -> TopN(x) -> Project(a AS x, a AS y, b AS z) -> Scan(a,b). This loop protects only z -> b; probing y can still mark a lazy, after which pruning removes scan slot a while retaining a AS x for the inner TopN, and the final Validator rejects the plan when fe_debug=false. Please close identity-alias dependencies for retained descendant consumers/already-required slots (stopping at the same boundaries) and add this nested regression.
| // The same shapes through the inverted-index filter path. | ||
| sql """ set topn_lazy_materialization_using_index = true; """ | ||
|
|
||
| order_qt_using_index_repeated_alias """ |
Contributor
There was a problem hiding this comment.
[P2] Exercise an index-mode shape that failed before this patch These two index-mode queries are pre-fix-insensitive: when topn_lazy_materialization_using_index is enabled, MaterializeProbeVisitor.visitPhysicalProject refuses an identity Alias as a lazy source and eagerly requires its child, so both alias-heavy shapes already keep lazy_col materialized on the old code. A sensitive case is select lazy_col AS x, lazy_col, other_col from topn_lazy_order_by_alias_tbl where sort_col > 0 order by x limit 1: before this change the bare lazy_col can be pruned while the retained alias still reads it, whereas the fix should protect it and leave other_col lazy. Please add this shape (with the validator enabled), plus a plan assertion if this suite is intended to prove selective laziness rather than only successful execution.
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
2 participants
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
Related PR: None
Problem Summary:
SELECT lazy_col AS x, lazy_col AS y FROM t ORDER BY x LIMIT 1failed planning withA expression contains slot not from children.The TopN order key is the alias slot, so that alias has to be computed below the TopN.
MaterializeProbeVisitoronly protects the order key slot itself (an order key slot is inTopN.getInputSlots()) and never resolves an identity alias down to the column the alias reads.The probe of the other output (
lazy_col AS y) therefore resolved to the base columnlazy_coland classified it as lazily materialized, so
LazySlotPruningremovedlazy_colfrom the scan whilelazy_col AS xbelow the TopN still read it. The finalValidatorrejected the resulting plan andthe query returned an error. With
fe_debug=truethe failure was caught insideLazyMaterializeTopNinstead, which silently skipped lazy materialization (the query succeeded but lost the optimization).
Reproduction (master,
fe_debug=false):Fix:
LazyMaterializeTopNresolves the TopN order keys through the identity alias chain of theProjects under the TopN and adds the resolved slots (plus the intermediate alias slots) to
requiredMaterializedSlots, so the probe rejects every lazy candidate backed by a column an orderkey reads. The resolution stops at set operations, which the probe never materializes through
(lazy materialization is not supported through set operations today; if that ever changes, order
keys have to be resolved per branch).
Effect: affected plans now either keep only the ordering column materialized (other columns are
still fetched lazily) or skip lazy materialization, and the plan stays valid. Plans that order by a
plain column are unchanged.
Release note
TopN lazy materialization no longer builds an invalid plan (no more
A expression contains slot not from children) when a query orders by an alias of a column.The column that feeds the order key is materialized during the scan, while other columns keep using
lazy materialization.
Check List (For Author)
regression-test/suites/query_p0/topn_lazy/order_by_alias)TopnLazyMaterializeTest,LazyMaterializeTopNTest)ordering column is kept materialized instead of being pruned from the scan.
Check List (For Reviewer who merge this PR)