airborne12 opened a new pull request, #69399:
URL: https://github.com/apache/doris/pull/69399
### What problem does this PR solve?
Issue Number: N/A
Related PR: #69292 (depends on this fix)
Problem Summary:
A CTE fails to plan when the optimizer inlines it after one of its scans got
a virtual column:
```sql
WITH c AS (
SELECT id, score() AS s FROM t WHERE c1 MATCH 'apple'
ORDER BY s DESC LIMIT 2
)
SELECT * FROM c;
-- ERROR: A expression contains slot not from children
```
A score or ANN TopN pushed into a scan, and a MATCH in the select list, add
a virtual column to the scan. After rewrite, the optimizer inlines a CTE that
has one consumer left (`Optimizer.normalizeCtePlan` → `CTEInliner`) by deep
copying its rewritten body, so the copy includes these scans:
- `score()` and `l2_distance_approximate()` are nondeterministic, so the
rewrite does not inline a CTE that calls them, even with one consumer. Once the
TopN pushdown has replaced them with a virtual column of the scan, the
optimizer inlines the CTE. So every CTE with `ORDER BY score() LIMIT n` or an
ANN TopN fails, with default settings.
- A CTE with MATCH in its select list reaches the optimizer when the rewrite
removes all but one of its consumers, e.g. `... UNION ALL SELECT * FROM c WHERE
1 = 0`.
`LogicalPlanDeepCopier` copied a scan with virtual columns wrongly:
- it gave each virtual column two new ids: the slots above the scan were
mapped to the first one, and the copied scan output the second one;
- the copied scan still output the original virtual column, with its
original id, next to the new one;
- the score and ANN order keys of the copied scan still used the original
slots.
So the plan above the copied scan read a slot that the scan does not output.
The fix:
- The copier gives each virtual column one new id, maps the original id to
it, and maps the score and ANN order keys to the new slots. Relations without
virtual columns are copied as before.
- `LogicalOlapScan.withVirtualColumns` replaces the virtual columns of the
scan, but kept their old slots in the output next to the new ones. It now
replaces them there too and keeps the other output slots, which the copier has
already mapped. Computing the output of the copy again would not work: it gives
VARIANT subpath slots new ids. `AdjustNullable`, the other caller that replaces
existing virtual columns, no longer keeps two slots for a virtual column whose
nullability changes.
#69292 binds `score()` to its scan before CTEs are inlined, so it needs this
fix; it carries the same change until this PR is merged.
### Release note
Fix "A expression contains slot not from children" for a CTE that orders by
`score()` or by an ANN distance with a LIMIT, or that has MATCH in its select
list and is inlined after rewrite.
### Check List (For Author)
- Test
- [x] Regression test
- [x] 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
- Unit tests:
`LogicalPlanDeepCopierTest.testDeepCopyOlapScanWithVirtualColumn` and
`CTEInlineTest.inlineCteWhoseScanHasVirtualColumns` (CTEs with a score TopN, an
ANN TopN, a MATCH projection, and a VARIANT subpath with a MATCH projection,
with one consumer and with a second consumer removed by `WHERE 1 = 0`). Both
fail on master (4 output slots instead of 3; `A expression contains slot not
from children`) and pass with this change. `OrExpansionTest`,
`SplitMultiDistinctTest`, `DecomposeRepeatWithPreAggregationTest`,
`LogicalOlapScanTest`, `IvmDeltaRewriteHelperTest` and
`PushDownVirtualColumnsIntoOlapScanTest` pass. Checkstyle passes.
- Regression test:
`nereids_rules_p0/cte/test_cte_inline_virtual_column`, with its `.out`
generated by the framework, on an FE built from this branch and a BE built from
41a58afba2 plus #69292's `SegmentIterator` change (this change is FE only). The
VARIANT case is only in the unit test, because that BE reads `v['x']` as NULL
even without a CTE.
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
