Skip to content

Commit a6529b1

Browse files
alambccciudatu
authored andcommitted
Minor: Remove some clone in TypeCoercion (apache#10203)
* Remove some clone in TypeCoercion * Less clone * less copy
1 parent 3c6981e commit a6529b1

File tree

1 file changed

+37
-52
lines changed

1 file changed

+37
-52
lines changed

datafusion/optimizer/src/analyzer/type_coercion.rs

Lines changed: 37 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -171,26 +171,26 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
171171
))))
172172
}
173173
Expr::Not(expr) => Ok(Transformed::yes(not(get_casted_expr_for_bool_op(
174-
&expr,
174+
*expr,
175175
&self.schema,
176176
)?))),
177177
Expr::IsTrue(expr) => Ok(Transformed::yes(is_true(
178-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
178+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
179179
))),
180180
Expr::IsNotTrue(expr) => Ok(Transformed::yes(is_not_true(
181-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
181+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
182182
))),
183183
Expr::IsFalse(expr) => Ok(Transformed::yes(is_false(
184-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
184+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
185185
))),
186186
Expr::IsNotFalse(expr) => Ok(Transformed::yes(is_not_false(
187-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
187+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
188188
))),
189189
Expr::IsUnknown(expr) => Ok(Transformed::yes(is_unknown(
190-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
190+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
191191
))),
192192
Expr::IsNotUnknown(expr) => Ok(Transformed::yes(is_not_unknown(
193-
get_casted_expr_for_bool_op(&expr, &self.schema)?,
193+
get_casted_expr_for_bool_op(*expr, &self.schema)?,
194194
))),
195195
Expr::Like(Like {
196196
negated,
@@ -308,15 +308,12 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
308308
Expr::ScalarFunction(ScalarFunction { func_def, args }) => match func_def {
309309
ScalarFunctionDefinition::UDF(fun) => {
310310
let new_expr = coerce_arguments_for_signature(
311-
args.as_slice(),
311+
args,
312312
&self.schema,
313313
fun.signature(),
314314
)?;
315-
let new_expr = coerce_arguments_for_fun(
316-
new_expr.as_slice(),
317-
&self.schema,
318-
&fun,
319-
)?;
315+
let new_expr =
316+
coerce_arguments_for_fun(new_expr, &self.schema, &fun)?;
320317
Ok(Transformed::yes(Expr::ScalarFunction(
321318
ScalarFunction::new_udf(fun, new_expr),
322319
)))
@@ -336,7 +333,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
336333
AggregateFunctionDefinition::BuiltIn(fun) => {
337334
let new_expr = coerce_agg_exprs_for_signature(
338335
&fun,
339-
&args,
336+
args,
340337
&self.schema,
341338
&fun.signature(),
342339
)?;
@@ -353,7 +350,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
353350
}
354351
AggregateFunctionDefinition::UDF(fun) => {
355352
let new_expr = coerce_arguments_for_signature(
356-
args.as_slice(),
353+
args,
357354
&self.schema,
358355
fun.signature(),
359356
)?;
@@ -387,7 +384,7 @@ impl TreeNodeRewriter for TypeCoercionRewriter {
387384
expr::WindowFunctionDefinition::AggregateFunction(fun) => {
388385
coerce_agg_exprs_for_signature(
389386
fun,
390-
&args,
387+
args,
391388
&self.schema,
392389
&fun.signature(),
393390
)?
@@ -454,12 +451,12 @@ fn coerce_scalar(target_type: &DataType, value: &ScalarValue) -> Result<ScalarVa
454451
/// Downstream code uses this signal to treat these values as *unbounded*.
455452
fn coerce_scalar_range_aware(
456453
target_type: &DataType,
457-
value: &ScalarValue,
454+
value: ScalarValue,
458455
) -> Result<ScalarValue> {
459-
coerce_scalar(target_type, value).or_else(|err| {
456+
coerce_scalar(target_type, &value).or_else(|err| {
460457
// If type coercion fails, check if the largest type in family works:
461458
if let Some(largest_type) = get_widest_type_in_family(target_type) {
462-
coerce_scalar(largest_type, value).map_or_else(
459+
coerce_scalar(largest_type, &value).map_or_else(
463460
|_| exec_err!("Cannot cast {value:?} to {target_type:?}"),
464461
|_| ScalarValue::try_from(target_type),
465462
)
@@ -484,7 +481,7 @@ fn get_widest_type_in_family(given_type: &DataType) -> Option<&DataType> {
484481
/// Coerces the given (window frame) `bound` to `target_type`.
485482
fn coerce_frame_bound(
486483
target_type: &DataType,
487-
bound: &WindowFrameBound,
484+
bound: WindowFrameBound,
488485
) -> Result<WindowFrameBound> {
489486
match bound {
490487
WindowFrameBound::Preceding(v) => {
@@ -530,31 +527,30 @@ fn coerce_window_frame(
530527
}
531528
WindowFrameUnits::Rows | WindowFrameUnits::Groups => &DataType::UInt64,
532529
};
533-
window_frame.start_bound =
534-
coerce_frame_bound(target_type, &window_frame.start_bound)?;
535-
window_frame.end_bound = coerce_frame_bound(target_type, &window_frame.end_bound)?;
530+
window_frame.start_bound = coerce_frame_bound(target_type, window_frame.start_bound)?;
531+
window_frame.end_bound = coerce_frame_bound(target_type, window_frame.end_bound)?;
536532
Ok(window_frame)
537533
}
538534

539535
// Support the `IsTrue` `IsNotTrue` `IsFalse` `IsNotFalse` type coercion.
540536
// The above op will be rewrite to the binary op when creating the physical op.
541-
fn get_casted_expr_for_bool_op(expr: &Expr, schema: &DFSchemaRef) -> Result<Expr> {
537+
fn get_casted_expr_for_bool_op(expr: Expr, schema: &DFSchemaRef) -> Result<Expr> {
542538
let left_type = expr.get_type(schema)?;
543539
get_input_types(&left_type, &Operator::IsDistinctFrom, &DataType::Boolean)?;
544-
cast_expr(expr, &DataType::Boolean, schema)
540+
expr.cast_to(&DataType::Boolean, schema)
545541
}
546542

547543
/// Returns `expressions` coerced to types compatible with
548544
/// `signature`, if possible.
549545
///
550546
/// See the module level documentation for more detail on coercion.
551547
fn coerce_arguments_for_signature(
552-
expressions: &[Expr],
548+
expressions: Vec<Expr>,
553549
schema: &DFSchema,
554550
signature: &Signature,
555551
) -> Result<Vec<Expr>> {
556552
if expressions.is_empty() {
557-
return Ok(vec![]);
553+
return Ok(expressions);
558554
}
559555

560556
let current_types = expressions
@@ -565,58 +561,47 @@ fn coerce_arguments_for_signature(
565561
let new_types = data_types(&current_types, signature)?;
566562

567563
expressions
568-
.iter()
564+
.into_iter()
569565
.enumerate()
570-
.map(|(i, expr)| cast_expr(expr, &new_types[i], schema))
571-
.collect::<Result<Vec<_>>>()
566+
.map(|(i, expr)| expr.cast_to(&new_types[i], schema))
567+
.collect()
572568
}
573569

574570
fn coerce_arguments_for_fun(
575-
expressions: &[Expr],
571+
expressions: Vec<Expr>,
576572
schema: &DFSchema,
577573
fun: &Arc<ScalarUDF>,
578574
) -> Result<Vec<Expr>> {
579-
if expressions.is_empty() {
580-
return Ok(vec![]);
581-
}
582-
let mut expressions: Vec<Expr> = expressions.to_vec();
583-
584575
// Cast Fixedsizelist to List for array functions
585576
if fun.name() == "make_array" {
586-
expressions = expressions
577+
expressions
587578
.into_iter()
588579
.map(|expr| {
589580
let data_type = expr.get_type(schema).unwrap();
590581
if let DataType::FixedSizeList(field, _) = data_type {
591-
let field = field.as_ref().clone();
592-
let to_type = DataType::List(Arc::new(field));
582+
let to_type = DataType::List(field.clone());
593583
expr.cast_to(&to_type, schema)
594584
} else {
595585
Ok(expr)
596586
}
597587
})
598-
.collect::<Result<Vec<_>>>()?;
588+
.collect()
589+
} else {
590+
Ok(expressions)
599591
}
600-
601-
Ok(expressions)
602-
}
603-
604-
/// Cast `expr` to the specified type, if possible
605-
fn cast_expr(expr: &Expr, to_type: &DataType, schema: &DFSchema) -> Result<Expr> {
606-
expr.clone().cast_to(to_type, schema)
607592
}
608593

609594
/// Returns the coerced exprs for each `input_exprs`.
610595
/// Get the coerced data type from `aggregate_rule::coerce_types` and add `try_cast` if the
611596
/// data type of `input_exprs` need to be coerced.
612597
fn coerce_agg_exprs_for_signature(
613598
agg_fun: &AggregateFunction,
614-
input_exprs: &[Expr],
599+
input_exprs: Vec<Expr>,
615600
schema: &DFSchema,
616601
signature: &Signature,
617602
) -> Result<Vec<Expr>> {
618603
if input_exprs.is_empty() {
619-
return Ok(vec![]);
604+
return Ok(input_exprs);
620605
}
621606
let current_types = input_exprs
622607
.iter()
@@ -627,10 +612,10 @@ fn coerce_agg_exprs_for_signature(
627612
type_coercion::aggregates::coerce_types(agg_fun, &current_types, signature)?;
628613

629614
input_exprs
630-
.iter()
615+
.into_iter()
631616
.enumerate()
632-
.map(|(i, expr)| cast_expr(expr, &coerced_types[i], schema))
633-
.collect::<Result<Vec<_>>>()
617+
.map(|(i, expr)| expr.cast_to(&coerced_types[i], schema))
618+
.collect()
634619
}
635620

636621
fn coerce_case_expression(case: Case, schema: &DFSchemaRef) -> Result<Case> {

0 commit comments

Comments
 (0)