You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/04/27 04:31:53 UTC
(datafusion) 01/01: Skip map_expressions rebuild for Extension nodes with empty expressions (#21701)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-21701-65f337d6637aa069012e0e9286f859fb3a743253
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 7fa6e2118b5567ccadf75623f11a65aa5ecfa57e
Author: Qi Zhu <qi...@polygon.io>
AuthorDate: Mon Apr 27 12:31:29 2026 +0800
Skip map_expressions rebuild for Extension nodes with empty expressions (#21701)
## Which issue does this PR close?
- Closes #21700.
## Rationale for this change
When an Extension node has no expressions, `map_expressions` was still
cloning all inputs and calling `with_exprs_and_inputs` to reconstruct
the node — wasted work since there are no expressions to transform. This
is common for Extension nodes like view matching candidates that carry
multiple children but no expressions.
## What changes are included in this PR?
1. **Code change** (`datafusion/expr/src/logical_plan/tree_node.rs`):
Add early return when `node.expressions()` is empty, skipping the clone
+ rebuild path.
2. **Micro-benchmark** (`datafusion/expr/benches/map_expressions.rs`):
Criterion benchmark comparing `map_expressions` on Extension nodes with
and without expressions, varying the number of children (1, 3, 5, 10).
## Are these changes tested?
Yes — existing tests pass, and the new benchmark validates the
optimization.
Benchmark results:
| Children | no_expr (optimized) | with_expr (rebuild) | Speedup |
|----------|--------------------|--------------------|---------|
| 1 | 24 ns | 167 ns | 7x |
| 3 | 23 ns | 192 ns | 8x |
| 5 | 23 ns | 181 ns | 8x |
| 10 | 24 ns | 216 ns | 9x |
The `no_expr` path is constant time regardless of children count.
In a real optimizer pipeline (~15 rules × 5-child Extension), this saves
~2.4 us per optimization pass.
## Next step
A more general optimization would be to change
`UserDefinedLogicalNode::expressions()` to return references (`&[Expr]`)
instead of cloned `Vec<Expr>`, and only clone + rebuild when the
transform actually modifies an expression. This would avoid the clone +
`with_exprs_and_inputs` rebuild even for non-empty expression lists when
the transform is a no-op. Added a TODO comment in the code for this
direction. This would be a larger API change, so the empty-expressions
shortcut is a pragmatic first step.
## Are there any user-facing changes?
No — purely internal optimization. No API changes.
---
Cargo.lock | 99 ++++++++++++---------------
datafusion/expr/src/logical_plan/tree_node.rs | 34 ++++++---
2 files changed, 68 insertions(+), 65 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index 5a76c063bb..be5dc21c3d 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -510,9 +510,9 @@ dependencies = [
[[package]]
name = "async-compression"
-version = "0.4.41"
+version = "0.4.42"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "d0f9ee0f6e02ffd7ad5816e9464499fba7b3effd01123b515c41d1697c43dad1"
+checksum = "e79b3f8a79cccc2898f31920fc69f304859b3bd567490f75ebf51ae1c792a9ac"
dependencies = [
"compression-codecs",
"compression-core",
@@ -1057,9 +1057,9 @@ dependencies = [
[[package]]
name = "blake3"
-version = "1.8.4"
+version = "1.8.5"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4d2d5991425dfd0785aed03aedcf0b321d61975c9b5b3689c774a2610ae0b51e"
+checksum = "0aa83c34e62843d924f905e0f5c866eb1dd6545fc4d719e803d9ba6030371fce"
dependencies = [
"arrayref",
"arrayvec",
@@ -1237,9 +1237,9 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5"
[[package]]
name = "cc"
-version = "1.2.60"
+version = "1.2.61"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "43c5703da9466b66a946814e1adf53ea2c90f10063b86290cc9eb67ce3478a20"
+checksum = "d16d90359e986641506914ba71350897565610e87ce0ad9e6f28569db3dd5c6d"
dependencies = [
"find-msvc-tools",
"jobserver",
@@ -1403,9 +1403,9 @@ dependencies = [
[[package]]
name = "compression-codecs"
-version = "0.4.37"
+version = "0.4.38"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "eb7b51a7d9c967fc26773061ba86150f19c50c0d65c887cb1fbe295fd16619b7"
+checksum = "ce2548391e9c1929c21bf6aa2680af86fe4c1b33e6cea9ac1cfeec0bd11218cf"
dependencies = [
"bzip2",
"compression-core",
@@ -1418,9 +1418,9 @@ dependencies = [
[[package]]
name = "compression-core"
-version = "0.4.31"
+version = "0.4.32"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "75984efb6ed102a0d42db99afb6c1948f0380d1d91808d5529916e6c08b49d8d"
+checksum = "cc14f565cf027a105f7a44ccf9e5b424348421a1d8952a8fc9d499d313107789"
[[package]]
name = "console"
@@ -1545,9 +1545,9 @@ dependencies = [
[[package]]
name = "crc-catalog"
-version = "2.4.0"
+version = "2.5.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "19d374276b40fb8bbdee95aef7c7fa6b5316ec764510eb64b8dd0e2ed0d7e7f5"
+checksum = "217698eaf96b4a3f0bc4f3662aaa55bdf913cd54d7204591faa790070c6d0853"
[[package]]
name = "crc32fast"
@@ -1677,9 +1677,9 @@ dependencies = [
[[package]]
name = "ctor"
-version = "0.10.0"
+version = "0.10.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "95d0d11eb38e7642efca359c3cf6eb7b2e528182d09110165de70192b0352775"
+checksum = "83cf0d42651b16c6dfe68685716d18480d18a9c39c62d76e8cf3eb6ed5d8bcbf"
dependencies = [
"ctor-proc-macro",
"dtor",
@@ -1688,9 +1688,9 @@ dependencies = [
[[package]]
name = "ctor-proc-macro"
-version = "0.0.12"
+version = "0.0.13"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a7ab264ea985f1bd27887d7b21ea2bb046728e05d11909ca138d700c494730db"
+checksum = "7a949c44fcacbbbb7ada007dc7acb34603dd97cd47de5d054f2b6493ecebb483"
[[package]]
name = "ctutils"
@@ -2834,18 +2834,18 @@ dependencies = [
[[package]]
name = "dtor"
-version = "0.7.0"
+version = "0.8.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "17f72721db8027a4e96dd6fb50d2a1d32259c9d3da1b63dee612ccd981e14293"
+checksum = "edf234dd1594d6dd434a8fb8cada51ddbbc593e40e4a01556a0b31c62da2775b"
dependencies = [
"dtor-proc-macro",
]
[[package]]
name = "dtor-proc-macro"
-version = "0.0.12"
+version = "0.0.13"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "8c98b077c7463d01d22dde8a24378ddf1ca7263dc687cffbed38819ea6c21131"
+checksum = "2647271c92754afcb174e758003cfd1cbf1e43e5a7853d7b1813e63e19e39a73"
[[package]]
name = "dunce"
@@ -3479,9 +3479,9 @@ checksum = "135b12329e5e3ce057a9f972339ea52bc954fe1e9358ef27f95e89716fbc5424"
[[package]]
name = "hybrid-array"
-version = "0.4.10"
+version = "0.4.11"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3944cf8cf766b40e2a1a333ee5e9b563f854d5fa49d6a8ca2764e97c6eddb214"
+checksum = "08d46837a0ed51fe95bd3b05de33cd64a1ee88fc797477ca48446872504507c5"
dependencies = [
"typenum",
]
@@ -3846,9 +3846,9 @@ checksum = "8f42a60cbdf9a97f5d2305f08a87dc4e09308d1276d28c869c684d7777685682"
[[package]]
name = "jiff"
-version = "0.2.23"
+version = "0.2.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1a3546dc96b6d42c5f24902af9e2538e82e39ad350b0c766eb3fbf2d8f3d8359"
+checksum = "f00b5dbd620d61dfdcb6007c9c1f6054ebd75319f163d886a9055cec1155073d"
dependencies = [
"jiff-static",
"log",
@@ -3859,9 +3859,9 @@ dependencies = [
[[package]]
name = "jiff-static"
-version = "0.2.23"
+version = "0.2.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2a8c8b344124222efd714b73bb41f8b5120b27a7cc1c75593a6ff768d9d05aa4"
+checksum = "e000de030ff8022ea1da3f466fbb0f3a809f5e51ed31f6dd931c35181ad8e6d7"
dependencies = [
"proc-macro2",
"quote",
@@ -3967,9 +3967,9 @@ checksum = "b3a6a8c165077efc8f3a971534c50ea6a1a18b329ef4a66e897a7e3a1494565f"
[[package]]
name = "libc"
-version = "0.2.185"
+version = "0.2.186"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "52ff2c0fe9bc6cb6b14a0592c2ff4fa9ceb83eea9db979b0487cd054946a2b8f"
+checksum = "68ab91017fe16c622486840e4c83c9a37afeff978bd239b5293d61ece587de66"
[[package]]
name = "libloading"
@@ -4009,9 +4009,9 @@ checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981"
[[package]]
name = "libmimalloc-sys"
-version = "0.1.46"
+version = "0.1.47"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bc89deee4af0429081d2a518c0431ae068222a5a262a3bc6ff4d8535ec2e02fe"
+checksum = "2d1eacfa31c33ec25e873c136ba5669f00f9866d0688bea7be4d3f7e43067df6"
dependencies = [
"cc",
"cty",
@@ -4043,9 +4043,9 @@ dependencies = [
[[package]]
name = "link-section"
-version = "0.2.0"
+version = "0.2.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "468808413fa8bdf0edbe61c2bbc182dfc59885b94f496cf3fb42c9c96b1e0149"
+checksum = "b685d66585d646efe09fec763d796c291049c8b6bf84e04954bffc8748341f0d"
[[package]]
name = "linux-raw-sys"
@@ -4123,9 +4123,9 @@ checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79"
[[package]]
name = "mimalloc"
-version = "0.1.49"
+version = "0.1.50"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "aca3c01a711f395b4257b81674c0e90e8dd1f1e62c4b7db45f684cc7a4fcb18a"
+checksum = "b3627c4272df786b9260cabaa46aec1d59c93ede723d4c3ef646c503816b0640"
dependencies = [
"libmimalloc-sys",
]
@@ -4897,9 +4897,9 @@ dependencies = [
[[package]]
name = "psm"
-version = "0.1.30"
+version = "0.1.31"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3852766467df634d74f0b2d7819bf8dc483a0eb2e3b0f50f756f9cfe8b0d18d8"
+checksum = "645dbe486e346d9b5de3ef16ede18c26e6c70ad97418f4874b8b1889d6e761ea"
dependencies = [
"ar_archive_writer",
"cc",
@@ -5362,9 +5362,9 @@ dependencies = [
[[package]]
name = "rustls"
-version = "0.23.38"
+version = "0.23.39"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "69f9466fb2c14ea04357e91413efb882e2a6d4a406e625449bc0a5d360d53a21"
+checksum = "7c2c118cb077cca2822033836dfb1b975355dfb784b5e8da48f7b6c5db74e60e"
dependencies = [
"aws-lc-rs",
"log",
@@ -5390,9 +5390,9 @@ dependencies = [
[[package]]
name = "rustls-pki-types"
-version = "1.14.0"
+version = "1.14.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "be040f8b0a225e40375822a563fa9524378b9d63112f53e19ffff34df5d33fdd"
+checksum = "30a7197ae7eb376e574fe940d068c30fe0462554a3ddbe4eca7838e049c937a9"
dependencies = [
"web-time",
"zeroize",
@@ -5882,15 +5882,15 @@ checksum = "6ce2be8dc25455e1f91df71bfa12ad37d7af1092ae736f3a6cd0e37bc7810596"
[[package]]
name = "stacker"
-version = "0.1.23"
+version = "0.1.24"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "08d74a23609d509411d10e2176dc2a4346e3b4aea2e7b1869f19fdedbc71c013"
+checksum = "640c8cdd92b6b12f5bcb1803ca3bbf5ab96e5e6b6b96b9ab77dabe9e880b3190"
dependencies = [
"cc",
"cfg-if",
"libc",
"psm",
- "windows-sys 0.59.0",
+ "windows-sys 0.61.2",
]
[[package]]
@@ -7124,15 +7124,6 @@ dependencies = [
"windows-targets 0.52.6",
]
-[[package]]
-name = "windows-sys"
-version = "0.59.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1e38bc4d79ed67fd075bcc251a1c39b32a1776bbe92e5bef1f0bf1f8c531853b"
-dependencies = [
- "windows-targets 0.52.6",
-]
-
[[package]]
name = "windows-sys"
version = "0.60.2"
@@ -7291,9 +7282,9 @@ checksum = "d6bbff5f0aada427a1e5a6da5f1f98158182f26556f345ac9e04d36d0ebed650"
[[package]]
name = "winnow"
-version = "1.0.1"
+version = "1.0.2"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "09dac053f1cd375980747450bfc7250c264eaae0583872e845c0c7cd578872b5"
+checksum = "2ee1708bef14716a11bae175f579062d4554d95be2c6829f518df847b7b3fdd0"
dependencies = [
"memchr",
]
diff --git a/datafusion/expr/src/logical_plan/tree_node.rs b/datafusion/expr/src/logical_plan/tree_node.rs
index a1285510da..77991db1bf 100644
--- a/datafusion/expr/src/logical_plan/tree_node.rs
+++ b/datafusion/expr/src/logical_plan/tree_node.rs
@@ -584,17 +584,29 @@ impl LogicalPlan {
.map_elements(f)?
.update_data(|expr| LogicalPlan::Sort(Sort { expr, input, fetch })),
LogicalPlan::Extension(Extension { node }) => {
- // would be nice to avoid this copy -- maybe can
- // update extension to just observer Exprs
- let exprs = node.expressions().map_elements(f)?;
- let plan = LogicalPlan::Extension(Extension {
- node: UserDefinedLogicalNode::with_exprs_and_inputs(
- node.as_ref(),
- exprs.data,
- node.inputs().into_iter().cloned().collect::<Vec<_>>(),
- )?,
- });
- Transformed::new(plan, exprs.transformed, exprs.tnr)
+ let raw_exprs = node.expressions();
+ if raw_exprs.is_empty() {
+ // No expressions to transform — skip expensive clone of
+ // all inputs and reconstruction via with_exprs_and_inputs.
+ Transformed::no(LogicalPlan::Extension(Extension { node }))
+ } else {
+ // TODO: a more general optimization would be to change
+ // `UserDefinedLogicalNode::expressions()` to return
+ // references (`&[Expr]`) instead of cloned `Vec<Expr>`,
+ // and only clone + rebuild when the transform actually
+ // modifies an expression. This would avoid the clone +
+ // `with_exprs_and_inputs` rebuild even for non-empty
+ // expression lists when the transform is a no-op.
+ let exprs = raw_exprs.map_elements(f)?;
+ let plan = LogicalPlan::Extension(Extension {
+ node: UserDefinedLogicalNode::with_exprs_and_inputs(
+ node.as_ref(),
+ exprs.data,
+ node.inputs().into_iter().cloned().collect::<Vec<_>>(),
+ )?,
+ });
+ Transformed::new(plan, exprs.transformed, exprs.tnr)
+ }
}
LogicalPlan::TableScan(TableScan {
table_name,
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org