raghav-reglobe opened a new pull request, #69582:
URL: https://github.com/apache/doris/pull/69582
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
With `eager_aggregation_on_broadcast_join = true`, `PushDownAggregation`
decomposes `Sum/Max/Min(If(cond, a, b))` into `func(a)` + `func(b)` pushed
below the join, then rebuilds the upper output with a replace map keyed on the
branch VALUE expressions (`func.child(0)`). `ExpressionUtils.replace` swaps
every equal expression in the output, so for `sum(if(flag > 0, 1, 0))` the
literal `0` inside the **condition** is replaced by the pushed BIGINT sum slot.
The BE then fails the query:
```
[RUNTIME_ERROR] not same type in function gt, left: TINYINT, right: BIGINT
```
Reproduce on master (any plan big enough for the rewrite to fire — e.g. a
broadcast join under the aggregate):
```sql
set eager_aggregation_on_broadcast_join = true;
create table t4 (id4 int, flag boolean, qty bigint) duplicate key(id4)
distributed by hash(id4) buckets 1 properties("replication_num"="1");
create table t2 (id2 int) duplicate key(id2) distributed by hash(id2)
buckets 1 properties("replication_num"="1");
select sum(if(t4.flag > 0, 1, 0)), t2.id2 from t4 join t2 on t4.id4 = t2.id2
group by t2.id2;
```
`EXPLAIN VERBOSE` shows `cast(flag as TINYINT) > sum[#n]` where `sum[#n]` is
the pushed sum of the literal `0`. Exposure: comparison literals that stay
equal to a branch literal after type coercion (BOOLEAN/TINYINT columns → BE
type error; for max/min the pushed slot is the constant itself, numerically
harmless). A 2-table shape where the rewrite does not fire, or `flag = TRUE`,
works.
Fix: key the replacement on the WHOLE aggregate function
(`ExpressionUtils.replace` is top-down and short-circuits): a decomposed
`func(If(cond, a, b))` is rebuilt as `func(If(cond, pushedA, pushedB))`, a
plain `func(x)` as `func(pushedX)`, `count(x)` as `sum0(pushedX)` as before.
Nothing inside the condition or beside the function is touched. This also stops
two functions over the same child in one output expression from overwriting
each other's child-keyed entry.
### Release note
Fix eager aggregation (`eager_aggregation_on_broadcast_join`) replacing a
literal inside an `IF` condition with the pushed aggregate slot when it equals
a decomposed branch value (`sum(if(col > 0, 1, 0))` failed with "not same type
in function gt").
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [x] 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 <!-- Add your reason? -->
Unit test: `EagerAggRewriterTest.testDecomposedIfKeepsConditionLiteral`
— `sum(if(bool > 0, 1, 0))`, `max(if(tinyint = 1, 1, 0))`, `sum(if(bigint > 1,
1, 0))` under a join: the rewrite still fires, both branch values are pushed
slots, every condition keeps its literal (the first two assertions fail without
this change).
Manual: the repro above returns the right row instead of the BE type
error; the production statement that failed 111 times a day (a 9-join aggregate
over BOOLEAN columns) has run clean for 4+ hours on a build with this patch
with the rewrite enabled globally; `EXPLAIN VERBOSE` keeps the `cnt$c`
multipliers (eager aggregation still applies) and shows `cast(is_delivered as
TINYINT) > 0` with the literal intact.
- Behavior changed:
- [x] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
