sunchao commented on code in PR #6447:
URL: https://github.com/apache/datafusion-comet/pull/6447#discussion_r4162029591
##########
spark/src/main/scala/org/apache/comet/rules/CometExecRule.scala:
##########
@@ -763,14 +710,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 NaN hashing is corrected. On
x86-64, for a Parquet `DOUBLE` column `d` containing canonical NaN, `SELECT
hash(1.0D / (-d)), xxhash64(1.0D / (-d)) FROM t` previously matched Spark. The
native path now returns `(-1489914710, 9200374361256412029)` instead of
`(-1281358385, -3127944061524951246)` in both ANSI modes. Removing
`normalizePlan` also removes the `Divide` divisor wrapper. The new comparison
normalization covers the zero-divisor check, but arithmetic still consumes the
original negative NaN. This exposes the existing hash limitation on a
previously correct projection. Retain the arithmetic wrapper or canonicalize
NaNs in both hash paths before removing it.
Evidence: An exact-head disposable Rust test used `create_negate_expr`, the
legacy `IfExpr` zero guard or ANSI `checked_div`, and both shipped hash
functions. With the former divisor wrapper, quotient bits were
`0x7ff8000000000000` and both hashes matched Spark. Without it, bits were
`0xfff8000000000000` and both hashes differed. The regression assertion failed
in both modes. Spark 3.5.9 executed the SQL over Spark-written Parquet and
returned the expected pair in both modes. Spark’s supported-version
`hash.scala` implementations canonicalize NaNs through
`Double.doubleToLongBits`. Reproduction:
`/tmp/review6447-current-1790903311-repro.rs`; native output:
`/tmp/review6447-current-1790903311-repro.log`; Spark output:
`/tmp/review6447-current-1790903311-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]
