yujun777 opened a new pull request, #64335:
URL: https://github.com/apache/doris/pull/64335
## Problem
`SimplifyAggGroupBy` simplified `GROUP BY f(x)` to `GROUP BY x` without
verifying that `f(x)` is injective (one-to-one). This caused wrong results:
| Expression | Why wrong |
|---|---|
| `a * 0` / `0 * a` | always evaluates to 0 — all rows fall into one group |
| `0 / a` | always evaluates to 0 |
| `a / 0` | division by zero |
| `a + NULL` / `a * NULL` / ... | always evaluates to NULL |
| `a * 0.1` with float/double | precision loss may map different inputs to
same result |
## Fix
1. **`isBinaryArithmeticSlot`**: restructured to separate slot-expr from
literal,
then validate each independently. Float/double check runs early, before
slot extraction.
2. **New `checkLiteral(expr, literal)`**: rejects NULL literal and
Multiply/Divide by zero.
3. **New `canExtractSlot(expr)`**: replaces the old unconditional
`extractSlotOrCastOnSlot` — only accepts bare `Slot` or implicit lossless
widening casts (integral→integral, float→double, integral→decimal,
decimal→decimal). Range and scale are compared directly for correctness.
4. **New `isLosslessWidening(src, tgt)`**: type-family-aware widening check
using `width()` for integral/float, and `DecimalV3Type.forType()` for
decimal range/scale comparison.
## Changes
- `SimplifyAggGroupBy.java`: +80 lines, rewritten core logic
- `ExpressionUtils.java`: -35 lines, removed unused `isSlotOrCastOnSlot` /
`extractSlotOrCastOnSlot`
- `SimplifyAggGroupByTest.java`: +216 lines, 25 tests covering all new paths
--
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]
