alamb commented on a change in pull request #8032:
URL: https://github.com/apache/arrow/pull/8032#discussion_r482899692
##########
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:
I wonder if renaming this to `FunctionRegistry` might make it a little
more specific, without being overly verbose. I don't feel strongly about this
change
```suggestion
pub trait FunctionRegistry {
```
##########
File path: rust/datafusion/src/optimizer/type_coercion.rs
##########
@@ -22,34 +22,24 @@
use arrow::datatypes::Schema;
-use crate::error::{ExecutionError, Result};
+use crate::error::Result;
use crate::logical_plan::Expr;
use crate::logical_plan::LogicalPlan;
use crate::optimizer::optimizer::OptimizerRule;
use crate::optimizer::utils;
-use crate::physical_plan::{
- expressions::numerical_coercion, udf::ScalarFunctionRegistry,
-};
+use crate::physical_plan::expressions::numerical_coercion;
use utils::optimize_explain;
/// Optimizer that applies coercion rules to expressions in the logical plan.
///
/// This optimizer does not alter the structure of the plan, it only changes
expressions on it.
-pub struct TypeCoercionRule<'a, P>
-where
- P: ScalarFunctionRegistry,
-{
- scalar_functions: &'a P,
-}
+pub struct TypeCoercionRule {}
Review comment:
❤️ (for removing the reference)
##########
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:
I want to verify my understanding -- this is a code cleanup that is not
directly required for UDFs, right?
----------------------------------------------------------------
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
