From 79861e3eb1a87fbae9177c91ac3fc57880134ae1 Mon Sep 17 00:00:00 2001 From: jatin Date: Sun, 3 Nov 2024 02:43:00 +0530 Subject: [PATCH] using as_ref --- .../datasource/physical_plan/statistics.rs | 2 +- .../src/physical_optimizer/sort_pushdown.rs | 21 +++++++++++-------- .../functions-aggregate/src/array_agg.rs | 2 +- .../functions-aggregate/src/first_last.rs | 6 +++--- .../functions-aggregate/src/nth_value.rs | 2 +- datafusion/physical-expr/src/aggregate.rs | 2 +- .../physical-expr/src/window/built_in.rs | 2 +- .../physical-plan/src/aggregates/mod.rs | 4 ++-- datafusion/physical-plan/src/sorts/sort.rs | 2 +- 9 files changed, 23 insertions(+), 20 deletions(-) diff --git a/datafusion/core/src/datasource/physical_plan/statistics.rs b/datafusion/core/src/datasource/physical_plan/statistics.rs index ef9d3763d41d..184dcd01ceba 100644 --- a/datafusion/core/src/datasource/physical_plan/statistics.rs +++ b/datafusion/core/src/datasource/physical_plan/statistics.rs @@ -269,7 +269,7 @@ impl MinMaxStatistics { Ok(Self { min_by_sort_order: min.map_err(|e| e.context("build min rows"))?, max_by_sort_order: max.map_err(|e| e.context("build max rows"))?, - sort_order: sort_order.clone(), + sort_order: LexOrdering::from_ref(sort_order), }) } diff --git a/datafusion/core/src/physical_optimizer/sort_pushdown.rs b/datafusion/core/src/physical_optimizer/sort_pushdown.rs index db8a3c126950..7d46e8c2df7d 100644 --- a/datafusion/core/src/physical_optimizer/sort_pushdown.rs +++ b/datafusion/core/src/physical_optimizer/sort_pushdown.rs @@ -242,7 +242,7 @@ fn pushdown_requirement_to_children( Some(JoinSide::Left) => try_pushdown_requirements_to_join( smj, parent_required, - &parent_required_expr, + parent_required_expr.as_ref(), JoinSide::Left, ), Some(JoinSide::Right) => { @@ -364,29 +364,32 @@ fn try_pushdown_requirements_to_join( let mut smj_required_orderings = smj.required_input_ordering(); let right_requirement = smj_required_orderings.swap_remove(1); let left_requirement = smj_required_orderings.swap_remove(0); - let left_ordering = smj.left().output_ordering().cloned().unwrap_or_default(); - let right_ordering = smj.right().output_ordering().cloned().unwrap_or_default(); + let left_ordering = &smj.left().output_ordering().cloned().unwrap_or_default(); + let right_ordering = &smj.right().output_ordering().cloned().unwrap_or_default(); + let (new_left_ordering, new_right_ordering) = match push_side { JoinSide::Left => { - let left_eq_properties = - left_eq_properties.clone().with_reorder(sort_expr.clone()); + let left_eq_properties = left_eq_properties + .clone() + .with_reorder(LexOrdering::from_ref(sort_expr)); if left_eq_properties .ordering_satisfy_requirement(&left_requirement.unwrap_or_default()) { // After re-ordering requirement is still satisfied - (sort_expr, &right_ordering) + (sort_expr, right_ordering) } else { return Ok(None); } } JoinSide::Right => { - let right_eq_properties = - right_eq_properties.clone().with_reorder(sort_expr.clone()); + let right_eq_properties = right_eq_properties + .clone() + .with_reorder(LexOrdering::from_ref(sort_expr)); if right_eq_properties .ordering_satisfy_requirement(&right_requirement.unwrap_or_default()) { // After re-ordering requirement is still satisfied - (&left_ordering, sort_expr) + (left_ordering, sort_expr) } else { return Ok(None); } diff --git a/datafusion/functions-aggregate/src/array_agg.rs b/datafusion/functions-aggregate/src/array_agg.rs index 252a07cb11d8..7c22c21e38c9 100644 --- a/datafusion/functions-aggregate/src/array_agg.rs +++ b/datafusion/functions-aggregate/src/array_agg.rs @@ -135,7 +135,7 @@ impl AggregateUDFImpl for ArrayAgg { OrderSensitiveArrayAggAccumulator::try_new( &data_type, &ordering_dtypes, - acc_args.ordering_req.clone(), + LexOrdering::from_ref(acc_args.ordering_req), acc_args.is_reversed, ) .map(|acc| Box::new(acc) as _) diff --git a/datafusion/functions-aggregate/src/first_last.rs b/datafusion/functions-aggregate/src/first_last.rs index 99bac5b93a8e..db4d035c6842 100644 --- a/datafusion/functions-aggregate/src/first_last.rs +++ b/datafusion/functions-aggregate/src/first_last.rs @@ -130,7 +130,7 @@ impl AggregateUDFImpl for FirstValue { FirstValueAccumulator::try_new( acc_args.return_type, &ordering_dtypes, - acc_args.ordering_req.clone(), + LexOrdering::from_ref(acc_args.ordering_req), acc_args.ignore_nulls, ) .map(|acc| Box::new(acc.with_requirement_satisfied(requirement_satisfied)) as _) @@ -455,7 +455,7 @@ impl AggregateUDFImpl for LastValue { LastValueAccumulator::try_new( acc_args.return_type, &ordering_dtypes, - acc_args.ordering_req.clone(), + LexOrdering::from_ref(acc_args.ordering_req), acc_args.ignore_nulls, ) .map(|acc| Box::new(acc.with_requirement_satisfied(requirement_satisfied)) as _) @@ -647,7 +647,7 @@ impl Accumulator for LastValueAccumulator { if compare_rows( &self.orderings, orderings, - &get_sort_options(&self.ordering_req), + &get_sort_options(self.ordering_req.as_ref()), )? .is_lt() { diff --git a/datafusion/functions-aggregate/src/nth_value.rs b/datafusion/functions-aggregate/src/nth_value.rs index f3e892fa73d8..5f3a8cf2f161 100644 --- a/datafusion/functions-aggregate/src/nth_value.rs +++ b/datafusion/functions-aggregate/src/nth_value.rs @@ -133,7 +133,7 @@ impl AggregateUDFImpl for NthValueAgg { n, &data_type, &ordering_dtypes, - acc_args.ordering_req.clone(), + LexOrdering::from_ref(acc_args.ordering_req), ) .map(|acc| Box::new(acc) as _) } diff --git a/datafusion/physical-expr/src/aggregate.rs b/datafusion/physical-expr/src/aggregate.rs index cbfe704d141d..5dc138933430 100644 --- a/datafusion/physical-expr/src/aggregate.rs +++ b/datafusion/physical-expr/src/aggregate.rs @@ -298,7 +298,7 @@ impl AggregateFunctionExpr { } if !self.order_sensitivity().is_insensitive() { - return Some(&self.ordering_req); + return Some(self.ordering_req.as_ref()); } None diff --git a/datafusion/physical-expr/src/window/built_in.rs b/datafusion/physical-expr/src/window/built_in.rs index f5625b622f78..7173835c74bc 100644 --- a/datafusion/physical-expr/src/window/built_in.rs +++ b/datafusion/physical-expr/src/window/built_in.rs @@ -267,7 +267,7 @@ impl WindowExpr for BuiltInWindowExpr { Arc::new(BuiltInWindowExpr::new( reverse_expr, &self.partition_by.clone(), - &reverse_order_bys(&self.order_by), + reverse_order_bys(self.order_by.as_ref()).as_ref(), Arc::new(self.window_frame.reverse()), )) as _ }) diff --git a/datafusion/physical-plan/src/aggregates/mod.rs b/datafusion/physical-plan/src/aggregates/mod.rs index ed23e44ecd9b..a71039b5733b 100644 --- a/datafusion/physical-plan/src/aggregates/mod.rs +++ b/datafusion/physical-plan/src/aggregates/mod.rs @@ -1019,7 +1019,7 @@ pub fn get_finer_aggregate_exprs_requirement( if let Some(finer_ordering) = finer_ordering(&requirement, aggr_expr, group_by, eq_properties, agg_mode) { - if eq_properties.ordering_satisfy(&finer_ordering) { + if eq_properties.ordering_satisfy(finer_ordering.as_ref()) { // Requirement is satisfied by existing ordering requirement = finer_ordering; continue; @@ -1033,7 +1033,7 @@ pub fn get_finer_aggregate_exprs_requirement( eq_properties, agg_mode, ) { - if eq_properties.ordering_satisfy(&finer_ordering) { + if eq_properties.ordering_satisfy(finer_ordering.as_ref()) { // Reverse requirement is satisfied by exiting ordering. // Hence reverse the aggregator requirement = finer_ordering; diff --git a/datafusion/physical-plan/src/sorts/sort.rs b/datafusion/physical-plan/src/sorts/sort.rs index 0efc6116d90e..ce7efce41577 100644 --- a/datafusion/physical-plan/src/sorts/sort.rs +++ b/datafusion/physical-plan/src/sorts/sort.rs @@ -677,7 +677,7 @@ pub(crate) fn lexsort_to_indices_multi_columns( /// /// Support sorting datasets that are larger than the memory allotted /// by the memory manager, by spilling to disk. -#[derive(Debug)] +#[derive(Debug, Clone)] pub struct SortExec { /// Input schema pub(crate) input: Arc,