comphead commented on code in PR #7115:
URL: https://github.com/apache/datafusion-comet/pull/7115#discussion_r4361444795


##########
spark/src/test/scala/org/apache/comet/exec/CometAggregateSuite.scala:
##########
@@ -4480,4 +4480,27 @@ class CometAggregateSuite extends CometTestBase with 
AdaptiveSparkPlanHelper {
     }
   }
 
+  // Before SPARK-45599 (fixed in 3.5.2 and 4.0.0), mode's frequency map 
hashes a float by its bits
+  // but matches keys with ==, so every NaN is a key of its own (#6738).
+  test("mode float keys by Spark patch version") {
+    Seq("3.4.3", "3.5.0", "3.5.1", "3.5.1-SNAPSHOT").foreach { version =>
+      assert(!CometMode.keysNaNsTogether(version), version)
+    }
+    Seq("3.5.2", "3.5.9", "3.6.0", "4.0.0", "4.1.3", "4.2.0-SNAPSHOT").foreach 
{ version =>
+      assert(CometMode.keysNaNsTogether(version), version)
+    }
+  }
+
+  test("mode support level for float input follows the Spark version") {

Review Comment:
   This test computes its expectation from 
`CometMode.keysNaNsTogether(SPARK_VERSION)`, the same predicate 
`getSupportLevel` uses, so the float branch is only checked against itself. 
`mode float keys by Spark patch version` pins the predicate at each boundary, 
`mode_floating_point_spark_3_4.sql` asserts the fallback end to end on 3.4, 
`mode_floating_point.sql` asserts the native path from 3.5, and `mode.sql` 
already covers the `IntegerType` `Incompatible` case. Could this test be 
dropped, keeping only the boundary test?



-- 
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]

Reply via email to