andygrove commented on code in PR #11585: URL: https://github.com/apache/datafusion/pull/11585#discussion_r1685811723
########## datafusion/physical-expr/src/expressions/binary.rs: ########## @@ -289,6 +289,14 @@ impl PhysicalExpr for BinaryExpr { return apply_cmp_for_nested(self.op, &lhs, &rhs); } + if left_data_type.is_floating() { + lhs = normalize_floating_zeros(lhs, &left_data_type)?; + }; + + if right_data_type.is_floating() { + rhs = normalize_floating_zeros(rhs, &right_data_type)?; + } Review Comment: I'm curious to know how much overhead we will be adding to all binary operations on floating point numbers with this normalization approach. It would be good to have some criterion benchmarks in place so we can assess this. The alternative to normalizing zeroes would be to update the comparison kernels to handle the negative zero case and this would avoid introducing extra array transformations. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org