Draft: add fast-path for with_new_children

[askalt]

• closes Avoid recomputing PlanProperties redundently #19796

This patch aims to implement a fast-path for the ExecutionPlan::with_new_children function for some plans, moving closer to a physical plan re-use implementation and improving planning performance. If the passed children properties are the same as in self, we do not actually recompute self's properties (which could be costly if projection mapping is required). Instead, we just replace the children and re-use self's properties as-is.

To be able to compare two different properties -- ExecutionPlan::properties(...) signature is modified and now returns &Arc<PlanProperties>. If children properties are the same in with_new_children -- we clone our properties arc and then a parent plan will consider our properties as unchanged, doing the same.

Also, there are other improvenets, all changes:

• Return &Arc<PlanProperties> from ExecutionPlan::properties(...) instead of a reference.
• Implement with_new_children fast-path if there is no children properties changes for plans:
  • SortExec
  • RepartitionExec
  • ProjectionExec
  • FilterExec
  • CoalescePartitionsExec
  • AggregateExec
• Export reset_plan_states function.
• Store Arc<[usize]> instead of vector within FilterExec.
• Store Arc<[Arc<AggregateFunctionExpr>]> instead of vec for aggr expr and filters.
• Store Arc<[ProjectionExpr]> instead of vec in ProjectionExprs` struct.
• Get Option<&[usize]> instead of option vec ref in project_schema -- it makes API
  more flexible.

Note: currently, reset_plan_states does not allow to re-use plan in general: it is not
supported for dynamic filters and recursive queries features, as in this case state reset
should update pointers in the children plans.

[askalt]

Also added a typical analytical query plan re-usage benchmark. On the main branch it runs ~4-5 ms when at this MR it spends ~100us.

$ cargo bench  --profile=release-nonlto   --bench plan_reuse

[alamb]

I filed a ticket to track this idea

• Avoid recomputing PlanProperties redundently #19796

[alamb]

run benchmark sql_planner

[alamb-ghbot]

🤖 ./gh_compare_branch_bench.sh compare_branch_bench.sh Running
Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing askalt/with_new_children_fast_path (5601c4f) to 4c67d02 diff
BENCH_NAME=sql_planner
BENCH_COMMAND=cargo bench --features=parquet --bench sql_planner
BENCH_FILTER=
BENCH_BRANCH_NAME=askalt_with_new_children_fast_path
Results will be posted here when complete

[alamb]

Also added a typical analytical query plan re-usage benchmark. On the main branch it runs ~4-5 ms when at this MR it spends ~100us.

$ cargo bench  --profile=release-nonlto   --bench plan_reuse

Could you move the plan_reuse benchmark into its own PR (as I think it is valuable both for this PR and others, and it makes it easier to automatically compare performance)

[alamb-ghbot]

Benchmark script failed with exit code 101.

Last 10 lines of output:

Click to expand

                        time:   [5.6457 ms 5.6877 ms 5.7286 ms]

Benchmarking physical_plan_clickbench_q50
Benchmarking physical_plan_clickbench_q50: Warming up for 3.0000 s

thread 'main' (3793320) panicked at datafusion/core/benches/sql_planner.rs:62:14:
called `Result::unwrap()` on an `Err` value: Context("type_coercion", Internal("Expect TypeSignatureClass::Native(LogicalType(Native(String), String)) but received NativeType::Binary, DataType: BinaryView"))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

error: bench failed, to rerun pass `-p datafusion --bench sql_planner`

[askalt]

Also added a typical analytical query plan re-usage benchmark. On the main branch it runs ~4-5 ms when at this MR it spends ~100us.

$ cargo bench  --profile=release-nonlto   --bench plan_reuse

Could you move the plan_reuse benchmark into its own PR (as I think it is valuable both for this PR and others, and it makes it easier to automatically compare performance)

Done in #19806

[askalt]

Benchmark script failed with exit code 101.

Last 10 lines of output:

Click to expand

The same panic on 4c67d02

Created an issue for it: #19809

[alamb]

Thanks @askalt -- this is quite clever and I think it looks very promising

I also think we may be able to potentially make with_new_children even faster by checking the children as well -- and if they are the same there is no reason to recompute everything either.

However, this likely won't help your usecase as the children will likely change (their states need to be reset) 🤔

[alamb]

💯 for this change

[alamb]

I think we coulda avoid even more copying / cloning in the common case using Arc::make_mut https://doc.rust-lang.org/std/sync/struct.Arc.html#method.make_mut

Something like

        if has_same_children_properties(&self, &children)? {
            // Avoid properties re-computation.
            let me = Arc::make_mut(&mut self);
            me.input = children.swap_remove(0);
            me.metrics = ExecutionPlanMetricsSet::new();
            return Ok(self);
        }

[alamb]

Now that I write that, I wonder if we could use make_mut to avoid lots of cloning in general 🤔

We could check if the new children are the same as the existing children and if so return self 🤔

[alamb]

👍