From 66fa0ada219540e4d913c6eb68a118a4a238081d Mon Sep 17 00:00:00 2001
From: Jonathan Chen <chenleejonathan@gmail.com>
Date: Wed, 6 Nov 2024 18:56:21 -0500
Subject: [PATCH 1/2] move OptimizeAggregateOrder

---
 datafusion/core/src/physical_optimizer/mod.rs |  1 -
 datafusion/physical-optimizer/src/lib.rs      |  1 +
 .../src}/update_aggr_exprs.rs                 | 19 +++++++++++++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)
 rename datafusion/{core/src/physical_optimizer => physical-optimizer/src}/update_aggr_exprs.rs (91%)

diff --git a/datafusion/core/src/physical_optimizer/mod.rs b/datafusion/core/src/physical_optimizer/mod.rs
index efdd3148d03f..fe799a23059f 100644
--- a/datafusion/core/src/physical_optimizer/mod.rs
+++ b/datafusion/core/src/physical_optimizer/mod.rs
@@ -32,7 +32,6 @@ pub mod replace_with_order_preserving_variants;
 pub mod sanity_checker;
 #[cfg(test)]
 pub mod test_utils;
-pub mod update_aggr_exprs;
 
 mod sort_pushdown;
 mod utils;
diff --git a/datafusion/physical-optimizer/src/lib.rs b/datafusion/physical-optimizer/src/lib.rs
index 439f1dc873d1..6ae55df92d68 100644
--- a/datafusion/physical-optimizer/src/lib.rs
+++ b/datafusion/physical-optimizer/src/lib.rs
@@ -24,5 +24,6 @@ pub mod limited_distinct_aggregation;
 mod optimizer;
 pub mod output_requirements;
 pub mod topk_aggregation;
+pub mod update_aggr_exprs;
 
 pub use optimizer::PhysicalOptimizerRule;
diff --git a/datafusion/core/src/physical_optimizer/update_aggr_exprs.rs b/datafusion/physical-optimizer/src/update_aggr_exprs.rs
similarity index 91%
rename from datafusion/core/src/physical_optimizer/update_aggr_exprs.rs
rename to datafusion/physical-optimizer/src/update_aggr_exprs.rs
index d563d0c56d36..e76f641f8259 100644
--- a/datafusion/core/src/physical_optimizer/update_aggr_exprs.rs
+++ b/datafusion/physical-optimizer/src/update_aggr_exprs.rs
@@ -27,14 +27,15 @@ use datafusion_physical_expr::aggregate::AggregateFunctionExpr;
 use datafusion_physical_expr::{
     reverse_order_bys, EquivalenceProperties, PhysicalSortRequirement,
 };
-use datafusion_physical_expr_common::sort_expr::{LexOrdering, LexRequirement};
-use datafusion_physical_optimizer::PhysicalOptimizerRule;
+use datafusion_physical_expr::{LexOrdering, LexRequirement};
 use datafusion_physical_plan::aggregates::concat_slices;
 use datafusion_physical_plan::windows::get_ordered_partition_by_indices;
 use datafusion_physical_plan::{
     aggregates::AggregateExec, ExecutionPlan, ExecutionPlanProperties,
 };
 
+use crate::PhysicalOptimizerRule;
+
 /// This optimizer rule checks ordering requirements of aggregate expressions.
 ///
 /// There are 3 kinds of aggregators in terms of ordering requirements:
@@ -60,6 +61,20 @@ impl OptimizeAggregateOrder {
 }
 
 impl PhysicalOptimizerRule for OptimizeAggregateOrder {
+    /// Applies the `OptimizeAggregateOrder` rule to the provided execution plan.
+    ///
+    /// This function traverses the execution plan tree, identifies `AggregateExec` nodes,
+    /// and optimizes their aggregate expressions based on existing input orderings.
+    /// If optimizations are applied, it returns a modified execution plan.
+    ///
+    /// # Arguments
+    ///
+    /// * `plan` - The root of the execution plan to optimize.
+    /// * `_config` - Configuration options (currently unused).
+    ///
+    /// # Returns
+    ///
+    /// A `Result` containing the potentially optimized execution plan or an error.
     fn optimize(
         &self,
         plan: Arc<dyn ExecutionPlan>,

From 108c8aa3bce6daef245af0fe7ab3b2eeeb638c96 Mon Sep 17 00:00:00 2001
From: Jonathan Chen <chenleejonathan@gmail.com>
Date: Wed, 6 Nov 2024 20:02:11 -0500
Subject: [PATCH 2/2] clippy fix

---
 datafusion/physical-optimizer/src/update_aggr_exprs.rs | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/datafusion/physical-optimizer/src/update_aggr_exprs.rs b/datafusion/physical-optimizer/src/update_aggr_exprs.rs
index e76f641f8259..6228ed10ec34 100644
--- a/datafusion/physical-optimizer/src/update_aggr_exprs.rs
+++ b/datafusion/physical-optimizer/src/update_aggr_exprs.rs
@@ -100,7 +100,12 @@ impl PhysicalOptimizerRule for OptimizeAggregateOrder {
                 let requirement = indices
                     .iter()
                     .map(|&idx| {
-                        PhysicalSortRequirement::new(groupby_exprs[idx].clone(), None)
+                        PhysicalSortRequirement::new(
+                            Arc::<dyn datafusion_physical_plan::PhysicalExpr>::clone(
+                                &groupby_exprs[idx],
+                            ),
+                            None,
+                        )
                     })
                     .collect::<Vec<_>>();