sunchao commented on PR #5421: URL: https://github.com/apache/datafusion-comet/pull/5421#issuecomment-5691933060
Thanks @comphead. Addressed these in [4b5f1704d](https://github.com/apache/datafusion-comet/commit/4b5f1704dab8790363bd3ef1a90a5ed57d51ef10). The removals in `fe66f74643` were deliberate simplification, rather than a rebase loss. They removed useful diagnostics and coverage along with the larger test matrix. This update restores the focused cases you identified while keeping the simpler structure. 1. **Decimal SUM and the remaining audit:** decimal SUM cannot opt in yet despite its matching buffer layout. With one ordered partition of `DECIMAL(38,38)` values `0.6, 0.6, -0.6`, Spark's generated scalar SUM returns `0.6` in both Legacy and ANSI. A control using the matching native library and asserting native Partial/Final stages returns NULL in Legacy and throws SUM overflow in ANSI: the native update makes the intermediate precision overflow sticky or throws before cancellation. The new regression verifies that the mixed plan keeps the Partial in Spark, returns `0.6`, records the fallback tag, and retains native input work. One small source correction: the untouched native scalar state is `(0, true)`, not `(null, true)`; the exclusion is about update semantics. I filed [#5975](https://github.com/apache/datafusion-comet/issues/5975) for the remaining forward-compatibility audits and updated the description to state the loss of native aggregation coverage with shuffle disabled , including decimal SUM going from one native stage to zero. 2. **Conditional diagnostic:** restored a read-only check along the known aggregate/exchange/AQE path. If repair fails with an incompatible native Partial or PartialMerge remaining, it warns once and attaches an EXPLAIN reason to the Spark Final. It covers the stage-placeholder shape you described without rewriting existing stages. Tests cover repeat passes, both fallback-logging settings, and benign boundaries. This is synthetic planner coverage; I am not claiming a naturally occurring SQL reproduction for that shape. 3. **Focused coverage:** restored tests for the immediate reason with a non-native child, independent direction defaults, and a sticky prior refusal of the lower shuffle. `revertUnsafePartialAggregates` is again `private[rules]` so the diagnostic can be tested directly. Validation: 107 tests passed on Spark 4.1.3 and 105 on Spark 3.4.3, with two expected version-specific cancellations on 3.4. Compilation and style checks passed on both. Runtime checks used the refreshed [Linux CI native artifact](https://github.com/apache/datafusion-comet/actions/runs/34914245755/attempts/2), whose native source tree matches this update; the separate decimal SUM control also verified the loaded library. [CI for the new commit](https://github.com/apache/datafusion-comet/actions/runs/35054516968) is running. -- 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]
