Skip to content

Commit bcf715c

Browse files
authored
Minor: avoid copying order by exprs in planner (#11634)
1 parent 5901df5 commit bcf715c

File tree

5 files changed

+10
-12
lines changed

5 files changed

+10
-12
lines changed

datafusion/sql/src/expr/function.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
274274
.map(|e| self.sql_expr_to_logical_expr(e, schema, planner_context))
275275
.collect::<Result<Vec<_>>>()?;
276276
let mut order_by = self.order_by_to_sort_expr(
277-
&window.order_by,
277+
window.order_by,
278278
schema,
279279
planner_context,
280280
// Numeric literals in window function ORDER BY are treated as constants
@@ -350,7 +350,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
350350
// User defined aggregate functions (UDAF) have precedence in case it has the same name as a scalar built-in function
351351
if let Some(fm) = self.context_provider.get_aggregate_meta(&name) {
352352
let order_by = self.order_by_to_sort_expr(
353-
&order_by,
353+
order_by,
354354
schema,
355355
planner_context,
356356
true,
@@ -375,7 +375,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
375375
// next, aggregate built-ins
376376
if let Ok(fun) = AggregateFunction::from_str(&name) {
377377
let order_by = self.order_by_to_sort_expr(
378-
&order_by,
378+
order_by,
379379
schema,
380380
planner_context,
381381
true,

datafusion/sql/src/expr/order_by.rs

+4-6
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
3737
/// If false, interpret numeric literals as constant values.
3838
pub(crate) fn order_by_to_sort_expr(
3939
&self,
40-
exprs: &[OrderByExpr],
40+
exprs: Vec<OrderByExpr>,
4141
input_schema: &DFSchema,
4242
planner_context: &mut PlannerContext,
4343
literal_to_column: bool,
@@ -87,11 +87,9 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
8787
input_schema.qualified_field(field_index - 1),
8888
))
8989
}
90-
e => self.sql_expr_to_logical_expr(
91-
e.clone(),
92-
order_by_schema,
93-
planner_context,
94-
)?,
90+
e => {
91+
self.sql_expr_to_logical_expr(e, order_by_schema, planner_context)?
92+
}
9593
};
9694
let asc = asc.unwrap_or(true);
9795
expr_vec.push(Expr::Sort(Sort::new(

datafusion/sql/src/query.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
5959
other => {
6060
let plan = self.set_expr_to_plan(other, planner_context)?;
6161
let order_by_rex = self.order_by_to_sort_expr(
62-
&query.order_by,
62+
query.order_by,
6363
plan.schema(),
6464
planner_context,
6565
true,

datafusion/sql/src/select.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
101101
// Order-by expressions prioritize referencing columns from the select list,
102102
// then from the FROM clause.
103103
let order_by_rex = self.order_by_to_sort_expr(
104-
&order_by,
104+
order_by,
105105
projected_plan.schema().as_ref(),
106106
planner_context,
107107
true,

datafusion/sql/src/statement.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
967967
for expr in order_exprs {
968968
// Convert each OrderByExpr to a SortExpr:
969969
let expr_vec =
970-
self.order_by_to_sort_expr(&expr, schema, planner_context, true, None)?;
970+
self.order_by_to_sort_expr(expr, schema, planner_context, true, None)?;
971971
// Verify that columns of all SortExprs exist in the schema:
972972
for expr in expr_vec.iter() {
973973
for column in expr.column_refs().iter() {

0 commit comments

Comments
 (0)