andygrove commented on code in PR #6447:
URL: https://github.com/apache/datafusion-comet/pull/6447#discussion_r4185761715
##########
native/spark-expr/src/comet_scalar_funcs.rs:
##########
@@ -315,6 +316,11 @@ pub fn create_comet_physical_fun_with_eval_mode(
// SparkMakeTime already throws on invalid input, so accept the flag
here rather
// than falling through to the registry fail-closed path.
"make_time" =>
Ok(Arc::new(ScalarUDF::new_from_impl(SparkMakeTime::new()))),
+ // Floats, and arrays and structs holding them, need Spark's float
ordering. Other types
+ // keep DataFusion's `greatest` and `least` from the registry.
+ "greatest" | "least" if SparkGreatestLeast::handles(&data_type) =>
Ok(Arc::new(
Review Comment:
This one comes from #6457, which merged yesterday, so main already has it.
Since the main merge in f2c2fc4e5, `comet_scalar_funcs.rs` isn't part of this
diff any more. `floating-point.md` describes the difference at the end of its
`min`, `max`, `greatest`, and `least` section.
I don't think there's a single Spark answer to match. On 4.1.3, over `(a, b)
= (0.0, -0.0)`, Spark returns `0.0, -0.0, 0.0, -0.0` for `greatest(a, b),
greatest(b, a), least(a, b), least(b, a)` once
`spark.sql.subexpressionElimination.enabled` is off. That is what Comet returns
under either setting. With elimination on, Spark evaluates one expression from
each pair and returns `0.0` all four times. A lone `greatest(b, a)` returns
`-0.0` from Spark under both settings, and from Comet too. Before #6457 Comet
returned `0.0` for it, because DataFusion orders `0.0` above `-0.0`, and that
same ordering is the only reason the paired query used to match.
To match the paired case we'd have to replace each expression in a
projection with the first semantically equal one before serializing it, which
is how `EquivalentExpressions` picks its representative. I can open an issue
for that if you think it's worth doing, but I'd rather not hold this PR for it.
--
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]
