Skip to content

Commit 05a6c45

Browse files
committed
Skip unnecessary plan rebuild in adjust_input_keys_ordering for non-join plans (apache#21947)
Closes apache#21946 `adjust_input_keys_ordering` returns `Transformed::yes` unconditionally in the default else branch, even when `requirements.data` is empty and no changes were made. This triggers unnecessary `with_new_children` rebuilds on every node in the plan tree for non-join/non-aggregate queries. For plans with custom `ExecutionPlan` nodes whose `with_new_children` is expensive (e.g. nodes that re-evaluate cost functions on rebuild), this causes significant overhead. Add an early return with `Transformed::no` when `requirements.data.is_empty()` in the default else branch of `adjust_input_keys_ordering`. This skips the unnecessary plan tree rebuild for simple scan/filter/limit plans that have no join key reordering requirements. Yes, two unit tests added: - `adjust_input_keys_ordering_no_transform_for_scan` — verifies a bare parquet scan returns `Transformed::no` - `adjust_input_keys_ordering_no_transform_for_filter_scan` — verifies a filter→scan tree returns `Transformed::no` via `transform_down` No. This is a performance optimization that does not change query results or plan structure.
1 parent 7768d24 commit 05a6c45

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

datafusion/core/tests/physical_optimizer/enforce_distribution.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3676,3 +3676,33 @@ fn test_replace_order_preserving_variants_with_fetch() -> Result<()> {
36763676

36773677
Ok(())
36783678
}
3679+
3680+
/// Verifies that `adjust_input_keys_ordering` returns `Transformed::no`
3681+
/// for a simple scan plan with no key requirements, avoiding an
3682+
/// unnecessary plan rebuild.
3683+
#[test]
3684+
fn adjust_input_keys_ordering_no_transform_for_scan() -> Result<()> {
3685+
let plan: Arc<dyn ExecutionPlan> = parquet_exec();
3686+
let requirements = PlanWithKeyRequirements::new_default(plan);
3687+
let result = adjust_input_keys_ordering(requirements)?;
3688+
assert!(
3689+
!result.transformed,
3690+
"expected Transformed::no for a scan plan with empty requirements"
3691+
);
3692+
Ok(())
3693+
}
3694+
3695+
/// Verifies that `adjust_input_keys_ordering` applied via `transform_down`
3696+
/// over a filter -> scan tree returns `Transformed::no` when there are no
3697+
/// join/aggregate key requirements.
3698+
#[test]
3699+
fn adjust_input_keys_ordering_no_transform_for_filter_scan() -> Result<()> {
3700+
let plan: Arc<dyn ExecutionPlan> = filter_exec(parquet_exec());
3701+
let requirements = PlanWithKeyRequirements::new_default(plan);
3702+
let result = requirements.transform_down(adjust_input_keys_ordering)?;
3703+
assert!(
3704+
!result.transformed,
3705+
"expected Transformed::no for a filter->scan tree with no key requirements"
3706+
);
3707+
Ok(())
3708+
}

datafusion/physical-optimizer/src/enforce_distribution.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,9 @@ pub fn adjust_input_keys_ordering(
425425
|| plan.as_any().downcast_ref::<WindowAggExec>().is_some()
426426
{
427427
requirements.data.clear();
428+
} else if requirements.data.is_empty() {
429+
// No requirements to push down and no plan changes — skip rebuild.
430+
return Ok(Transformed::no(requirements));
428431
} else {
429432
// By default, push down the parent requirements to children
430433
for child in requirements.children.iter_mut() {

0 commit comments

Comments
 (0)