sunchao commented on code in PR #6447:
URL: https://github.com/apache/datafusion-comet/pull/6447#discussion_r4174326162
##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -826,14 +773,12 @@ case class CometExecRule(session: SparkSession)
plan
}
} else {
- val normalizedPlan = normalizePlan(plan)
-
val planWithJoinRewritten = if (CometConf.COMET_FORCE_SHJ.get()) {
- normalizedPlan.transformUp { case p =>
+ plan.transformUp { case p =>
RewriteJoin.rewrite(p)
}
} else {
- normalizedPlan
+ plan
Review Comment:
[P2] Preserve divisor normalization until native extrema are
Spark-compatible. For a Spark-written Parquet `DOUBLE` column `d` containing
NaN, `SELECT least(1.0D / (-d), 0.0D), greatest(1.0D / (-d), 0.0D) FROM t`
previously matched Spark’s `(0.0, NaN)`. Removing `normalizePlan` passes a
negative NaN quotient into the native extrema kernels, which return `(NaN,
0.0)` in both ANSI modes on x86-64. This exposes their existing ordering
limitation on a previously correct projection. The hash fix does not protect
these consumers. Retain the arithmetic divisor wrapper or correct extrema
ordering before removing it, with SQL regression coverage.
Evidence: An exact-head disposable Rust test exercised `create_negate_expr`,
the non-ANSI `IfExpr` zero guard or ANSI `checked_div`, and the
`least`/`greatest` functions returned by `create_comet_physical_fun`. Restoring
the former divisor wrapper produced `(0.0, NaN)` in both modes. Without it,
both produced `(NaN, 0.0)` and the regression assertion failed. A fresh Spark
3.5.9 run over Spark-written Parquet returned `(0.0, NaN)` in both modes.
Spark’s supported-version extrema use SQL ordering, which places every NaN
above finite values. Reproduction: `/tmp/review6447-864c2-session-repro.rs`.
Native output: `/tmp/review6447-864c2-session-repro.log`. Spark output:
`/tmp/review6447-864c2-session-spark-oracle.log`.
--
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]
