sunchao commented on code in PR #5421:
URL: https://github.com/apache/datafusion-comet/pull/5421#discussion_r3874670632
##########
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:
Confirmed and fixed in
[46ad7eca3](https://github.com/apache/datafusion-comet/commit/46ad7eca3c45be564d4edf065ae3821167acf954).
With this branch's freshly rebuilt native library, both AQE variants
reproduced `(COUNT=1, AVG=NULL)` instead of `(1, 1.0)` on the published policy,
with a Comet Partial feeding Spark Final.
`CometAverage.supportsNativePartialToSparkFinal` now explicitly returns
false until the emitted-state fix in #5420 is present. COUNT-only
NativePartialToSparkFinal remains enabled. AVG's existing non-decimal
SparkPartialToNativeFinal support is unchanged, as is fully native COUNT+AVG
execution. This restriction belongs to the prerequisite with the old AVG
implementation; it is not being copied blindly into #5420's repaired native
path.
The new regressions pin four Parquet partitions and cover one surviving
value, an all-null value, and no rows with AQE on/off and both early and
post-conversion fallback. They check the fallback tag and retained native
filter, plus a two-stage native control. I updated the existing AVG plan
expectations rather than leaving tests that claimed this unsafe direction was
supported.
The exact native build and full Spark 4.1.3 JVM reactor/style checks pass,
as do all **49 focused tests** (31 planner, 18 execution). The prior local run
reused #5420's already-repaired native library and did not cover this
standalone COUNT+AVG boundary; the new before/after runs use this branch's
native source with the loaded library hash verified. No full-suite or
multi-version run is claimed.
--
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]
