sunchao commented on PR #5420: URL: https://github.com/apache/datafusion-comet/pull/5420#issuecomment-5655883462
Rebased onto current main in `45800b356`. Thanks, Andy. I addressed the accumulator and configuration points: - Grouped merge overflow now records a sticky invalid state and raises during evaluation. Tests cover precision and `i128` overflow, prefix emission, state round trips, and later updates in ANSI, legacy, and TRY modes. - Removed the redundant scalar `is_empty` field. - Changed grouped ANSI decimal AVG from `Unsupported` to `Incompatible`. Users can opt in with `spark.comet.operator.HashAggregateExec.allowIncompatible=true` or `spark.comet.operator.ObjectHashAggregateExec.allowIncompatible=true`, according to the operator. Decimal buffer boundaries and global/window restrictions still apply. The default remains conservative. Deferring errors helps prefix emission, but native finalization can still evaluate an unconsumed group within a batch. I agree that queries which consume every group pay for a fallback they do not need. A top-level `TakeOrderedAndProjectExec` alone does not establish that guarantee: Spark can use `take(limit)` when its child already provides the requested ordering, and AQE can plan aggregate stages separately from their consumer. I documented that limitation and the opt-in. Added `--grouped-ansi-avg` to compare default fallback and native opt-in on the same ANSI workload, with result and plan checks outside timing. Measurements remain pending: the configured local registry cannot resolve the rebased lockfile's DataFusion 55.1.0, preventing a fresh native build and benchmark run. The [previous hosted run](https://github.com/apache/datafusion-comet/actions/runs/34438085805) passed 1,273 Rust tests and 862 Spark 4.1 execution tests. Its six failing jobs hit the same redundant string-interpolation prefix in the benchmark; I removed it. Those results cover the previous synthetic merge. The new revision needs its own hosted CI results. Full-reactor Spark 4.0.4 test compilation, syntactic Scalafix 0.14.6, Rust formatting, Spotless, Scalastyle, and whitespace checks pass locally. -- 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]
