andygrove opened a new issue, #5066:
URL: https://github.com/apache/datafusion-comet/issues/5066
### Describe the bug
`MakeDecimal` returns NULL on overflow where Spark throws under ANSI mode.
The Scala serde serializes the flag correctly, but the native registration
discards it.
Spark 4.1 (`decimalExpressions.scala`): `MakeDecimal(child, precision,
scale, nullOnOverflow)` with `nullOnOverflow = !SQLConf.get.ansiEnabled`. With
`nullOnOverflow = false` it calls `Decimal.set(longInput, precision, scale)`,
which throws `NUMERIC_VALUE_OUT_OF_RANGE` on overflow.
Comet serde
(`spark/src/main/scala/org/apache/comet/serde/decimalExpressions.scala:56-60`)
passes `failOnError = !expr.nullOnOverflow` via
`scalarFunctionExprToProtoWithReturnType`, and the planner forwards it
(`native/core/src/execution/planner.rs`, `create_comet_physical_fun(...,
Some(expr.fail_on_error))`). But the native registration uses the macro arm
that does not thread the flag through:
```rust
// native/spark-expr/src/comet_scalar_funcs.rs:142-144
"make_decimal" => {
make_comet_scalar_udf!("make_decimal", spark_make_decimal, data_type)
}
```
and `spark_make_decimal`
(`native/spark-expr/src/math_funcs/internal/make_decimal.rs`) has no
`fail_on_error` parameter at all; `long_to_decimal` unconditionally returns
`None` on overflow.
### Steps to reproduce
`MakeDecimal` is generated by the `DecimalAggregates` optimizer rule rather
than user SQL, so it surfaces on decimal aggregations over low-precision
decimals (sum/avg of `DECIMAL(p, s)` with small `p`) where the unscaled long
overflows the target precision, with `spark.sql.ansi.enabled=true`. Spark
throws `NUMERIC_VALUE_OUT_OF_RANGE`; Comet returns NULL.
### Expected behavior
The native `make_decimal` should receive `fail_on_error` (compare the
`"spark_modulo"` registration in `comet_scalar_funcs.rs`, which uses the
`without $data_type, $fail_on_error` arm) and raise
`SparkError::NumericValueOutOfRange` on overflow when set.
### Additional context
Found by an ANSI-mode audit of all native expressions against Spark 4.1.1.
This is an instance of a general hazard: the scalar-function wiring makes it
easy to serialize `fail_on_error` and silently lose it natively.
--
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]
