comphead commented on code in PR #6110:
URL: https://github.com/apache/datafusion-comet/pull/6110#discussion_r4354978550
##########
spark/src/test/scala/org/apache/comet/exec/CometColumnarShuffleSuite.scala:
##########
@@ -838,6 +836,107 @@ abstract class CometColumnarShuffleSuite extends
CometTestBase with AdaptiveSpar
}
}
+ // The struct is orderable by the JVM, but CometSortOrder reports
Incompatible for it: its
+ // fields are nullable, so canHoldNestedNull holds, and with the default
null ordering the rule
+ // falls through to the strict floating-point case (#6476). The columnar
path never sends the
+ // sort order to native code, so that verdict must not gate it (#5971).
+ test("range partitioning on a nested floating-point key uses columnar
shuffle") {
+ withSQLConf(
+ CometConf.COMET_EXEC_STRICT_FLOATING_POINT.key -> "true",
+ CometConf.getExprAllowIncompatConfigKey("SortOrder") -> "false") {
+ withParquetTable((0 until 20).map(i => (i.toDouble, i)),
"range_struct_tbl") {
+ val df = sql("SELECT struct(_1 AS a, _2 AS b) AS c FROM
range_struct_tbl")
+ .repartitionByRange(4, col("c"))
+ checkShuffleAnswer(df, 1)
+ }
+ }
+ }
+
+ // A Scala UDF has no serde once the codegen dispatcher is off, so
exprToProto returns None for
+ // it. The columnar path evaluates partition keys on the JVM through
UnsafeProjection, so that
+ // verdict must not gate the exchange (#5971). Turning the dispatcher off is
only a cheap and
+ // stable way to obtain an unserializable partition key; any expression
without a serde reaches
+ // the same gate, and unlike the strictFloatingPoint cases above nothing
here depends on strict
+ // mode.
+ test("range partitioning on an unserializable expression uses columnar
shuffle") {
+ withSQLConf(CometConf.COMET_SCALA_UDF_CODEGEN_ENABLED.key -> "false") {
+ withParquetTable((0 until 20).map(i => (i, i.toString)),
"range_udf_tbl") {
+ val bump = udf((x: Int) => x + 1)
+ val df = sql("SELECT _1, _2 FROM range_udf_tbl").repartitionByRange(4,
bump(col("_1")))
+ checkShuffleAnswer(df, 1)
+ }
+ }
+ }
+
+ test("hash partitioning on an unserializable expression uses columnar
shuffle") {
Review Comment:
As far as I can tell this test is covered by `columnar shuffle partition
assignment matches Spark for keys Comet cannot serialize` further down. Both
turn off `spark.comet.exec.scalaUDF.codegen.enabled`, repartition on a Scala
UDF hash key and call `checkCometExchange(df, 1, false)`, and the assignment
test also compares `spark_partition_id()` per row. Could this one be dropped,
or could `checkPartitionAssignmentMatchesSpark` also call `checkSparkAnswer` so
the answer check is kept?
--
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]
