[I] SUM(decimal) returns NULL or throws under ANSI when an intermediate sum overflows the buffer precision but the final result fits [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove opened a new issue, #6002:
URL: https://github.com/apache/datafusion-comet/issues/6002

   ### Describe the bug
   
   Comet's native decimal `SUM` validates the accumulator against the sum buffer's precision on **every update** and latches the failure. Spark does not check intermediate values at all in several common buffer layouts, so it can recover from a temporary overflow and return a result that fits.
   
   Where the two disagree, Comet returns `NULL` in legacy mode and raises `ARITHMETIC_OVERFLOW` under ANSI, while Spark returns a value. The aggregate runs entirely in Comet, so no mixed Comet/Spark plan is needed to hit this.
   
   ### Steps to reproduce
   
   One partition, no `GROUP BY`, whole-stage codegen enabled:
   
   ```sql
   CREATE TABLE t AS
     SELECT CAST(v AS DECIMAL(38,38)) AS v FROM VALUES ('0.6'), ('0.6'), ('-0.6') AS s(v);
   
   SELECT SUM(v) FROM t;
   ```
   
   `DECIMAL(38,38)` matters because `Sum.resultType` is `DecimalType.bounded(p + 10, s)`, which saturates at `(38, 38)` and leaves no headroom. The running sum reaches `1.2`, which does not fit in that precision, and the third value brings it back to `0.6`, which does.
   
   ### Expected behavior
   
   Spark returns `0.6` in both legacy and ANSI mode.
   
   Comet returns `NULL` in legacy mode and throws `ARITHMETIC_OVERFLOW` under ANSI.
   
   ### Additional context
   
   **Why Spark differs.** `Sum.add` uses `DecimalAddNoOverflowCheck` for decimal inputs. The doc comment on that expression says the overflow check is deliberately skipped because `UnsafeRowWriter` will perform it when the aggregation buffer is written. That only holds when the buffer actually is an `UnsafeRow`, and it is not in at least three places:
   
   - ungrouped `HashAggregateExec` under whole-stage codegen, which keeps the buffer in local `Decimal` variables (`doProduceWithoutKeys`),
   - window frames, where `AggregateProcessor` buffers into a `SpecificInternalRow`,
   - `ObjectHashAggregateExec`, which buffers the same way.
   
   In all three Spark keeps the wide intermediate and only `CheckOverflowInSum` at evaluate time decides. Grouped `HashAggregateExec` does write an `UnsafeRow`, so Spark nulls there too and Comet agrees.
   
   **Comet side.** `SumDecimalAccumulator::update_single` (`native/spark-expr/src/agg_funcs/sum_decimal.rs`, around line 202) and `SumDecimalGroupsAccumulator::update_single` (around line 421) both call `Decimal128Type::is_valid_decimal_precision` per row, then either set the sum to `None` permanently or raise immediately under ANSI. Once latched, a later cancelling value cannot recover the sum.
   
   **Scope.** This needs a decimal whose sum precision has little headroom, in practice an input precision of 28 or more, together with an intermediate that exceeds the buffer precision and later inputs that bring it back into range. `CometWindowExec` has no guard for the ever-expanding decimal `SUM` case either.
   
   **Related work.**
   
   - PR #5420 fixes exactly this class for decimal `AVG`, by keeping global and expanding-window decimal AVG in Spark once the sum precision reaches `DecimalType.MAX_PRECISION`. Decimal `SUM` has no equivalent guard in either the aggregate or the window path.
   - #5975 records the `0.6, 0.6, -0.6` behaviour, but as justification for an exclusion inside an enhancement about recovering native partial aggregate coverage. It does not track the wrong answer itself.
   - The measured control for the `DECIMAL(38,38)` case, run against a matching native library, is in https://github.com/apache/datafusion-comet/pull/5421#issuecomment-5504530845.
   - #4729 (closed) covered a different decimal SUM window overflow, where the result wrapped instead of returning NULL.
   
   **Suggested fix.** Either match Spark's deferred check by keeping an unbounded intermediate and validating once at `evaluate`, or add a precision guard mirroring the one in #5420, covering the ungrouped aggregate, the ever-expanding window frame, and `ObjectHashAggregate`.
   


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] SUM(decimal) returns NULL or throws under ANSI when an intermediate sum overflows the buffer precision but the final result fits [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove closed issue #6002: SUM(decimal) returns NULL or throws under ANSI when an intermediate sum overflows the buffer precision but the final result fits
URL: https://github.com/apache/datafusion-comet/issues/6002


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org