sunchao commented on code in PR #6735:
URL: https://github.com/apache/datafusion-comet/pull/6735#discussion_r4200011279
##########
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:
[P2] Update the existing strict nested-sort assertions when removing this
fallback. With `strictFloatingPoint=true` and
`SortOrder.allowIncompatible=false`, default ascending sorts over nullable
float arrays/structs now correctly return `Compatible()`, but
`CometExpressionSuite.checkStrictNestedFloatingPointSort` still calls
`checkSparkAnswerAndFallbackReason(..., "can hold a null element or field")`.
Consequently, both `sort array of floating point with negative zero` and `sort
struct containing floating point with negative zero` fail on every supported
Spark profile, blocking required CI. Change that helper to assert native
`CometSortExec` execution and Spark-equivalent results under the
disabled-incompatibility configuration.
Evidence: Exact-head Spark 4.1 CI reports both failures with `Expected
fallback reason 'can hold a null element or field' but no fallback reasons were
found`, and its plan contains `CometSort`:
https://github.com/apache/datafusion-comet/actions/runs/37516839080/job/112455744290.
The same failures occur in the Spark 3.4, 3.5, 4.0 and 4.2 expression jobs in
run 37516849615. The helper at CometExpressionSuite.scala:247–278 is unchanged
from the base. After building native code, a focused reproduction is `./mvnw
test -Dtest=none -Dsuites="org.apache.comet.CometExpressionSuite negative
zero"`.
--
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]
