andygrove commented on PR #4727:
URL:
https://github.com/apache/datafusion-comet/pull/4727#issuecomment-5876335103
This is a light fully automated review since there are so many PRs open.
Exempting `CollectList`/`CollectSet` from the `sparkPartialMergeMode` check
(`spark/src/main/scala/org/apache/spark/sql/comet/operators.scala:1931`) breaks
an assumption that `tagUnsafePartialAggregates` still depends on. When a
consumer can't convert, that pass only tags the bottom `Partial`, and its
comment at
`spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:1161` says the
`missingCometProducer` guard then cascades the fallback up through the
intermediate `PartialMerge` stages. That guard no longer fires for collect, so
the lower `PartialMerge` now converts over the tagged Spark `Partial` and sends
native `ArrayType` state into a Spark consumer that calls
`deserialize(inputBuffer.getBinary(...))` on it, which is the #1389 failure the
tagging exists to prevent. For example, take `SELECT k, sum(DISTINCT d),
collect_list(v) FROM t GROUP BY k` with `d DECIMAL(30, 2)`. The grouped
max-precision decimal `SUM` makes
`CometObjectHashAggregateExec.getSupportLevel` return
`Unsupported` for the two upper stages only, so the plan becomes Spark
`Partial` -> Comet `PartialMerge` -> Spark `{PartialMerge, Partial}` -> Spark
`Final`, where main keeps the whole chain in Spark.
`revertUnsafePartialAggregates` can't repair it because `revertChain` stops at
the Spark `Partial`, so it only records the "could not restore a native
intermediate buffer producer" warning. Could the tagging pass tag every
aggregate between the consumer and the bottom `Partial`, since `doConvert`
already honors the tag in any mode? A `collect_list` variant of `CometExecRule
should not split distinct aggregate with incompatible buffer (Spark final)` in
`CometExecRuleSuite` would catch this, since with the `Final` disabled the two
middle stages currently convert.
--
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]
