alamb commented on a change in pull request #9527: URL: https://github.com/apache/arrow/pull/9527#discussion_r578499671
########## File path: rust/datafusion/src/logical_plan/expr.rs ########## @@ -335,75 +335,77 @@ impl Expr { } } - /// Equal - pub fn eq(&self, other: Expr) -> Expr { - binary_expr(self.clone(), Operator::Eq, other) + /// Return `self == other` + pub fn eq(self, other: Expr) -> Expr { Review comment: The change is to have all these functions take `self` rather than `&self`. Given they almost always are used like `let expr = lit("foo").eq(lit("bar"))` self is not reused. Furthermore, the `clone()` is actually doing a deep clone, so in the above example `lit("foo")` gets copied twice. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org