github-actions[bot] commented on code in PR #65846:
URL: https://github.com/apache/doris/pull/65846#discussion_r3656054884
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/SetPreAggStatus.java:
##########
@@ -579,7 +744,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:
[P1] Keep unsafe casts out of the new literal branches
This broadening makes the following plan return ON even though the
value-branch cast does not commute with storage MAX:
```text
Aggregate(max(if(k > 0, cast(v as tinyint), 0)))
Scan(t AGG_KEYS; v BIGINT MAX)
```
With the same key committed in two rowsets as `v=120` and `v=130`, PREAGG ON
returns partial states, so the compute MAX sees `120` and `NULL` (non-strict
narrowing overflow) and returns `120`. PREAGG OFF first merges storage MAX to
`130`, whose cast is `NULL`, so the logical result is `NULL`. `removeCast`
erases the narrowing cast before this check, and the new `isLiteral()`
acceptance is what makes the non-NULL `0` branch newly eligible.
String-order-changing casts inside IF/CASE have the same problem.
The earlier cast thread asked to preserve a known-safe widening cast; this
is the opposite over-acceptance. Please peel only casts proven total and
nondecreasing for the source/target types, apply that check to every IF/CASE
return, and add a duplicate-row result regression (not only EXPLAIN).
--
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]
