refactor: only sync the plan children when required

influxdata · Feb 26, 2025 · 238cb46 · 238cb46
1 parent 78ed122
commit 238cb46
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 2 deletions.
diff --git a/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs b/datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs
@@ -121,6 +121,7 @@ fn pushdown_sorts_helper(
             // remove current sort (which will be the new ordering to pushdown)
             let new_reqs = current_plan_reqs;
             sort_push_down = sort_push_down.children.swap_remove(0);
+            sort_push_down = sort_push_down.update_plan_from_children()?; // changed plan
 
             // add back sort exec matching parent
             sort_push_down =
@@ -148,7 +149,7 @@ fn pushdown_sorts_helper(
 
             // remove current sort, and get the sort's child
             sort_push_down = sort_push_down.children.swap_remove(0);
-            sort_push_down = sort_push_down.update_plan_from_children()?;
+            sort_push_down = sort_push_down.update_plan_from_children()?; // changed plan
 
             // set the stricter fetch
             sort_push_down.data.fetch = min_fetch(current_sort_fetch, parent_req_fetch);
@@ -198,7 +199,6 @@ fn pushdown_sorts_helper(
         assign_initial_requirements(&mut sort_push_down);
     }
 
-    sort_push_down = sort_push_down.update_plan_from_children()?;
     Ok(Transformed::yes(sort_push_down))
 }
 

diff --git a/datafusion/physical-optimizer/src/utils.rs b/datafusion/physical-optimizer/src/utils.rs
@@ -31,6 +31,10 @@ use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
 
 /// This utility function adds a `SortExec` above an operator according to the
 /// given ordering requirements while preserving the original partitioning.
+///
+/// Note that this updates the plan in both the [`PlanContext.children`] and
+/// the [`PlanContext.plan`]'s children. Therefore its not required to sync
+/// the child plans with [`PlanContext::update_plan_from_children`].
 pub fn add_sort_above<T: Clone + Default>(
     node: PlanContext<T>,
     sort_requirements: LexRequirement,