alamb commented on code in PR #10203:
URL: https://github.com/apache/datafusion/pull/10203#discussion_r1576803845
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -565,58 +561,47 @@ fn coerce_arguments_for_signature(
let new_types = data_types(¤t_types, signature)?;
expressions
- .iter()
+ .into_iter()
.enumerate()
- .map(|(i, expr)| cast_expr(expr, &new_types[i], schema))
- .collect::<Result<Vec<_>>>()
+ .map(|(i, expr)| expr.cast_to(&new_types[i], schema))
+ .collect()
}
fn coerce_arguments_for_fun(
- expressions: &[Expr],
+ expressions: Vec<Expr>,
schema: &DFSchema,
fun: &Arc<ScalarUDF>,
) -> Result<Vec<Expr>> {
- if expressions.is_empty() {
- return Ok(vec![]);
- }
- let mut expressions: Vec<Expr> = expressions.to_vec();
-
// Cast Fixedsizelist to List for array functions
if fun.name() == "make_array" {
- expressions = expressions
+ expressions
.into_iter()
.map(|expr| {
let data_type = expr.get_type(schema).unwrap();
if let DataType::FixedSizeList(field, _) = data_type {
- let field = field.as_ref().clone();
- let to_type = DataType::List(Arc::new(field));
+ let to_type = DataType::List(field.clone());
Review Comment:
FieldRef is an `Arc<Field>` -- so this clone is just an Arc::clone (rather
than copying the entire filed)
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
@@ -565,58 +561,47 @@ fn coerce_arguments_for_signature(
let new_types = data_types(¤t_types, signature)?;
expressions
- .iter()
+ .into_iter()
.enumerate()
- .map(|(i, expr)| cast_expr(expr, &new_types[i], schema))
- .collect::<Result<Vec<_>>>()
+ .map(|(i, expr)| expr.cast_to(&new_types[i], schema))
+ .collect()
}
fn coerce_arguments_for_fun(
- expressions: &[Expr],
+ expressions: Vec<Expr>,
schema: &DFSchema,
fun: &Arc<ScalarUDF>,
) -> Result<Vec<Expr>> {
- if expressions.is_empty() {
- return Ok(vec![]);
- }
- let mut expressions: Vec<Expr> = expressions.to_vec();
-
// Cast Fixedsizelist to List for array functions
if fun.name() == "make_array" {
- expressions = expressions
+ expressions
.into_iter()
.map(|expr| {
let data_type = expr.get_type(schema).unwrap();
if let DataType::FixedSizeList(field, _) = data_type {
- let field = field.as_ref().clone();
- let to_type = DataType::List(Arc::new(field));
+ let to_type = DataType::List(field.clone());
expr.cast_to(&to_type, schema)
} else {
Ok(expr)
}
})
- .collect::<Result<Vec<_>>>()?;
+ .collect()
+ } else {
+ Ok(expressions)
}
-
- Ok(expressions)
-}
-
-/// Cast `expr` to the specified type, if possible
-fn cast_expr(expr: &Expr, to_type: &DataType, schema: &DFSchema) ->
Result<Expr> {
Review Comment:
This function just wrapped `Expr::cast_to` in an API that forced a clone, so
I removed 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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
