Cache `PlanProperties`, add fast-path for `with_new_children` #19792

askalt · 2026-01-13T14:24:27Z

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 all
major plans.
Store Arc<[usize]> instead of vector within FilterExec.
Store Arc<[usize]> instead of vector within projection of HashJoinExec.
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 · 2026-01-13T15:21:49Z

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 · 2026-01-13T20:05:07Z

I filed a ticket to track this idea

Avoid recomputing PlanProperties redundently #19796

alamb · 2026-01-13T20:05:15Z

run benchmark sql_planner

alamb-ghbot · 2026-01-13T20:05:20Z

🤖 ./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 · 2026-01-13T20:06:40Z

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 · 2026-01-13T20:41:36Z

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 · 2026-01-14T10:29:24Z

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 · 2026-01-14T11:22:13Z

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 · 2026-01-14T18:38:06Z

datafusion-examples/examples/custom_data_source/custom_datasource.rs

    db: CustomDataSource,
    projected_schema: SchemaRef,
-    cache: PlanProperties,
+    cache: Arc<PlanProperties>,


💯 for this change

alamb · 2026-01-14T18:42:37Z

datafusion/physical-plan/src/aggregates/mod.rs

+        mut children: Vec<Arc<dyn ExecutionPlan>>,
    ) -> Result<Arc<dyn ExecutionPlan>> {
+        if has_same_children_properties(&self, &children)? {
+            // Avoid properties re-computation.


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); }

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 🤔

It seems we already check it explicitly in DynTreeNode implementation:

datafusion/datafusion/physical-plan/src/tree_node.rs

Line 38 in eadbed5

with_new_children_if_necessary(arc_self, new_children)

datafusion/datafusion/physical-plan/src/execution_plan.rs

Line 1154 in eadbed5

pub fn with_new_children_if_necessary(

datafusion/physical-plan/src/execution_plan.rs

askalt · 2026-01-15T11:57:30Z

datafusion/physical-plan/src/projection.rs

        self: Arc<Self>,
        mut children: Vec<Arc<dyn ExecutionPlan>>,
    ) -> Result<Arc<dyn ExecutionPlan>> {
+        check_if_same_properties!(self, children);


Fast-path is currently implemented for most plans. It seems quite odd to me that we need to ensure it's implemented for every plan (if at least one plan doesn't implement this check and recalculates its properties, then recalculation will be performed for all parent plans). I've reduced the required code as much as possible: a plan implementer needs to call the added macro and implement the with_new_children_and_same_properties(...) method for the plan.

alamb · 2026-01-15T18:13:03Z

This PR seems to have accumulated some conflicts

alamb · 2026-01-15T18:14:10Z

I plan to run the newly introduced benchmark in #19806

I suspect we'll see quite a nice improvement

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 improvements. The patch includes the following 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 all major plans. - Store `Arc<[usize]>` instead of vector within `FilterExec`. - Store `Arc<[usize]>` instead of vector within projection of `HashJoinExec`. - 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. Closes apache#19796

alamb · 2026-01-15T18:38:30Z

run benchmark reset_plan_states

alamb-ghbot · 2026-01-15T18:38:35Z

🤖 ./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 (21b8a3f) to 094e7ee diff
BENCH_NAME=reset_plan_states
BENCH_COMMAND=cargo bench --features=parquet --bench reset_plan_states
BENCH_FILTER=
BENCH_BRANCH_NAME=askalt_with_new_children_fast_path
Results will be posted here when complete

alamb-ghbot · 2026-01-15T19:07:02Z

🤖: Benchmark completed

Details

group     askalt_with_new_children_fast_path     main
-----     ----------------------------------     ----
query1    1.00      2.6±0.01µs        ? ?/sec    14935.96    39.4±0.71ms        ? ?/sec
query2    1.00      3.3±0.06µs        ? ?/sec    3184.28    10.4±0.14ms        ? ?/sec
query3    1.00    980.0±9.64ns        ? ?/sec    15606.83    15.3±0.41ms        ? ?/sec

alamb · 2026-01-15T19:19:39Z

group     askalt_with_new_children_fast_path     main
-----     ----------------------------------     ----
query1    1.00      2.6±0.01µs        ? ?/sec    14935.96    39.4±0.71ms        ? ?/sec
query2    1.00      3.3±0.06µs        ? ?/sec    3184.28    10.4±0.14ms        ? ?/sec
query3    1.00    980.0±9.64ns        ? ?/sec    15606.83    15.3±0.41ms        ? ?/sec

That is pretty amazing (3000x-15000x performance improvement 👍 )

I am also testing sql_planner time using

TESTING PLANNING #19838

alamb

Thank you @askalt

I went through this carefully -- I think the basic idea is great and we should proceed.

The only thing I think we should try and do is minimize the impact of the API changes when people upgrade. I have some thoughts on that below

For next steps, I suggest break this PR down into smaller parts (it is quite challenging to review all the changes now). Some ideas on relevant chunks:

Change ExecutionPlan::properties to return an &Arc<Properties> and the necessary plumbing changes
Changes to FilterExec to avoid cloning project
Changes to AggregateExec to avoid cloning the vec / exprs
Changes to the various join operations to avoid cloning the projections

thoughts on reducing upgrade impact

Since this PR does change some core APIs, think we need to be careful.

I think we can avoid the need to change Schema::project, see askalt#2 for my proposal
We should figure out some way to make FilterExec::with_projection easier to use -- the new signature is pretty awkward.

Maybe we can use ProjectionExprs or something like

struct SharedProjection {
  Arc<[usize]> 
}

impl From<Vec<usize>> for SharedProjection {
  ...
}

And then have FilterExec take

pub fn fn with_projection(mut self, projection impl Into<SharedProjection>)

With enough comments I think this could be clear and would also be backwards compatible

alamb · 2026-01-15T19:27:00Z

datafusion/physical-plan/src/execution_plan.rs

    /// This information is available via methods on [`ExecutionPlanProperties`]
    /// trait, which is implemented for all `ExecutionPlan`s.
-    fn properties(&self) -> &PlanProperties;
+    fn properties(&self) -> &Arc<PlanProperties>;


This is technically a breaking API change, so it should be documented in the upgrading.md guide:
https://github.com/apache/datafusion/blob/main/docs/source/library-user-guide/upgrading.md

Per the policy in
https://datafusion.apache.org/contributor-guide/api-health.html

I am happy to help write such an entry

alamb · 2026-01-15T19:30:28Z

datafusion/common/src/utils/mod.rs

 pub fn project_schema(
    schema: &SchemaRef,
-    projection: Option<&Vec<usize>>,
+    projection: Option<&[usize]>,


this is also technically a public API change, but I think it just relaxes the constraints (you can still pass in a &Vec so all downstream code should continue to work

alamb · 2026-01-15T20:02:43Z

datafusion/ffi/src/execution_plan.rs

    name: String,
    plan: FFI_ExecutionPlan,
-    properties: PlanProperties,
+    properties: Arc<PlanProperties>,


since the FFI is supposed to be a stable boundary, I think we shouldn't change this (and instead just copy the properties when needed)

So I suggest reverting this change

alamb · 2026-01-15T20:07:49Z

datafusion/physical-plan/src/aggregates/mod.rs

-            self.aggr_expr.clone(),
-            self.filter_expr.clone(),
-            Arc::clone(&children[0]),
+            self.aggr_expr.to_vec(),


we might be able to avoid these clones too by changing the signature of AggregateExec 🤔

askalt changed the title ~~add fast-path for with_new_children~~ Draft: add fast-path for with_new_children Jan 13, 2026

askalt marked this pull request as draft January 13, 2026 14:26

askalt force-pushed the askalt/with_new_children_fast_path branch from 72ff575 to 796f731 Compare January 13, 2026 15:20

askalt force-pushed the askalt/with_new_children_fast_path branch from 796f731 to 5601c4f Compare January 13, 2026 15:24

askalt mentioned this pull request Jan 13, 2026

feat: support stateless physical plans #19462

Open

alamb mentioned this pull request Jan 13, 2026

Andrew Lamb Weekly-ish Open Source plan - 2026-01-05 #19652

Open

44 tasks

askalt force-pushed the askalt/with_new_children_fast_path branch from 5601c4f to 99cf634 Compare January 14, 2026 10:32

askalt mentioned this pull request Jan 14, 2026

type coercion error in clickbench50 query #19809

Open

askalt force-pushed the askalt/with_new_children_fast_path branch from 99cf634 to b81dd66 Compare January 14, 2026 14:49

alamb reviewed Jan 14, 2026

View reviewed changes

askalt force-pushed the askalt/with_new_children_fast_path branch from b81dd66 to 741b085 Compare January 15, 2026 11:48

askalt marked this pull request as ready for review January 15, 2026 11:49

askalt commented Jan 15, 2026

View reviewed changes

askalt changed the title ~~Draft: add fast-path for with_new_children~~ add fast-path for with_new_children Jan 15, 2026

askalt force-pushed the askalt/with_new_children_fast_path branch from 741b085 to 21b8a3f Compare January 15, 2026 18:22

alamb mentioned this pull request Jan 15, 2026

TESTING PLANNING #19838

Draft

alamb changed the title ~~add fast-path for with_new_children~~ Cache PlanProperties, add fast-path for with_new_children Jan 15, 2026

alamb added the api change Changes the API exposed to users of the crate label Jan 15, 2026

alamb mentioned this pull request Jan 15, 2026

Avoid changing project_schema signature askalt/datafusion#2

Open

alamb reviewed Jan 15, 2026

View reviewed changes

Cache PlanProperties, add fast-path for with_new_children #19792

Are you sure you want to change the base?

Cache PlanProperties, add fast-path for with_new_children #19792

Conversation

askalt commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

askalt commented Jan 13, 2026

Uh oh!

alamb commented Jan 13, 2026

Uh oh!

alamb commented Jan 13, 2026

Uh oh!

alamb-ghbot commented Jan 13, 2026

Uh oh!

alamb commented Jan 13, 2026

Uh oh!

alamb-ghbot commented Jan 13, 2026

Uh oh!

askalt commented Jan 14, 2026

Uh oh!

askalt commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

askalt Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

alamb-ghbot commented Jan 15, 2026

Uh oh!

alamb-ghbot commented Jan 15, 2026

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

thoughts on reducing upgrade impact

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Cache `PlanProperties`, add fast-path for `with_new_children` #19792

Cache `PlanProperties`, add fast-path for `with_new_children` #19792

askalt commented Jan 13, 2026 •

edited

Loading

askalt commented Jan 14, 2026 •

edited

Loading

askalt Jan 15, 2026 •

edited

Loading