sunchao commented on code in PR #5420:
URL: https://github.com/apache/datafusion-comet/pull/5420#discussion_r4178965761
##########
spark/src/main/scala/org/apache/comet/serde/aggregates.scala:
##########
@@ -223,12 +223,12 @@ object CometCount extends
CometAggregateExpressionSerde[Count] {
object CometAverage extends CometAggregateExpressionSerde[Average] {
- // Keep the default native-to-Spark restriction until #5420: an untouched
native AVG emits
- // (null, 0), but Spark's merge needs (0.0, 0).
+ // Non-decimal AVG emits Spark's (sum: double, count: long) buffer,
including (0.0, 0) for
+ // empty partials. Decimal AVG nulls count differently on overflow and
remains unsafe to mix.
+ override def supportsNativePartialToSparkFinal(fn: Average): Boolean =
Review Comment:
[P2] Update the remaining AQE fallback fixtures alongside this AVG opt-in.
`CometExecRuleSuite` tests `aggregate fallback removes nested placeholders
around an existing AQE stage` and `aggregate fallback executes after an AQE
round-robin repartition` still use nondecimal `AVG(id)`/`AVG(v)` and expect two
Spark aggregates plus an unsafe-partial marker. This override correctly retains
the native partial, so both tests now fail and every supported Spark profile's
execution job is red. Use an unsafe buffer such as `AVG(CAST(... AS DECIMAL(20,
2)))`, updating the expected decimal results, so these tests continue
exercising fallback and placeholder cleanup rather than weakening their
assertions.
Evidence: Exact-head run 37221483291 reproduces both failures on Spark 3.4,
3.5, 4.0, 4.1 and 4.2. Spark 4.1 job
https://github.com/apache/datafusion-comet/actions/runs/37221483291/job/111497284546
reports `had size 1 instead of expected size 2` at
CometExecRuleSuite.scala:220 and :260. Both plans contain a Spark final above a
`CometHashAggregate` partial. The job finishes with 1,265 tests passed and
these two failed.
--
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]
