viirya commented on code in PR #6691:
URL: https://github.com/apache/datafusion-comet/pull/6691#discussion_r4213005670
##########
native/spark-expr/src/array_funcs/nested_comparison.rs:
##########
@@ -217,31 +311,176 @@ impl PhysicalExpr for NestedPredicate {
}
}
+/// A comparison of two Float32 or Float64 operands, or of two lists or
structs with a float leaf,
+/// in Spark's SQL ordering: `-0.0` equals `0.0`, all NaNs are equal, and NaN
sorts above every
+/// other value. It reads the operands as they are, without normalized copies
of them.
+#[derive(Debug, Eq)]
+pub struct SparkComparison {
Review Comment:
Before this PR, a flat float comparison came out as
`BinaryExpr(FloatNormalize(d) < FloatNormalize(e))`. `is_infallible` in
`conditional_funcs/case_when.rs` accepts both of those nodes, so the comparison
counted as infallible. It doesn't recognize `SparkComparison`, though. A `CASE
WHEN` or `IF` that has a float comparison in a later `WHEN` or in a branch
value is no longer infallible, so it falls back from #6350's eager evaluation
to lazy evaluation.
I checked this with `is_infallible(spark_comparison(...))` for `d < 1.5`, `d
= e` and `d <=> e`. It returns `true` on #6447's build and `false` on this
branch. Then I timed this query over 8192 rows in release mode:
```sql
CASE WHEN i > 0 THEN 1 WHEN d < 0.5 THEN 2 WHEN d > 2.0 THEN 3 ELSE 4 END
```
It took about 22µs per batch before and about 75µs on this branch.
Could `is_infallible` accept a `SparkComparison`? Flat Float32/Float64
operands of the same type can't fail. The nested case only fails on a type
mismatch, which planning already rules out. Could you also add a case to the
`is_infallible` test in `case_when.rs` that builds the comparison through
`spark_comparison` rather than by hand, so this can't silently regress again?
--
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]
