andygrove commented on code in PR #6735:
URL: https://github.com/apache/datafusion-comet/pull/6735#discussion_r4205167409


##########
spark/src/main/scala/org/apache/comet/serde/CometSortOrder.scala:
##########
@@ -62,26 +60,14 @@ object CometSortOrder extends 
CometExpressionSerde[SortOrder] {
       "places it by the null order " +
       "([#6476](https://github.com/apache/datafusion-comet/issues/6476))."
 
-  private val nullableNestedFloatingPointSort =
-    "Sorting on floating-point values nested in an array or struct that can 
hold a null " +
-      "element or field"
-
-  override def getIncompatibleReasons(): Seq[String] = Seq(
-    nestedNullOrderReason,
-    s"$nullableNestedFloatingPointSort is not 100% compatible with Spark when 
" +
-      s"`${CometConf.COMET_EXEC_STRICT_FLOATING_POINT.key}=true`")
+  override def getIncompatibleReasons(): Seq[String] = 
Seq(nestedNullOrderReason)
 
   override def getSupportLevel(expr: SortOrder): SupportLevel = {
-    val dataType = expr.child.dataType
-    if (!canHoldNestedNull(dataType)) {
-      Compatible()
-    } else if (expr.nullOrdering != expr.direction.defaultNullOrdering) {
+    if (canHoldNestedNull(expr.child.dataType) &&
+      expr.nullOrdering != expr.direction.defaultNullOrdering) {
       Incompatible(Some(nestedNullOrderReason))
     } else {
-      SupportLevel
-        .strictFloatingPointReason(dataType, nullableNestedFloatingPointSort)
-        .map(reason => Incompatible(Some(reason)))
-        .getOrElse(Compatible())
+      Compatible()

Review Comment:
   Fixed in 04b4123b0. `checkStrictNestedFloatingPointSort` now asserts a 
native `CometSortExec` and Spark's answer, with `SortOrder.allowIncompatible` 
both false and true. The separate opt-in block is folded into that loop. Both 
negative-zero sort tests pass locally on Spark 4.1. I also merged apache/main. 
The two Spark 4.2 `trunc_timestamp` failures are unrelated to this PR and are 
fixed by #6740.



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