apache / apache/auron

Decimal sum/avg overflow returns NULL under ANSI mode instead of raising, because CheckOverflowInSum is dropped during conversion

Open
#2,476 1 comment 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.