Decimal sum/avg overflow returns NULL under ANSI mode instead of raising, because CheckOverflowInSum is dropped during conversion
- Dominant language
- Rust
- Stars
- 1.8k
- Forks
- 241
- Avg merge
- 2d 12h
- Merged PRs (30d)
- 21
Description
### Describe the bug
With `spark.sql.ansi.enabled=true`, a decimal `sum()` or `avg()` whose running total exceeds the
target decimal precision must raise `ArithmeticException("Overflow in sum of decimals")`. Auron
returns `NULL` instead, which is the correct **non-ANSI** behaviour. ANSI mode is therefore
silently downgraded to non-ANSI for decimal aggregation: the user asked to be told about overflow
and gets a null.
The non-ANSI leg is already correct, so this is about the missing error path, not about overflow
handling in general.
### To Reproduce
```sql
SET spark.sql.ansi.enabled=true;
SELECT sum(d)
FROM (SELECT CAST('10000000000000000000' AS DECIMAL(38,18)) AS d FROM range(0, 12, 1, 1));
```
Or run the Spark suite tests directly:
- `DataFrameSuite` — `"SPARK-28067: Aggregate sum should not return wrong results for decimal overflow"`
- `DataFrameSuite` — `"SPARK-35955: Aggregate avg should not return wrong results for decimal overflow"`
Both fail with:
```text
Expected exception org.apache.spark.SparkException to be thrown, but no exception was thrown
(DataFrameSuite.scala:211)
```
`DataFrameSuite.assertDecimalSumOverflow` branches on the ANSI flag. The non-ANSI branch is
`checkAnswer(df, Row(null))` and passes under Auron; the ANSI branch expects a `SparkException`
caused by an `ArithmeticException`, and that is the branch that fails.
### Expected behavior
`ArithmeticException: Overflow in sum of decimals`, matching vanilla Spark.
### Actual behavior
`NULL`.
### Root cause
Spark's plan for a decimal `sum` is:
```
CheckOverflowInSum( Sum( PromotePrecision(Cast(child)) ), resultType, nullOnOverflow = !ansiEnabled )
```
Auron converts the `Sum` and **silently drops the `CheckOverflowInSum` wrapper**, and the cast
feeding it becomes a *try* cast. The native plan logged during the failing run is:
```
Agg [groupings=[], aggs=[AggExpr { field_name: "#133006", mode: Partial,
agg: Sum(TryCastExpr { expr: Column { name: "#133050", index: 0 },
cast_type: Decimal128(38, 12) }) }]]
```
Note what is present and what is not: the aggregate **is** native, its child is a `TryCastExpr`
which **nulls** on failure, and there is **no `CheckOverflow` node anywhere in the plan**.
So overflow yields `NULL` unconditionally. The `nullOnOverflow` flag never reaches the native plan
at all — there is nothing there to consult, and no native-side logic can recover information that
was dropped during conversion.
Verified code facts on current `master`:
| Fact | Evidence |
|---|---|
| `CheckOverflowInSum` (the wrapper Spark 3.2+ puts around decimal `Sum`) is **not converted at all** | zero matches for `CheckOverflowInSum` in any `.scala` file |
| `CheckOverflow` **is** converted, but its `nullOnOverflow` field is **dropped** | `spark-extension/src/main/scala/org/apache/spark/sql/auron/NativeConverters.scala:1096` builds `Spark_CheckOverflow` from `(child, precision, scale)` only; the identifier `nullOnOverflow` appears nowhere in the repo |
| Native `Spark_CheckOverflow` has no null-on-overflow parameter and **always** nulls | `native-engine/datafusion-ext-functions/src/spark_check_overflow.rs` |
| `AggSum` accumulates with a plain `+` on `i128`, with no precision or overflow check | `native-engine/datafusion-ext-plans/src/agg/sum.rs:116` (`partial_update`), `:141` (`partial_merge`) |
### Possible fixes
**Option 1 (small).** In the aggregate conversion near `NativeConverters.scala:1217`, refuse to
convert `Sum`/`Average` whose `dataType` is a `DecimalType` when `SQLConf.get.ansiEnabled`. The
operator is then tagged `NeverConvert` and the aggregate runs on Spark, giving exact Spark
semantics including the error message Spark's own assertions check for. Roughly five lines. Costs
the native aggregate for ANSI decimal queries only.
**Option 2 (full fidelity, larger).**
1. Pass `e.nullOnOverflow` as a fourth boolean literal argument at `NativeConverters.scala:1096`,
and have `spark_check_overflow.rs` raise `"Overflow in sum of decimals"` instead of returning
`None` when it is false.
2. Convert `CheckOverflowInSum` at all — today it is dropped, so decimal `Sum` has no overflow
check in the native plan regardless of eval mode.
3. Give `AggSum`'s decimal path a `checked_add` and a precision check.
4. Map the resulting native error to `ArithmeticException` so Spark's `intercept[SparkException]`
and its cause assertions pass.
### Related risk worth a separate reproduction
Because `CheckOverflowInSum` is dropped **and** `AggSum` adds with an unchecked `+` on `i128`, a
decimal sum large enough to exceed `i128` would wrap around rather than saturate. The `TryCastExpr`
then has nothing to null — the wrapped value is a perfectly valid `Decimal128` — so the query would
return a **silently wrong number in non-ANSI mode**, which is more serious than this issue's
ANSI-only symptom. The `SPARK-28067` values exceed the declared precision (~1.2e38 unscaled) but
stay under `i128::MAX` (~1.7e38), so they null rather than wrap and the suite does not exercise
this. This is a hypothesis, not an observation — worth confirming with larger inputs.
### Environment
- Auron 7.0.0-incubating
- Spark 3.1.1 and 3.5.2 (reproduces on both)
- `spark.sql.ansi.enabled=true`
---
*Note: this issue's original root-cause section attributed the defect to the aggregate buffers
carrying no `failOnError` flag. That was imprecise. Inspecting the logged native plans showed the
aggregate runs natively as `Sum(TryCastExpr {...})` with no `CheckOverflow` node at all, which
locates the loss of information in the expression converter rather than in the accumulator. The
text above is the corrected analysis.*
Contributor guide
Research direction
Start with the failing DataFrameSuite cases for SPARK-28067 and SPARK-35955, then trace decimal aggregate conversion near NativeConverters.scala:1217. Read spark_check_overflow.rs and native-engine/datafusion-ext-plans/src/agg/sum.rs to compare native behavior with Spark's ANSI and non-ANSI branches. Done means ANSI overflow raises the expected exception while non-ANSI overflow remains NULL.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, scala, sql
- Domain
- backend, data-engineering
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100