[GitHub] [arrow] jorgecarleitao commented on a change in pull request #8032: ARROW-9836: [Rust][DataFusion] Improve API for usage of UDFs

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

jorgecarleitao commented on a change in pull request #8032:
URL: https://github.com/apache/arrow/pull/8032#discussion_r482988513



##########
File path: rust/datafusion/src/logical_plan/mod.rs
##########
@@ -1089,6 +1079,15 @@ pub fn can_coerce_from(type_into: &DataType, type_from: &DataType) -> bool {
     }
 }
 
+/// A registry knows how to build logical expressions out of user-defined function' names
+pub trait Registry {

Review comment:
       [Explicit is better than implicit](https://www.python.org/dev/peps/pep-0020/#id2) (done)

##########
File path: rust/datafusion/src/logical_plan/mod.rs
##########
@@ -1184,19 +1183,14 @@ impl LogicalPlanBuilder {
     /// Apply a projection
     pub fn project(&self, expr: Vec<Expr>) -> Result<Self> {
         let input_schema = self.plan.schema();
-        let projected_expr = if expr.contains(&Expr::Wildcard) {
-            let mut expr_vec = vec![];
-            (0..expr.len()).for_each(|i| match &expr[i] {
-                Expr::Wildcard => {
-                    (0..input_schema.fields().len())
-                        .for_each(|i| expr_vec.push(col(input_schema.field(i).name())));
-                }
-                _ => expr_vec.push(expr[i].clone()),
-            });
-            expr_vec
-        } else {
-            expr.clone()
-        };
+        let mut projected_expr = vec![];

Review comment:
       Indirectly, it is related: `expr.contains(&Expr::Wildcard)` requires `PartialEq`, which was dropped [in this line](https://github.com/apache/arrow/pull/8032/files/d2ff9c9b838e68d98d76b1f18c3721f2fa7e411f#diff-9922d86c805a8dc858387eb47612caaeL227) due to the addition of an `Arc<ScalarFunction>` in [this line](https://github.com/apache/arrow/pull/8032/files/d2ff9c9b838e68d98d76b1f18c3721f2fa7e411f#diff-9922d86c805a8dc858387eb47612caaeR279).
   
   However, since `if expr.contains(&Expr::Wildcard)` thankfully was entirely optional, I dropped it ^_^




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