sunchao opened a new issue, #5975: URL: https://github.com/apache/datafusion-comet/issues/5975
### What is the problem the feature request solves? PR #5421 makes native Partial → Spark Final compatibility an independent, explicit opt-in. This repairs unsafe aggregate boundaries, but also removes native partial aggregation for functions whose forward compatibility has not been established. With `spark.comet.exec.shuffle.enabled=false`, this is a concrete loss of native aggregation coverage: decimal SUM cases in `CometAggregateSuite`, for example, move from one native aggregate to zero. This issue tracks recovering coverage where supported by evidence. Matching buffer schemas or ordinary-input results alone do not establish compatibility. Decimal SUM illustrates the distinction. Its `(sum, isEmpty)` layout matches Spark, but its native accumulator makes intermediate precision overflow sticky, or throws immediately in ANSI mode. Spark's generated scalar aggregation can retain a wider temporary sum and recover after cancellation: `DECIMAL(38,38)` values `0.6, 0.6, -0.6` can return `0.6`. The audit must establish safe subcases or repair these semantics before enabling them. ### Describe the potential solution Audit the remaining functions individually, beginning with: - [ ] Decimal SUM, including LEGACY, ANSI, and TRY modes. - [ ] FIRST / LAST, including both `ignoreNulls` settings. - [ ] Standard deviation, variance, covariance, correlation, and regression aggregates. - [ ] MinBy / MaxBy. - [ ] Mode. - [ ] Exact and approximate percentile aggregates. For each function: 1. Compare actual native state with Spark's merge inputs across supported Spark versions: field order, types, nullability, flags, serialization, and meaning. Check scalar and grouped accumulators separately. 2. Cover empty and all-null partitions, zero surviving rows, and mixed empty/nonempty partitions. Check overflow, cancellation, error timing, precision/scale, NaN/infinity, and order-dependent behavior where applicable. 3. Validate with a source-matched native build. Execution tests must assert that a native Partial actually feeds a Spark Final; a fallback plan is not evidence of interoperability. Include AQE on/off and initial/materialized plans, plus PartialMerge and colocated aggregate expressions where relevant. 4. Enable proven-safe cases through `supportsNativePartialToSparkFinal`, documenting the state contract and retaining independent reverse-direction eligibility. 5. Keep unsafe cases in Spark, document the concrete reason, and link any focused repair needed. Do not opt in functions solely because their schemas match. Record the audited scope and remaining restrictions per function. Known representation mismatches, including collection buffers, remain excluded until their interoperability is repaired. ### Additional context Requested in [the second review of #5421](https://github.com/apache/datafusion-comet/pull/5421#issuecomment-5690517929). #5419 tracks the unsafe-boundary bug; this issue tracks recovering native coverage. AVG state fixes are tracked by #5418 / #5420. Reverse-direction COUNT support (#4242) and approx_count_distinct mixed support (#4820) have separate scope. -- 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]
