mingmwang commented on issue #2175: URL: https://github.com/apache/arrow-datafusion/issues/2175#issuecomment-1262630212
> I wonder if we could extend PhysicalExpr to require `PartialEq` > > So like change > > https://github.com/apache/arrow-datafusion/blob/41b4e491663029f653e491b110d0b5e74d08a0b6/datafusion/physical-expr/src/physical_expr.rs#L37 > > to be > > ```rust > pub trait PhysicalExpr: Send + Sync + Display + Debug + PartialEq { > ``` > > This would certainly still require code churn downstream (to implement PartialEq) but it wouldn't be nearly as large as changing to an `enum` Thanks for the suggestion. I can try this way first. But since Logical Expressions are already enums, why we have the Physical Exprs stick to Trait. In the current code base, there are only a few physical optimization rules, I think when people try to add more and more optimization rules, we will see more inconvenience to the Trait. -- 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]
