Minor: avoid copying order by exprs in planner (#11634)

apache · Jul 24, 2024 · bcf715c · bcf715c
1 parent 5901df5
commit bcf715c
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 12 deletions.
diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs
@@ -274,7 +274,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                 .map(|e| self.sql_expr_to_logical_expr(e, schema, planner_context))
                 .collect::<Result<Vec<_>>>()?;
             let mut order_by = self.order_by_to_sort_expr(
-                &window.order_by,
+                window.order_by,
                 schema,
                 planner_context,
                 // Numeric literals in window function ORDER BY are treated as constants
@@ -350,7 +350,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             // User defined aggregate functions (UDAF) have precedence in case it has the same name as a scalar built-in function
             if let Some(fm) = self.context_provider.get_aggregate_meta(&name) {
                 let order_by = self.order_by_to_sort_expr(
-                    &order_by,
+                    order_by,
                     schema,
                     planner_context,
                     true,
@@ -375,7 +375,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             // next, aggregate built-ins
             if let Ok(fun) = AggregateFunction::from_str(&name) {
                 let order_by = self.order_by_to_sort_expr(
-                    &order_by,
+                    order_by,
                     schema,
                     planner_context,
                     true,

diff --git a/datafusion/sql/src/expr/order_by.rs b/datafusion/sql/src/expr/order_by.rs
@@ -37,7 +37,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
     /// If false, interpret numeric literals as constant values.
     pub(crate) fn order_by_to_sort_expr(
         &self,
-        exprs: &[OrderByExpr],
+        exprs: Vec<OrderByExpr>,
         input_schema: &DFSchema,
         planner_context: &mut PlannerContext,
         literal_to_column: bool,
@@ -87,11 +87,9 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
                         input_schema.qualified_field(field_index - 1),
                     ))
                 }
-                e => self.sql_expr_to_logical_expr(
-                    e.clone(),
-                    order_by_schema,
-                    planner_context,
-                )?,
+                e => {
+                    self.sql_expr_to_logical_expr(e, order_by_schema, planner_context)?
+                }
             };
             let asc = asc.unwrap_or(true);
             expr_vec.push(Expr::Sort(Sort::new(

diff --git a/datafusion/sql/src/query.rs b/datafusion/sql/src/query.rs
@@ -59,7 +59,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
             other => {
                 let plan = self.set_expr_to_plan(other, planner_context)?;
                 let order_by_rex = self.order_by_to_sort_expr(
-                    &query.order_by,
+                    query.order_by,
                     plan.schema(),
                     planner_context,
                     true,

diff --git a/datafusion/sql/src/select.rs b/datafusion/sql/src/select.rs
@@ -101,7 +101,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         // Order-by expressions prioritize referencing columns from the select list,
         // then from the FROM clause.
         let order_by_rex = self.order_by_to_sort_expr(
-            &order_by,
+            order_by,
             projected_plan.schema().as_ref(),
             planner_context,
             true,

diff --git a/datafusion/sql/src/statement.rs b/datafusion/sql/src/statement.rs
@@ -967,7 +967,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
         for expr in order_exprs {
             // Convert each OrderByExpr to a SortExpr:
             let expr_vec =
-                self.order_by_to_sort_expr(&expr, schema, planner_context, true, None)?;
+                self.order_by_to_sort_expr(expr, schema, planner_context, true, None)?;
             // Verify that columns of all SortExprs exist in the schema:
             for expr in expr_vec.iter() {
                 for column in expr.column_refs().iter() {