github-actions[bot] commented on code in PR #69646:
URL: https://github.com/apache/doris/pull/69646#discussion_r4384340283
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterPushDownVisitor.java:
##########
@@ -108,6 +112,12 @@ public Boolean visitPhysicalProject(
return project.child().accept(this, ctxProjectProbeExpr);
}
+ private static boolean hasEvaluationSensitiveExpressions(Plan plan) {
+ return plan.getExpressions().stream().anyMatch(expression ->
+ expression.containsVolatileOrNoneMovableExpression()
+ ||
ExpressionUtils.containsUnsafeDateArithmetic(expression));
Review Comment:
[P2] Include throwing epoch conversions in the safety classification
`containsUnsafeDateArithmetic` recognizes only DateAddSubMonotonic and
DateCeilFloorMonotonic. `from_second`, `from_millisecond`, and
`from_microsecond` instead implement FromSecondMonotonic and are neither
volatile nor NoneMovable, so this helper permits them even though BE
`TimestampToDateTime::execute_impl` throws on negative input. A reduced
triggering plan is:
```text
TopN(k ASC, v ASC LIMIT 1)
Project(k, from_second(ts) AS v)
FileScan(k INT, ts BIGINT)
```
The secondary key keeps `v` below TopN, and translation can fuse it into the
scanner projection. In one split/scanner/local TopN, let physical RG0 contain
`(k=100, ts=-1)` and RG1 contain several full batches of `(k=0, ts=0)`.
Physical order evaluates RG0 and throws. Best-first ordering can read RG1,
publish bound `k=0`, and skip RG0 at the footer recheck before evaluating the
conversion. The scan's reorder flag sees only conjuncts and cannot detect this
projection.
Include the throwing epoch-conversion family in the evaluation-sensitivity
classification and cover a negative epoch with a secondary sort key. This is a
distinct conversion family from the existing `weeks_add` report, whose
DateAddSubMonotonic instance is now correctly fenced.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/processor/post/TopnFilterPushDownVisitor.java:
##########
@@ -192,6 +202,10 @@ private PushDownContext
adjustProbeExprNullableThroughOuterJoin(Plan leftChild,
@Override
public Boolean visitPhysicalHashJoin(PhysicalHashJoin<? extends Plan, ?
extends Plan> join,
PushDownContext ctx) {
+ // Scan conjuncts cannot fence ON expressions retained on the join,
including mark predicates.
+ if (hasEvaluationSensitiveExpressions(join)) {
Review Comment:
[P2] Fence sensitive residual filters and generators before descent
Checking the join's expression list still misses a retained WHERE predicate.
This reduced plan is reachable:
```text
TopN(l.k ASC LIMIT 1)
Filter(assert_true(l.k < 3, 'bad'))
InnerHashJoin(l.id = r.id)
FileScan(l.id, l.k)
small broadcast build r
```
`PushDownFilterThroughJoin` retains this NoneMovable expression in the
Filter, but the default TopN visitor descends through it unchecked.
`join.getExpressions()` contains only hash/other/mark conjuncts at this stage;
`PhysicalPlanTranslator.visitPhysicalFilter` attaches the WHERE expression to
the join only later. It therefore never reaches the scan's
`allow_row_reordering` fence. With one split/scanner/local TopN, matching IDs,
physical RG0 containing `k=100` and RG1 containing several full batches of
`k=0`, physical order raises `bad` before any frontier; the new ordering can
publish bound `0` from RG1 and reject RG0 before the assertion runs.
The same unchecked default traversal permits
`Generate(explode(array(assert_true(k < 3, 'bad'))))`; its argument executes in
the table-function operator, outside scan conjuncts. Fence sensitive
Filter/Generate evaluation before traversing their children and add these
planner/error cases. This is distinct from the existing ON-predicate thread:
that fixed instance is visible in the join expression list, while these
operators are not.
--
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]
