peterxcli commented on code in PR #5166:
URL: https://github.com/apache/datafusion-comet/pull/5166#discussion_r3741191150
##########
spark/src/main/scala/org/apache/comet/serde/aggregates.scala:
##########
@@ -861,7 +872,12 @@ object CometCollectSet extends
CometAggregateExpressionSerde[CollectSet] {
inputs: Seq[Attribute],
binding: Boolean,
conf: SQLConf): Option[ExprOuterClass.AggExpr] = {
- val child = expr.children.head
+ val child = aggExpr.mode match {
+ case Partial | Complete if isSpark42Plus =>
+ CometExecUtils.normalizeFloatingNumbers(expr.children.head)
Review Comment:
> Two follow-ups. Could the PR description mention this so the
execution-mode change is on the record? And would you file an issue to teach
the native `NormalizeNaNAndZero` in
`native/spark-expr/src/math_funcs/internal/normalize_nan.rs` to recurse into
`List` and `Struct`, so the serde can emit a single native node for the nested
shapes?
1.
2. teach the native `NormalizeNaNAndZero` to recurse into `List` and
`Struct`: plan to address this issue along with my new review:
1.
https://github.com/apache/datafusion-comet/pull/5235#pullrequestreview-4889228174,
2.
https://github.com/apache/datafusion-comet/pull/5235#discussion_r3741122515,
3.
https://github.com/apache/datafusion-comet/pull/5235#discussion_r3741131088
> Worth noting in that issue that with
`spark.comet.exec.scalaUDF.codegen.enabled=false` the `ArrayTransform`
serializes to `None`, which makes `convert` return `None` and the whole
aggregate fall back to Spark. That degrades safely, which is good, but it is
slower than it needs to be.
I added this in https://github.com/apache/datafusion-comet/issues/5312
follow-up per another inline review,
https://github.com/apache/datafusion-comet/pull/5166#discussion_r3732293880
--
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]
