comphead commented on code in PR #7220:
URL: https://github.com/apache/datafusion-comet/pull/7220#discussion_r4379461080
##########
spark/src/test/scala/org/apache/comet/exec/CometAggregateSuite.scala:
##########
@@ -471,6 +471,106 @@ class CometAggregateSuite extends CometTestBase with
AdaptiveSparkPlanHelper {
}
}
+ for {
+ dataType <- Seq("FLOAT", "DOUBLE")
Review Comment:
The decision this matrix pins depends only on the element type, the mode and
the Spark version, and the SQL file harness covers all of it.
`collect_set_floating_fallback.sql` (`-- MaxSparkVersion: 4.1`) already has
FLOAT and DOUBLE tables with NaN, signed zeros, infinities and NULLs, and a
`ConfigMatrix` over `useObjectHashAggregateExec`. A sibling file with `--
Config: spark.comet.exec.strictFloatingPoint=false`, `ConfigMatrix` lines over
`useObjectHashAggregateExec` and `spark.sql.adaptive.enabled`, and `query
expect_fallback(collect_set on FLOAT/DOUBLE in PartialMerge requires Spark
4.2)` over `count(DISTINCT ...)` beside `collect_set` would cover the
native-input half. A `VALUES` source with
`spark.comet.exec.localTableScan.enabled=false` would cover the Spark-partial
half. On 4.2, the same query added to `collect_set_normalization.sql` (`--
MinSparkVersion: 4.2`) checks the native answer. That replaces the 16 generated
Scala tests with a few queries compared against Spark, an
d the control test fits as plain `query` blocks in the same files.
--
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]
