fixup! fixup! fixup! WIP

apache · Aug 26, 2024 · f2a254f · f2a254f
1 parent e67b44b
commit f2a254f
Show file tree

Hide file tree

Showing 6 changed files with 15 additions and 15 deletions.
diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs
@@ -54,6 +54,7 @@ use datafusion_common::{
 // backwards compatibility
 use crate::display::PgJsonVisitor;
 use crate::logical_plan::tree_node::unwrap_arc;
+use crate::tree_node::replace_sort_expressions;
 pub use datafusion_common::display::{PlanType, StringifiedPlan, ToStringifiedPlan};
 pub use datafusion_common::{JoinConstraint, JoinType};
 
@@ -887,15 +888,15 @@ impl LogicalPlan {
                 Aggregate::try_new(Arc::new(inputs.swap_remove(0)), expr, agg_expr)
                     .map(LogicalPlan::Aggregate)
             }
-            LogicalPlan::Sort(Sort { /*fetch,*/ .. }) => {
-                // TODO FIXME
-                // LogicalPlan::Sort(Sort { fetch, .. }) => Ok(LogicalPlan::Sort(Sort {
-                //                 expr,
-                //                 input: Arc::new(inputs.swap_remove(0)),
-                //                 fetch: *fetch,
-                //             })),
-                internal_err!("with_ew_exprs not implemented for Sort")
-            },
+            LogicalPlan::Sort(Sort {
+                expr: sort_expr,
+                fetch,
+                ..
+            }) => Ok(LogicalPlan::Sort(Sort {
+                expr: replace_sort_expressions(sort_expr.clone(), expr),
+                input: Arc::new(inputs.swap_remove(0)),
+                fetch: *fetch,
+            })),
             LogicalPlan::Join(Join {
                 join_type,
                 join_constraint,

diff --git a/datafusion/expr/src/logical_plan/tree_node.rs b/datafusion/expr/src/logical_plan/tree_node.rs
@@ -491,7 +491,7 @@ impl LogicalPlan {
                     .visit_sibling(|| filter.iter().apply_until_stop(f))
             }
             LogicalPlan::Sort(Sort { expr, .. }) => {
-                expr.iter().apply_until_stop(|sort| f(&*sort.expr))
+                expr.iter().apply_until_stop(|sort| f(&sort.expr))
             }
             LogicalPlan::Extension(extension) => {
                 // would be nice to avoid this copy -- maybe can

diff --git a/datafusion/expr/src/tree_node.rs b/datafusion/expr/src/tree_node.rs
@@ -97,7 +97,7 @@ impl TreeNode for Expr {
                     expr_vec.push(f.as_ref());
                 }
                 if let Some(order_by) = order_by {
-                    expr_vec.extend(order_by.into_iter().map(|sort| sort.expr.as_ref()));
+                    expr_vec.extend(order_by.iter().map(|sort| sort.expr.as_ref()));
                 }
                 expr_vec
             }
@@ -109,7 +109,7 @@ impl TreeNode for Expr {
             }) => {
                 let mut expr_vec = args.iter().collect::<Vec<_>>();
                 expr_vec.extend(partition_by);
-                expr_vec.extend(order_by.into_iter().map(|sort| sort.expr.as_ref()));
+                expr_vec.extend(order_by.iter().map(|sort| sort.expr.as_ref()));
                 expr_vec
             }
             Expr::InList(InList { expr, list, .. }) => {

diff --git a/datafusion/optimizer/src/common_subexpr_eliminate.rs b/datafusion/optimizer/src/common_subexpr_eliminate.rs
@@ -886,7 +886,6 @@ enum ExprMask {
     /// - [`Columns`](Expr::Column)
     /// - [`ScalarVariable`](Expr::ScalarVariable)
     /// - [`Alias`](Expr::Alias)
-    /// - [`Sort`](Expr::Sort)
     /// - [`Wildcard`](Expr::Wildcard)
     /// - [`AggregateFunction`](Expr::AggregateFunction)
     Normal,

diff --git a/datafusion/sql/src/unparser/expr.rs b/datafusion/sql/src/unparser/expr.rs
@@ -253,7 +253,7 @@ impl Unparser<'_> {
                 };
                 let order_by: Vec<ast::OrderByExpr> = order_by
                     .iter()
-                    .map(|expr| sort_to_sql(expr))
+                    .map(sort_to_sql)
                     .collect::<Result<Vec<_>>>()?;
 
                 let start_bound = self.convert_bound(&window_frame.start_bound)?;

diff --git a/datafusion/sql/src/unparser/rewrite.rs b/datafusion/sql/src/unparser/rewrite.rs
@@ -86,7 +86,7 @@ pub(super) fn normalize_union_schema(plan: &LogicalPlan) -> Result<LogicalPlan>
 }
 
 /// Rewrite sort expressions that have a UNION plan as their input to remove the table reference.
-fn rewrite_sort_expr_for_union(exprs: Vec<SortExpr>) -> Result<Vec<SortExpr>> {
+fn rewrite_sort_expr_for_union(_exprs: Vec<SortExpr>) -> Result<Vec<SortExpr>> {
     // TODO FIXME
     //  this impl does't compile
     /*