andygrove opened a new pull request, #5145: URL: https://github.com/apache/datafusion-comet/pull/5145
## Which issue does this PR close? Cherry-pick of #5080 (merged to `main` as `a1b022b52`) onto `branch-1.0`. Closes #5066 for the 1.0.0 release branch. ## Rationale for this change `MakeDecimal` with `nullOnOverflow = false` (ANSI mode) must fail on overflow, matching Spark's `Decimal.set(unscaled, precision, scale)`, which throws `NUMERIC_VALUE_OUT_OF_RANGE`. The Scala serde already serializes `failOnError = !nullOnOverflow`, but the native registration of `make_decimal` discarded the flag, so Comet returned NULL where Spark throws. `MakeDecimal` is generated by the `DecimalAggregates` optimizer rule rather than written by users, so the divergence surfaces on ordinary decimal aggregations — a silently wrong `NULL` instead of the error Spark raises. That is worth carrying into 1.0.0 rather than shipping as a known ANSI-mode correctness gap. ## What changes are included in this PR? A clean `git cherry-pick -x` of `a1b022b52`. The diff is byte-identical to the commit on `main`; there were no conflicts and no adaptation was needed. - `spark_make_decimal` now takes `fail_on_error` and raises `SparkError::NumericValueOutOfRange` (via the existing `decimal_overflow_error` helper, as `CheckOverflow` already does) when the unscaled long does not fit the target precision. Non-ANSI behavior is unchanged: overflow becomes NULL. - The `make_decimal` registration in `comet_scalar_funcs.rs` threads `fail_on_error` through instead of dropping it. - Test-only: `createMakeDecimalColumn` in `ShimCometTestBase` (all three Spark shim variants — `spark-3.4`, `spark-3.5`, `spark-4.x`) gained an overload that accepts `nullOnOverflow`, since the existing helper hardcoded the null-on-overflow behavior. ## How are these changes tested? By the tests added in #5080, verified on this branch: - `cargo test -p datafusion-comet-spark-expr make_decimal` — 4 passed. Covers array and scalar overflow with `fail_on_error` set (error) and unset (NULL), plus NULL input under `fail_on_error`. - `CometExpressionSuite` filtered to `make decimal` — 3 passed, including `make decimal using DataFrame API - overflow throws when nullOnOverflow=false`, which asserts Spark and Comet both fail with the `cannot be represented as Decimal(3, 0)` message. - Because this touches all three test shim variants, also confirmed `test-compile` succeeds under `-Pspark-3.4` and `-Pspark-4.0` (JDK 17) in addition to the default `-Pspark-3.5`. -- 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]
