andygrove commented on code in PR #6691:
URL: https://github.com/apache/datafusion-comet/pull/6691#discussion_r4220321902


##########
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:
   Good catch, thanks. Fixed in 5b0705ce9f. `is_infallible` now asks the 
`SparkComparison`, which counts as infallible when its two operands have the 
same type. Every flat `FLOAT` or `DOUBLE` comparison passes that check. For 
nested operands the check matters: the nested comparators reject a 
dictionary-encoded leaf against a plain one, and the logical type check in 
`spark_comparison` lets that pair through. Nested `=` and `<>` are still 
`NestedPredicate`s, which `is_infallible` didn't accept before this PR either, 
so they stay lazy.
   
   The `is_infallible` test now builds `d < 1.5`, `d = e` and `d <=> e` through 
`spark_comparison`, plus `<` and `<=>` on two `ARRAY<DOUBLE>` columns, and 
checks that an array compared with an array of dictionary-encoded doubles still 
counts as fallible. Leaving `SparkComparison` out of `is_infallible` fails the 
test, and so does counting every `SparkComparison` as infallible.
   
   I also ran your query over 8,192 rows in a scratch test. `CaseWhenExpr` 
takes the eager path again and returns the same rows as `CaseExpr`. In release 
mode that's about 16µs per batch, against about 90µs for the lazy path.
   



-- 
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]

Reply via email to