ziting-openai commented on code in PR #5421:
URL: https://github.com/apache/datafusion-comet/pull/5421#discussion_r3874119048
##########
spark/src/main/scala/org/apache/comet/serde/aggregates.scala:
##########
@@ -106,6 +106,10 @@ object CometMax extends CometAggregateExpressionSerde[Max]
{
}
object CometCount extends CometAggregateExpressionSerde[Count] {
+ // Both buffers are a single non-null Long. The AQE/count-bug restrictions
documented on the
+ // reverse direction concern a Comet Final; retaining Spark's Final
preserves those rewrites.
+ override def supportsNativePartialToSparkFinal(fn: Count): Boolean = true
Review Comment:
[P1] Do not let the new COUNT exemption expose incorrect AVG buffers
This newly enables a mixed aggregate such as `SELECT COUNT(*), AVG(value)
FROM parquet_input WHERE id = 1` to run with a Comet Partial and Spark Final:
`COUNT` now returns true here and non-decimal `AVG` already claims forward
compatibility. Previously `COUNT` forced the *entire* aggregate into Spark, so
this query was protected. With four input partitions and only one surviving
`BIGINT`, the existing native AVG defect independently demonstrated in
https://github.com/apache/datafusion-comet/pull/5421#issuecomment-5429161669
makes the newly admitted plan return `AVG = NULL` instead of `1.0`. The
dependent native AVG fix in #5420 is still unmerged, so landing this
prerequisite first introduces silent wrong results. Please keep mixed COUNT+AVG
plans in Spark until that fix lands (or mark native AVG-to-Spark-final unsafe)
and add a COUNT+AVG regression with empty partial partitions.
--
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]
