github-actions[bot] commented on code in PR #64486:
URL: https://github.com/apache/doris/pull/64486#discussion_r3409770563
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PullUpProjectExprUnderTopN.java:
##########
@@ -237,37 +170,27 @@ public Plan visitLogicalTopN(LogicalTopN topN,
CollectorContext context) {
* along the path from the TopN to the current node. An expression whose
output ExprId
* is in this set cannot be pulled up past the operators that reference it.
*/
Review Comment:
This records every alias in the replacement map, but later `simplifyProject`
removes any alias whose inputs became unavailable and `addUpperProject`
reconstructs it above the TopN. That bypasses `canPullUp` for aliases that were
intentionally rejected. For example, with `TopN -> Project(z = assert_true(x >
0)) -> Project(x = a + 1) -> Scan`, `x` is pullable, so the lower project is
simplified and `x` disappears from the upper project's child. Because `z` was
also placed in `pullUpExprReplaceMap`, the upper project is removed and `z =
assert_true(a + 1 > 0)` is restored above the TopN, even though `AssertTrue` is
a `NoneMovableFunction` and `canPullUp` would have returned false for `z`. The
same pattern applies to volatile/Score/L2Distance aliases. This can suppress
errors or change semantics for rows discarded by TopN, so the replacement map
needs to distinguish aliases that are safe to synthesize above TopN from
aliases that must remain below it.
--
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]
