Skip to content

Commit d648982

Browse files
authored
Skip unnecessary plan rebuild in adjust_input_keys_ordering for non-join plans (apache#21947)
## Which issue does this PR close? Closes apache#21946 ## Rationale for this change `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. ## What changes are included in this PR? 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. ## Are these changes tested? 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` ## Are there any user-facing changes? No. This is a performance optimization that does not change query results or plan structure.
1 parent c2824b5 commit d648982

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
@@ -3978,3 +3978,33 @@ fn maintains_order_preserves_spm_through_union_with_prefer_existing_sort() -> Re
39783978

39793979
Ok(())
39803980
}
3981+
3982+
/// Verifies that `adjust_input_keys_ordering` returns `Transformed::no`
3983+
/// for a simple scan plan with no key requirements, avoiding an
3984+
/// unnecessary plan rebuild.
3985+
#[test]
3986+
fn adjust_input_keys_ordering_no_transform_for_scan() -> Result<()> {
3987+
let plan: Arc<dyn ExecutionPlan> = parquet_exec();
3988+
let requirements = PlanWithKeyRequirements::new_default(plan);
3989+
let result = adjust_input_keys_ordering(requirements)?;
3990+
assert!(
3991+
!result.transformed,
3992+
"expected Transformed::no for a scan plan with empty requirements"
3993+
);
3994+
Ok(())
3995+
}
3996+
3997+
/// Verifies that `adjust_input_keys_ordering` applied via `transform_down`
3998+
/// over a filter -> scan tree returns `Transformed::no` when there are no
3999+
/// join/aggregate key requirements.
4000+
#[test]
4001+
fn adjust_input_keys_ordering_no_transform_for_filter_scan() -> Result<()> {
4002+
let plan: Arc<dyn ExecutionPlan> = filter_exec(parquet_exec());
4003+
let requirements = PlanWithKeyRequirements::new_default(plan);
4004+
let result = requirements.transform_down(adjust_input_keys_ordering)?;
4005+
assert!(
4006+
!result.transformed,
4007+
"expected Transformed::no for a filter->scan tree with no key requirements"
4008+
);
4009+
Ok(())
4010+
}

datafusion/physical-optimizer/src/enforce_distribution.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,9 @@ pub fn adjust_input_keys_ordering(
412412
|| plan.is::<WindowAggExec>()
413413
{
414414
requirements.data.clear();
415+
} else if requirements.data.is_empty() {
416+
// No requirements to push down and no plan changes — skip rebuild.
417+
return Ok(Transformed::no(requirements));
415418
} else {
416419
// By default, push down the parent requirements to children
417420
for child in requirements.children.iter_mut() {

0 commit comments

Comments
 (0)