andygrove commented on PR #5420: URL: https://github.com/apache/datafusion-comet/pull/5420#issuecomment-5876423243
This is a light fully automated review since there are so many PRs open. `avg()` at `native/spark-expr/src/agg_funcs/avg_decimal.rs:606` multiplies the sum by `10^(target_scale - sum_scale)`, which is `10^4` for most scales, in `i128` before dividing, and both callers (lines 328 and 551) turn its `None` into NULL. Spark's `DecimalDivideWithOverflowCheck` divides with `BigDecimal` and only checks the final result. That leaves two gaps in grouped decimal `AVG`, which this PR keeps native at every precision. With `v DECIMAL(38, 18)` and one group holding two `9000000000000000` rows, the unscaled sum `1.8e34` times `10^4` passes `i128::MAX`, so native returns NULL in every mode, while Spark's sum fits its `DECIMAL(38, 18)` buffer and it returns `9000000000000000`. For that type any group whose sum passes about `1.7e16` hits this, long before Spark's buffer overflows at `1e20`. And with a single `100000000000000000` row the average does not fit `DECIMAL(38, 22)`, so Spark raises `NUMERIC_VALUE_OUT_OF_RANGE` under ANSI where native returns NULL. Both predate this PR, but they sit in the `evaluate` paths it rewrites. Could `avg()` divide before scaling (or use `i256`), and raise `SparkError::NumericValueOutOfRange` in ANSI mode when the result really does not fit, with a grouped test covering both rows? If you'd rather keep this separate, could it get an issue and an entry next to the new grouped `AVG` line in the known result-value divergences list? -- 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]
