peter-toth commented on code in PR #7942:
URL: https://github.com/apache/arrow-datafusion/pull/7942#discussion_r1432393865


##########
datafusion/expr/src/expr.rs:
##########
@@ -81,8 +81,10 @@ use std::sync::Arc;
 ///   assert_eq!(binary_expr.op, Operator::Eq);
 /// }
 /// ```
-#[derive(Clone, PartialEq, Eq, Hash, Debug)]
+#[derive(Clone, PartialEq, Eq, Hash, Debug, Default)]
 pub enum Expr {
+    #[default]

Review Comment:
   The reason why I added `Expr::Nop` is to be able to refactor `unalias()` to 
be self mutating one: 
https://github.com/apache/arrow-datafusion/pull/7942/files#diff-204cfc4f999c3d12dc065f323cb952fb0ecb33c5570eed8dc1fb52b806e87004R960.
 I needed a dummy `Expr` for `mem::take()`, but as you showed in 
https://github.com/apache/arrow-datafusion/pull/7942/files#r1431799873 
`unalias()` doens't need to be self mutating.
   
   But, maybe having a default dummy value of `Expr` is still useful in some 
cases, like in @sadboy's PR: 
https://github.com/apache/arrow-datafusion/pull/8591/files#diff-6515fda3c67b9d487ab491fd21c27e32c411a8cbee24725d737290d19c10c199R388-R389,
 
https://github.com/apache/arrow-datafusion/pull/8591/files#diff-6515fda3c67b9d487ab491fd21c27e32c411a8cbee24725d737290d19c10c199R427-R428,
 `Expr::Wildcard { qualifier: None }` is used for such purposes.



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

Reply via email to