[GitHub] [arrow] andygrove commented on a change in pull request #8746: ARROW-10688: [Rust] [DataFusion] Implement CASE WHEN logical plan

[GitBox <gi...@apache.org>]

andygrove commented on a change in pull request #8746:
URL: https://github.com/apache/arrow/pull/8746#discussion_r528909049



##########
File path: rust/datafusion/src/optimizer/utils.rs
##########
@@ -216,26 +238,51 @@ pub fn from_plan(
 
 /// Returns all direct children `Expression`s of `expr`.
 /// E.g. if the expression is "(a + 1) + 1", it returns ["a + 1", "1"] (as Expr objects)
-pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<&Expr>> {
+pub fn expr_sub_expressions(expr: &Expr) -> Result<Vec<Expr>> {
     match expr {
-        Expr::BinaryExpr { left, right, .. } => Ok(vec![left, right]),
-        Expr::IsNull(e) => Ok(vec![e]),
-        Expr::IsNotNull(e) => Ok(vec![e]),
-        Expr::ScalarFunction { args, .. } => Ok(args.iter().collect()),
-        Expr::ScalarUDF { args, .. } => Ok(args.iter().collect()),
-        Expr::AggregateFunction { args, .. } => Ok(args.iter().collect()),
-        Expr::AggregateUDF { args, .. } => Ok(args.iter().collect()),
-        Expr::Cast { expr, .. } => Ok(vec![expr]),
+        Expr::BinaryExpr { left, right, .. } => {
+            Ok(vec![left.as_ref().to_owned(), right.as_ref().to_owned()])
+        }
+        Expr::IsNull(e) => Ok(vec![e.as_ref().to_owned()]),
+        Expr::IsNotNull(e) => Ok(vec![e.as_ref().to_owned()]),
+        Expr::ScalarFunction { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+        Expr::ScalarUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+        Expr::AggregateFunction { args, .. } => {
+            Ok(args.iter().map(|e| e.clone()).collect())
+        }
+        Expr::AggregateUDF { args, .. } => Ok(args.iter().map(|e| e.clone()).collect()),
+        Expr::Case {
+            expr,
+            when_then_expr,
+            else_expr,
+            ..
+        } => {
+            let mut expr_list: Vec<Expr> = vec![];

Review comment:
       This felt a bit hacky but our expression rewrite mechanism doesn't really work well for this operator which has multiple types of expressions. We probably need to come up with something better long term.




----------------------------------------------------------------
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:
users@infra.apache.org