github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3710502168
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -579,7 +788,7 @@ public PreAggStatus visitSum(Sum sum, List<Expression>
returnValues) {
public PreAggStatus visitMax(Max max, List<Expression>
returnValues) {
for (Expression value : returnValues) {
if (!(isAggTypeMatched(value, AggregateType.MAX) ||
isKeySlot(value)
- || value.isNullLiteral())) {
+ || value.isLiteral())) {
Review Comment:
[P2] Exercise the newly accepted literal path
Both visitors now accept every literal instead of only NULL, but none of the
added positive cases reaches this branch with PREAGGREGATION ON. The cast cases
q24-q33 deliberately exit OFF before accepting their return, q37's selected
ASOF scan is forced OFF before this checker, and q34/q38/q39 contain only slot
returns. A regression back to NULL-only handling (or a mistake in this new
route) would therefore leave the suite green. Please add a cast-free non-NULL
case such as `max(if(k6 > 0, v9, 0))` that asserts this scan ON and records the
result, plus MIN/CASE symmetry for the parallel changed branch.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -294,6 +405,41 @@ private PreAggStatus createPreAggStatus(LogicalOlapScan
logicalOlapScan, PreAggI
return PreAggStatus.off(String.format("Join conjuncts %s
contains non-key column %s",
joinConjuncts, joinInputSlots));
}
+
+ // Row-stability check: volatile expressions evaluated per partial
row
+ // produce different results than per merged logical row, even when
+ // their input slots are all key columns or empty. Check centrally
+ // before per-scan candidate filtering so the guard also covers
+ // other-table aggregates, slot-less filters, joins, and grouping.
+ for (AggregateFunction aggFunc : aggregateFuncs) {
+ if (aggFunc.containsVolatileExpression()) {
Review Comment:
[P1] Fence retained non-movable project expressions
```text
Aggregate(max(if(k1 > 0, v9, 0)))
Project(k1, v9, assert_true(v7 > 0, 'bad') AS checked) -- retained though
unused
Scan(t AGG_KEYS(k1); v7 SUM; v9 MAX)
```
Load the same full key in separate rowsets as `(v7=-2,v9=10)` and
`(v7=3,v9=20)`. Previously the non-NULL zero branch kept this scan OFF, so
storage merged `v7` to `1` before the retained `assert_true` ran and the query
succeeded. With the new literal acceptance the aggregate can turn the scan ON,
but these row-stability loops never inspect the unused `checked` producer;
`LogicalProject.pruneOutputs` deliberately retains its `NoneMovableFunction`,
so it sees raw `v7=-2` and BE throws `InvalidArgument`. Please track retained
non-movable project expressions in the context and keep affected scans OFF,
with a duplicate-full-key regression for this plan.
--
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]
