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



##########
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:
       FYI, I started/designed for the Polars library something a bit inspired 
by the Spark catalyst optimizer ( 
http://people.csail.mit.edu/matei/papers/2015/sigmod_spark_sql.pdf ), but a bit 
evolved in some ways to be useful in Rust. Specifically, it uses an arena to 
mutate the tree efficiently / without cloning.
   
   Code lives here currently: 
https://github.com/ritchie46/polars/blob/master/polars/src/lazy/logical_plan/optimizer/simplify_expr.rs#L777
   




----------------------------------------------------------------
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:
[email protected]


Reply via email to