tustvold commented on code in PR #8908:
URL: https://github.com/apache/arrow-datafusion/pull/8908#discussion_r1460841842
##########
datafusion/expr/src/expr.rs:
##########
@@ -1869,10 +1869,14 @@ mod test {
let exp2 = col("a") + lit(2);
let exp3 = !(col("a") + lit(2));
- assert!(exp1 < exp2);
- assert!(exp2 > exp1);
- assert!(exp2 > exp3);
- assert!(exp3 < exp2);
+ // Since comparisons are done using hash value of the expression
Review Comment:
I haven't checked this, but if the PartialEq implementation isn't also using
this hash approach, the two would be inconsistent
Edit: as suspected it looks like PartialEq is being derived using a proc
macro, and is therefore inconsistent with this implementation of PartialOrd -
https://github.com/apache/arrow-datafusion/blob/main/datafusion/expr/src/expr.rs#L87.
In particular PartialOrd could claim things to be equal due to a hash
collision, when PartialEq would indicate they are not
--
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]
