sunchao commented on code in PR #5420:
URL: https://github.com/apache/datafusion-comet/pull/5420#discussion_r4179044518
##########
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:
Fixed in `86f25149fb21a472a67d7fa3c0d929fc2931f6c5`. Both fixtures now use
`AVG(CAST(... AS DECIMAL(20, 2)))`, with expected decimal results `15.000000`,
`40.000000` and null. All existing aggregate-count, unsafe-partial, AQE-stage,
shuffle and placeholder-cleanup assertions are retained. The branch is also
rebased onto current main.
Both repaired tests pass on Spark 3.5 and 4.1 using the verified native
artifact built from this head. The native nondecimal AVG admission control also
passes on both profiles, and the ObjectHash decimal AVG control passes on 4.1:
three selected tests on 3.5 and four on 4.1. Full root-reactor compilation,
strict Scala 2.12 warnings, Spotless and Scalastyle pass. Broader CI is still
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]
