From a857d7f069a7b0c1e597ec2fae3fcad7c8b4b505 Mon Sep 17 00:00:00 2001 From: Peter Toth Date: Wed, 22 May 2024 09:38:41 +0200 Subject: [PATCH] use stricter references in apply() and visit() --- datafusion-cli/Cargo.lock | 128 +++++++++--------- .../examples/custom_datasource.rs | 2 +- datafusion/common/src/tree_node.rs | 125 ++++------------- .../datasource/physical_plan/arrow_file.rs | 2 +- .../core/src/datasource/physical_plan/avro.rs | 2 +- .../core/src/datasource/physical_plan/csv.rs | 2 +- .../core/src/datasource/physical_plan/json.rs | 2 +- .../datasource/physical_plan/parquet/mod.rs | 2 +- datafusion/core/src/execution/context/mod.rs | 4 +- .../aggregate_statistics.rs | 2 +- .../enforce_distribution.rs | 4 +- .../src/physical_optimizer/enforce_sorting.rs | 2 +- .../limited_distinct_aggregation.rs | 4 +- .../physical_optimizer/output_requirements.rs | 6 +- .../physical_optimizer/projection_pushdown.rs | 8 +- .../src/physical_optimizer/sort_pushdown.rs | 2 +- .../physical_optimizer/topk_aggregation.rs | 2 +- datafusion/core/src/physical_planner.rs | 2 +- datafusion/core/src/test/mod.rs | 2 +- datafusion/core/src/test_util/mod.rs | 2 +- datafusion/core/src/test_util/parquet.rs | 2 +- .../core/tests/custom_sources_cases/mod.rs | 4 +- .../provider_filter_pushdown.rs | 2 +- .../tests/custom_sources_cases/statistics.rs | 2 +- .../core/tests/fuzz_cases/aggregate_fuzz.rs | 4 +- .../core/tests/parquet/filter_pushdown.rs | 2 +- .../tests/user_defined/user_defined_plan.rs | 4 +- datafusion/expr/src/logical_plan/display.rs | 14 +- datafusion/expr/src/logical_plan/plan.rs | 18 +-- datafusion/expr/src/logical_plan/tree_node.rs | 10 +- datafusion/expr/src/tree_node.rs | 2 +- .../optimizer/src/common_subexpr_eliminate.rs | 6 +- .../src/expressions/column.rs | 2 +- .../physical-expr-common/src/physical_expr.rs | 2 +- .../physical-expr-common/src/tree_node.rs | 9 +- datafusion/physical-expr-common/src/utils.rs | 7 +- datafusion/physical-expr/Cargo.toml | 1 + .../physical-expr/src/equivalence/class.rs | 2 +- .../physical-expr/src/expressions/binary.rs | 4 +- .../physical-expr/src/expressions/case.rs | 19 ++- .../physical-expr/src/expressions/cast.rs | 4 +- .../physical-expr/src/expressions/column.rs | 2 +- .../physical-expr/src/expressions/in_list.rs | 6 +- .../src/expressions/is_not_null.rs | 4 +- .../physical-expr/src/expressions/is_null.rs | 4 +- .../physical-expr/src/expressions/like.rs | 4 +- .../physical-expr/src/expressions/literal.rs | 2 +- .../physical-expr/src/expressions/negative.rs | 4 +- .../physical-expr/src/expressions/no_op.rs | 2 +- .../physical-expr/src/expressions/not.rs | 4 +- .../physical-expr/src/expressions/try_cast.rs | 4 +- .../physical-expr/src/scalar_function.rs | 4 +- .../physical-plan/src/aggregates/mod.rs | 6 +- datafusion/physical-plan/src/analyze.rs | 4 +- .../physical-plan/src/coalesce_batches.rs | 4 +- .../physical-plan/src/coalesce_partitions.rs | 4 +- datafusion/physical-plan/src/display.rs | 2 +- datafusion/physical-plan/src/empty.rs | 2 +- datafusion/physical-plan/src/explain.rs | 2 +- datafusion/physical-plan/src/filter.rs | 4 +- datafusion/physical-plan/src/insert.rs | 4 +- .../physical-plan/src/joins/cross_join.rs | 4 +- .../physical-plan/src/joins/hash_join.rs | 4 +- .../src/joins/nested_loop_join.rs | 4 +- .../src/joins/sort_merge_join.rs | 4 +- .../src/joins/symmetric_hash_join.rs | 4 +- datafusion/physical-plan/src/lib.rs | 6 +- datafusion/physical-plan/src/limit.rs | 8 +- datafusion/physical-plan/src/memory.rs | 2 +- .../physical-plan/src/placeholder_row.rs | 2 +- datafusion/physical-plan/src/projection.rs | 4 +- .../physical-plan/src/recursive_query.rs | 8 +- .../physical-plan/src/repartition/mod.rs | 4 +- .../physical-plan/src/sorts/partial_sort.rs | 4 +- datafusion/physical-plan/src/sorts/sort.rs | 4 +- .../src/sorts/sort_preserving_merge.rs | 4 +- datafusion/physical-plan/src/streaming.rs | 2 +- datafusion/physical-plan/src/test/exec.rs | 12 +- datafusion/physical-plan/src/tree_node.rs | 9 +- datafusion/physical-plan/src/union.rs | 8 +- datafusion/physical-plan/src/unnest.rs | 4 +- datafusion/physical-plan/src/values.rs | 2 +- .../src/windows/bounded_window_agg_exec.rs | 4 +- .../src/windows/window_agg_exec.rs | 4 +- datafusion/physical-plan/src/work_table.rs | 2 +- datafusion/proto/src/physical_plan/mod.rs | 1 + 86 files changed, 281 insertions(+), 330 deletions(-) diff --git a/datafusion-cli/Cargo.lock b/datafusion-cli/Cargo.lock index db87f85e346b..dfe19dff6ff5 100644 --- a/datafusion-cli/Cargo.lock +++ b/datafusion-cli/Cargo.lock @@ -363,9 +363,9 @@ dependencies = [ [[package]] name = "async-compression" -version = "0.4.9" +version = "0.4.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e9eabd7a98fe442131a17c316bd9349c43695e49e730c3c8e12cfb5f4da2693" +checksum = "9c90a406b4495d129f00461241616194cb8a032c8d1c53c657f0961d5f8e0498" dependencies = [ "bzip2", "flate2", @@ -387,7 +387,7 @@ checksum = "c6fa2087f2753a7da8cc1c0dbfcf89579dd57458e36769de5ac750b4671737ca" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -869,9 +869,9 @@ dependencies = [ [[package]] name = "cc" -version = "1.0.97" +version = "1.0.98" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "099a5357d84c4c61eb35fc8eafa9a79a902c2f76911e5747ced4e032edd8d9b4" +checksum = "41c270e7540d725e65ac7f1b212ac8ce349719624d7bcff99f8e2e488e8cf03f" dependencies = [ "jobserver", "libc", @@ -1042,9 +1042,9 @@ dependencies = [ [[package]] name = "crc32fast" -version = "1.4.0" +version = "1.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b3855a8a784b474f333699ef2bbca9db2c4a1f6d9088a90a2d25b1eb53111eaa" +checksum = "a97769d94ddab943e4510d138150169a2758b5ef3eb191a9ee688de3e23ef7b3" dependencies = [ "cfg-if", ] @@ -1093,7 +1093,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edb49164822f3ee45b17acd4a208cfc1251410cf0cad9a833234c9890774dd9f" dependencies = [ "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -1354,6 +1354,7 @@ dependencies = [ "hex", "indexmap 2.2.6", "itertools", + "lazy_static", "log", "paste", "petgraph", @@ -1491,9 +1492,9 @@ checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10" [[package]] name = "either" -version = "1.11.0" +version = "1.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a47c1c47d2f5964e29c61246e81db715514cd532db6b5116a25ea3c03d6780a2" +checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b" [[package]] name = "encoding_rs" @@ -1531,9 +1532,9 @@ checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" [[package]] name = "errno" -version = "0.3.8" +version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245" +checksum = "534c5cf6194dfab3db3242765c03bbe257cf92f22b38f6bc0c58d59108a820ba" dependencies = [ "libc", "windows-sys 0.52.0", @@ -1681,7 +1682,7 @@ checksum = "87750cf4b7a4c0625b1529e4c543c2182106e4dedc60a2a6455e00d212c489ac" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -1983,9 +1984,9 @@ dependencies = [ [[package]] name = "instant" -version = "0.1.12" +version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" +checksum = "e0242819d153cba4b4b05a5a8f2a7e9bbf97b6055b2a002b395c96b5ff3c0222" dependencies = [ "cfg-if", "js-sys", @@ -2110,9 +2111,9 @@ dependencies = [ [[package]] name = "libc" -version = "0.2.154" +version = "0.2.155" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae743338b92ff9146ce83992f766a31066a91a8c84a45e0e9f21e7cf6de6d346" +checksum = "97b3888a4aecf77e811145cadf6eef5901f4782c53886191b2f693f24761847c" [[package]] name = "libflate" @@ -2146,9 +2147,9 @@ checksum = "4ec2a862134d2a7d32d7983ddcdd1c4923530833c9f2ea1a44fc5fa473989058" [[package]] name = "libmimalloc-sys" -version = "0.1.37" +version = "0.1.38" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "81eb4061c0582dedea1cbc7aff2240300dd6982e0239d1c99e65c1dbf4a30ba7" +checksum = "0e7bb23d733dfcc8af652a78b7bf232f0e967710d044732185e561e47c0336b6" dependencies = [ "cc", "libc", @@ -2166,9 +2167,9 @@ dependencies = [ [[package]] name = "linux-raw-sys" -version = "0.4.13" +version = "0.4.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" [[package]] name = "lock_api" @@ -2224,9 +2225,9 @@ checksum = "6c8640c5d730cb13ebd907d8d04b52f55ac9a2eec55b440c8892f40d56c76c1d" [[package]] name = "mimalloc" -version = "0.1.41" +version = "0.1.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f41a2280ded0da56c8cf898babb86e8f10651a34adcfff190ae9a1159c6908d" +checksum = "e9186d86b79b52f4a77af65604b51225e8db1d6ee7e3f41aec1e40829c71a176" dependencies = [ "libmimalloc-sys", ] @@ -2239,9 +2240,9 @@ checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" [[package]] name = "miniz_oxide" -version = "0.7.2" +version = "0.7.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9d811f3e15f28568be3407c8e7fdb6514c1cda3cb30683f15b6a1a1dc4ea14a7" +checksum = "87dfd01fe195c66b572b37921ad8803d010623c0aca821bea2302239d155cdae" dependencies = [ "adler", ] @@ -2285,9 +2286,9 @@ checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" [[package]] name = "num" -version = "0.4.2" +version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3135b08af27d103b0a51f2ae0f8632117b7b185ccf931445affa8df530576a41" +checksum = "35bd024e8b2ff75562e5f34e7f4905839deb4b22955ef5e73d2fea1b9813cb23" dependencies = [ "num-bigint", "num-complex", @@ -2309,9 +2310,9 @@ dependencies = [ [[package]] name = "num-complex" -version = "0.4.5" +version = "0.4.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23c6602fda94a57c990fe0df199a035d83576b496aa29f4e634a8ac6004e68a6" +checksum = "73f88a1307638156682bada9d7604135552957b7818057dcef22705b4d509495" dependencies = [ "num-traits", ] @@ -2344,11 +2345,10 @@ dependencies = [ [[package]] name = "num-rational" -version = "0.4.1" +version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0638a1c9d0a3c0914158145bc76cff373a75a627e6ecbfb71cbe6f453a5a19b0" +checksum = "f83d14da390562dca69fc84082e73e548e1ad308d24accdedd2720017cb37824" dependencies = [ - "autocfg", "num-bigint", "num-integer", "num-traits", @@ -2528,9 +2528,9 @@ checksum = "e3148f5046208a5d56bcfc03053e3ca6334e51da8dfb19b6cdc8b306fae3283e" [[package]] name = "petgraph" -version = "0.6.4" +version = "0.6.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1d3afd2628e69da2be385eb6f2fd57c8ac7977ceeff6dc166ff1657b0e386a9" +checksum = "b4c5cc86750666a3ed20bdaf5ca2a0344f9c67674cae0515bec2da16fbaa47db" dependencies = [ "fixedbitset", "indexmap 2.2.6", @@ -2591,7 +2591,7 @@ checksum = "2f38a4412a78282e09a2cf38d195ea5420d15ba0602cb375210efbc877243965" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -2680,9 +2680,9 @@ dependencies = [ [[package]] name = "proc-macro2" -version = "1.0.82" +version = "1.0.83" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ad3d49ab951a01fbaafe34f2ec74122942fe18a3f9814c3268f1bb72042131b" +checksum = "0b33eb56c327dec362a9e55b3ad14f9d2f0904fb5a5b03b513ab5465399e9f43" dependencies = [ "unicode-ident", ] @@ -2997,9 +2997,9 @@ dependencies = [ [[package]] name = "rustls-pki-types" -version = "1.6.0" +version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "51f344d206c5e1b010eec27349b815a4805f70a778895959d70b74b9b529b30a" +checksum = "976295e77ce332211c0d24d92c0e83e50f5c5f046d11082cea19f3df13a3562d" [[package]] name = "rustls-webpki" @@ -3013,9 +3013,9 @@ dependencies = [ [[package]] name = "rustversion" -version = "1.0.16" +version = "1.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "092474d1a01ea8278f69e6a358998405fae5b8b963ddaeb2b0b04a128bf1dfb0" +checksum = "955d28af4278de8121b7ebeb796b6a45735dc01436d898801014aced2773a3d6" [[package]] name = "rustyline" @@ -3117,29 +3117,29 @@ checksum = "a3f0bf26fd526d2a95683cd0f87bf103b8539e2ca1ef48ce002d67aad59aa0b4" [[package]] name = "serde" -version = "1.0.200" +version = "1.0.202" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ddc6f9cc94d67c0e21aaf7eda3a010fd3af78ebf6e096aa6e2e13c79749cce4f" +checksum = "226b61a0d411b2ba5ff6d7f73a476ac4f8bb900373459cd00fab8512828ba395" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.200" +version = "1.0.202" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "856f046b9400cee3c8c94ed572ecdb752444c24528c035cd35882aad6f492bcb" +checksum = "6048858004bcff69094cd972ed40a32500f153bd3be9f716b2eed2e8217c4838" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] name = "serde_json" -version = "1.0.116" +version = "1.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e17db7126d17feb94eb3fad46bf1a96b034e8aacbc2e775fe81505f8b0b2813" +checksum = "455182ea6142b14f93f4bc5320a2b31c1f266b66a4a5c858b013302a5d8cbfc3" dependencies = [ "itoa", "ryu", @@ -3267,7 +3267,7 @@ checksum = "01b2e185515564f15375f593fb966b5718bc624ba77fe49fa4616ad619690554" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3313,7 +3313,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3326,7 +3326,7 @@ dependencies = [ "proc-macro2", "quote", "rustversion", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3348,9 +3348,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.61" +version = "2.0.65" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c993ed8ccba56ae856363b1845da7266a7cb78e1d146c8a32d54b45a8b831fc9" +checksum = "d2863d96a84c6439701d7a38f9de935ec562c8832cc55d1dde0f513b52fad106" dependencies = [ "proc-macro2", "quote", @@ -3419,22 +3419,22 @@ checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" [[package]] name = "thiserror" -version = "1.0.60" +version = "1.0.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "579e9083ca58dd9dcf91a9923bb9054071b9ebbd800b342194c9feb0ee89fc18" +checksum = "c546c80d6be4bc6a00c0f01730c08df82eaa7a7a61f11d656526506112cc1709" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.60" +version = "1.0.61" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2470041c06ec3ac1ab38d0356a6119054dedaea53e12fbefc0de730a1c08524" +checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3529,7 +3529,7 @@ checksum = "5b8a1e28f2deaa14e508979454cb3a223b10b938b45af148bc0986de36f1923b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3625,7 +3625,7 @@ checksum = "34704c8d6ebcbc939824180af020566b01a7c01f80641264eba0999f6c2b6be7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3670,7 +3670,7 @@ checksum = "f03ca4cb38206e2bef0700092660bb74d696f808514dae47fa1467cbfe26e96e" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] @@ -3824,7 +3824,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", "wasm-bindgen-shared", ] @@ -3858,7 +3858,7 @@ checksum = "e94f17b526d0a461a191c78ea52bbce64071ed5c04c9ffe424dcb38f74171bb7" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -4123,7 +4123,7 @@ checksum = "15e934569e47891f7d9411f1a451d947a60e000ab3bd24fbb970f000387d1b3b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.61", + "syn 2.0.65", ] [[package]] diff --git a/datafusion-examples/examples/custom_datasource.rs b/datafusion-examples/examples/custom_datasource.rs index c2ea6f2b52a1..cfb49b023159 100644 --- a/datafusion-examples/examples/custom_datasource.rs +++ b/datafusion-examples/examples/custom_datasource.rs @@ -237,7 +237,7 @@ impl ExecutionPlan for CustomExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/common/src/tree_node.rs b/datafusion/common/src/tree_node.rs index f0b9e39af28e..40d9088bf2a9 100644 --- a/datafusion/common/src/tree_node.rs +++ b/datafusion/common/src/tree_node.rs @@ -123,26 +123,13 @@ pub trait TreeNode: Sized { /// TreeNodeVisitor::f_up(ChildNode2) /// TreeNodeVisitor::f_up(ParentNode) /// ``` - fn visit>( - &self, - visitor: &mut V, - ) -> Result { - visitor - .f_down(self)? - .visit_children(|| self.apply_children(|c| c.visit(visitor)))? - .visit_parent(|| visitor.f_up(self)) - } - - /// Similar to [`TreeNode::visit()`], but the lifetimes of the [`TreeNode`] references - /// passed to [`TreeNodeRefVisitor::f_down()`] and [`TreeNodeRefVisitor::f_up()`] - /// methods match the lifetime of the original root [`TreeNode`] reference. - fn visit_ref<'n, V: TreeNodeRefVisitor<'n, Node = Self>>( + fn visit<'n, V: TreeNodeVisitor<'n, Node = Self>>( &'n self, visitor: &mut V, ) -> Result { visitor .f_down(self)? - .visit_children(|| self.apply_children_ref(|c| c.visit_ref(visitor)))? + .visit_children(|| self.apply_children(|c| c.visit(visitor)))? .visit_parent(|| visitor.f_up(self)) } @@ -203,39 +190,18 @@ pub trait TreeNode: Sized { /// # See Also /// * [`Self::transform_down`] for the equivalent transformation API. /// * [`Self::visit`] for both top-down and bottom up traversal. - fn apply Result>( - &self, - mut f: F, - ) -> Result { - fn apply_impl Result>( - node: &N, - f: &mut F, - ) -> Result { - f(node)?.visit_children(|| node.apply_children(|c| apply_impl(c, f))) - } - - apply_impl(self, &mut f) - } - - /// Similar to [`TreeNode::apply()`], but the lifetime of the [`TreeNode`] references - /// passed to the `f` closures match the lifetime of the original root [`TreeNode`] - /// reference. - fn apply_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply<'n, F: FnMut(&'n Self) -> Result>( &'n self, mut f: F, ) -> Result { - fn apply_ref_impl< - 'n, - N: TreeNode, - F: FnMut(&'n N) -> Result, - >( + fn apply_impl<'n, N: TreeNode, F: FnMut(&'n N) -> Result>( node: &'n N, f: &mut F, ) -> Result { - f(node)?.visit_children(|| node.apply_children_ref(|c| apply_ref_impl(c, f))) + f(node)?.visit_children(|| node.apply_children(|c| apply_impl(c, f))) } - apply_ref_impl(self, &mut f) + apply_impl(self, &mut f) } /// Recursively rewrite the node's children and then the node using `f` @@ -461,18 +427,7 @@ pub trait TreeNode: Sized { /// /// Description: Apply `f` to inspect node's children (but not the node /// itself). - fn apply_children Result>( - &self, - f: F, - ) -> Result { - // The default implementation is the stricter `apply_children_ref()` - self.apply_children_ref(f) - } - - /// Similar to [`TreeNode::apply_children()`], but the lifetime of the [`TreeNode`] - /// references passed to the `f` closures match the lifetime of the original root - /// [`TreeNode`] reference. - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result; @@ -511,27 +466,7 @@ pub trait TreeNode: Sized { /// /// # See Also: /// * [`TreeNode::rewrite`] to rewrite owned `TreeNode`s -pub trait TreeNodeVisitor: Sized { - /// The node type which is visitable. - type Node: TreeNode; - - /// Invoked while traversing down the tree, before any children are visited. - /// Default implementation continues the recursion. - fn f_down(&mut self, _node: &Self::Node) -> Result { - Ok(TreeNodeRecursion::Continue) - } - - /// Invoked while traversing up the tree after children are visited. Default - /// implementation continues the recursion. - fn f_up(&mut self, _node: &Self::Node) -> Result { - Ok(TreeNodeRecursion::Continue) - } -} - -/// Similar to [`TreeNodeVisitor`], but the lifetimes of the [`TreeNode`] references -/// passed to [`TreeNodeRefVisitor::f_down()`] and [`TreeNodeRefVisitor::f_up()`] methods -/// match the lifetime of the original root [`TreeNode`] reference. -pub trait TreeNodeRefVisitor<'n>: Sized { +pub trait TreeNodeVisitor<'n>: Sized { /// The node type which is visitable. type Node: TreeNode; @@ -920,11 +855,7 @@ impl TransformedResult for Result> { /// its related `Arc` will automatically implement [`TreeNode`]. pub trait DynTreeNode { /// Returns all children of the specified `TreeNode`. - fn arc_children(&self) -> Vec>; - - fn children(&self) -> Vec<&Arc> { - panic!("DynTreeNode::children is not implemented yet") - } + fn arc_children(&self) -> Vec<&Arc>; /// Constructs a new node with the specified children. fn with_new_arc_children( @@ -937,18 +868,11 @@ pub trait DynTreeNode { /// Blanket implementation for any `Arc` where `T` implements [`DynTreeNode`] /// (such as [`Arc`]). impl TreeNode for Arc { - fn apply_children Result>( - &self, - f: F, - ) -> Result { - self.arc_children().iter().apply_until_stop(f) - } - - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result { - self.children().into_iter().apply_until_stop(f) + self.arc_children().into_iter().apply_until_stop(f) } fn map_children Result>>( @@ -957,7 +881,10 @@ impl TreeNode for Arc { ) -> Result> { let children = self.arc_children(); if !children.is_empty() { - let new_children = children.into_iter().map_until_stop_and_collect(f)?; + let new_children = children + .into_iter() + .cloned() + .map_until_stop_and_collect(f)?; // Propagate up `new_children.transformed` and `new_children.tnr` // along with the node containing transformed children. if new_children.transformed { @@ -989,7 +916,7 @@ pub trait ConcreteTreeNode: Sized { } impl TreeNode for T { - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result { @@ -1018,8 +945,8 @@ mod tests { use std::fmt::Display; use crate::tree_node::{ - Transformed, TreeNode, TreeNodeIterator, TreeNodeRecursion, TreeNodeRefVisitor, - TreeNodeRewriter, TreeNodeVisitor, + Transformed, TreeNode, TreeNodeIterator, TreeNodeRecursion, TreeNodeRewriter, + TreeNodeVisitor, }; use crate::Result; @@ -1036,7 +963,7 @@ mod tests { } impl TreeNode for TestTreeNode { - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result { @@ -1536,15 +1463,15 @@ mod tests { } } - impl TreeNodeVisitor for TestVisitor { + impl<'n, T: Display> TreeNodeVisitor<'n> for TestVisitor { type Node = TestTreeNode; - fn f_down(&mut self, node: &Self::Node) -> Result { + fn f_down(&mut self, node: &'n Self::Node) -> Result { self.visits.push(format!("f_down({})", node.data)); (*self.f_down)(node) } - fn f_up(&mut self, node: &Self::Node) -> Result { + fn f_up(&mut self, node: &'n Self::Node) -> Result { self.visits.push(format!("f_up({})", node.data)); (*self.f_up)(node) } @@ -2001,7 +1928,7 @@ mod tests { // | // A #[test] - fn test_apply_ref() -> Result<()> { + fn test_apply_and_visit_references() -> Result<()> { let node_a = TestTreeNode::new(vec![], "a".to_string()); let node_b = TestTreeNode::new(vec![], "b".to_string()); let node_d = TestTreeNode::new(vec![node_a], "d".to_string()); @@ -2022,7 +1949,7 @@ mod tests { let node_a_ref = &node_d_ref.children[0]; let mut m: HashMap<&TestTreeNode, usize> = HashMap::new(); - tree.apply_ref(|e| { + tree.apply(|e| { *m.entry(e).or_insert(0) += 1; Ok(TreeNodeRecursion::Continue) })?; @@ -2041,7 +1968,7 @@ mod tests { m: HashMap<&'n TestTreeNode, (usize, usize)>, } - impl<'n> TreeNodeRefVisitor<'n> for TestVisitor<'n> { + impl<'n> TreeNodeVisitor<'n> for TestVisitor<'n> { type Node = TestTreeNode; fn f_down(&mut self, node: &'n Self::Node) -> Result { @@ -2058,7 +1985,7 @@ mod tests { } let mut visitor = TestVisitor { m: HashMap::new() }; - tree.visit_ref(&mut visitor)?; + tree.visit(&mut visitor)?; let expected = HashMap::from([ (node_f_ref, (1, 1)), diff --git a/datafusion/core/src/datasource/physical_plan/arrow_file.rs b/datafusion/core/src/datasource/physical_plan/arrow_file.rs index 1e8775731015..e536ae823232 100644 --- a/datafusion/core/src/datasource/physical_plan/arrow_file.rs +++ b/datafusion/core/src/datasource/physical_plan/arrow_file.rs @@ -134,7 +134,7 @@ impl ExecutionPlan for ArrowExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { Vec::new() } diff --git a/datafusion/core/src/datasource/physical_plan/avro.rs b/datafusion/core/src/datasource/physical_plan/avro.rs index 4e5140e82d3f..934846046a4e 100644 --- a/datafusion/core/src/datasource/physical_plan/avro.rs +++ b/datafusion/core/src/datasource/physical_plan/avro.rs @@ -111,7 +111,7 @@ impl ExecutionPlan for AvroExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { Vec::new() } diff --git a/datafusion/core/src/datasource/physical_plan/csv.rs b/datafusion/core/src/datasource/physical_plan/csv.rs index cc7c837e471e..f0aee82457c6 100644 --- a/datafusion/core/src/datasource/physical_plan/csv.rs +++ b/datafusion/core/src/datasource/physical_plan/csv.rs @@ -173,7 +173,7 @@ impl ExecutionPlan for CsvExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } diff --git a/datafusion/core/src/datasource/physical_plan/json.rs b/datafusion/core/src/datasource/physical_plan/json.rs index 0180caa85011..cfd8f00439a5 100644 --- a/datafusion/core/src/datasource/physical_plan/json.rs +++ b/datafusion/core/src/datasource/physical_plan/json.rs @@ -138,7 +138,7 @@ impl ExecutionPlan for NdJsonExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { Vec::new() } diff --git a/datafusion/core/src/datasource/physical_plan/parquet/mod.rs b/datafusion/core/src/datasource/physical_plan/parquet/mod.rs index dd953878df49..0c09e1bdf1d7 100644 --- a/datafusion/core/src/datasource/physical_plan/parquet/mod.rs +++ b/datafusion/core/src/datasource/physical_plan/parquet/mod.rs @@ -363,7 +363,7 @@ impl ExecutionPlan for ParquetExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } diff --git a/datafusion/core/src/execution/context/mod.rs b/datafusion/core/src/execution/context/mod.rs index 59238ffc559a..a1b913249750 100644 --- a/datafusion/core/src/execution/context/mod.rs +++ b/datafusion/core/src/execution/context/mod.rs @@ -2441,10 +2441,10 @@ impl<'a> BadPlanVisitor<'a> { } } -impl<'a> TreeNodeVisitor for BadPlanVisitor<'a> { +impl<'n, 'a> TreeNodeVisitor<'n> for BadPlanVisitor<'a> { type Node = LogicalPlan; - fn f_down(&mut self, node: &Self::Node) -> Result { + fn f_down(&mut self, node: &'n Self::Node) -> Result { match node { LogicalPlan::Ddl(ddl) if !self.options.allow_ddl => { plan_err!("DDL not supported: {}", ddl.name()) diff --git a/datafusion/core/src/physical_optimizer/aggregate_statistics.rs b/datafusion/core/src/physical_optimizer/aggregate_statistics.rs index 1a82dac4658c..05f05d95b8db 100644 --- a/datafusion/core/src/physical_optimizer/aggregate_statistics.rs +++ b/datafusion/core/src/physical_optimizer/aggregate_statistics.rs @@ -123,7 +123,7 @@ fn take_optimizable(node: &dyn ExecutionPlan) -> Option> return Some(child); } } - if let [ref childrens_child] = child.children().as_slice() { + if let [childrens_child] = child.children().as_slice() { child = Arc::clone(childrens_child); } else { break; diff --git a/datafusion/core/src/physical_optimizer/enforce_distribution.rs b/datafusion/core/src/physical_optimizer/enforce_distribution.rs index cd84e911d381..df74a0058858 100644 --- a/datafusion/core/src/physical_optimizer/enforce_distribution.rs +++ b/datafusion/core/src/physical_optimizer/enforce_distribution.rs @@ -1375,8 +1375,8 @@ pub(crate) mod tests { vec![false] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } // model that it requires the output ordering of its input diff --git a/datafusion/core/src/physical_optimizer/enforce_sorting.rs b/datafusion/core/src/physical_optimizer/enforce_sorting.rs index bc435626c6a9..24306647c686 100644 --- a/datafusion/core/src/physical_optimizer/enforce_sorting.rs +++ b/datafusion/core/src/physical_optimizer/enforce_sorting.rs @@ -567,7 +567,7 @@ fn remove_corresponding_sort_from_sub_plan( // Replace with variants that do not preserve order. if is_sort_preserving_merge(&node.plan) { node.children = node.children.swap_remove(0).children; - node.plan = node.plan.children().swap_remove(0); + node.plan = node.plan.children().swap_remove(0).clone(); } else if let Some(repartition) = node.plan.as_any().downcast_ref::() { diff --git a/datafusion/core/src/physical_optimizer/limited_distinct_aggregation.rs b/datafusion/core/src/physical_optimizer/limited_distinct_aggregation.rs index 950bb3c8eeb2..1274fbe50a5f 100644 --- a/datafusion/core/src/physical_optimizer/limited_distinct_aggregation.rs +++ b/datafusion/core/src/physical_optimizer/limited_distinct_aggregation.rs @@ -78,7 +78,7 @@ impl LimitedDistinctAggregation { let mut is_global_limit = false; if let Some(local_limit) = plan.as_any().downcast_ref::() { limit = local_limit.fetch(); - children = local_limit.children(); + children = local_limit.children().into_iter().cloned().collect(); } else if let Some(global_limit) = plan.as_any().downcast_ref::() { global_fetch = global_limit.fetch(); @@ -86,7 +86,7 @@ impl LimitedDistinctAggregation { global_skip = global_limit.skip(); // the aggregate must read at least fetch+skip number of rows limit = global_fetch.unwrap() + global_skip; - children = global_limit.children(); + children = global_limit.children().into_iter().cloned().collect(); is_global_limit = true } else { return None; diff --git a/datafusion/core/src/physical_optimizer/output_requirements.rs b/datafusion/core/src/physical_optimizer/output_requirements.rs index 5bf86e88d646..67b38dba90ca 100644 --- a/datafusion/core/src/physical_optimizer/output_requirements.rs +++ b/datafusion/core/src/physical_optimizer/output_requirements.rs @@ -157,8 +157,8 @@ impl ExecutionPlan for OutputRequirementExec { vec![true] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn required_input_ordering(&self) -> Vec>> { @@ -273,7 +273,7 @@ fn require_top_ordering_helper( // When an operator requires an ordering, any `SortExec` below can not // be responsible for (i.e. the originator of) the global ordering. let (new_child, is_changed) = - require_top_ordering_helper(children.swap_remove(0))?; + require_top_ordering_helper(children.swap_remove(0).clone())?; Ok((plan.with_new_children(vec![new_child])?, is_changed)) } else { // Stop searching, there is no global ordering desired for the query. diff --git a/datafusion/core/src/physical_optimizer/projection_pushdown.rs b/datafusion/core/src/physical_optimizer/projection_pushdown.rs index fe1290e40774..077668844907 100644 --- a/datafusion/core/src/physical_optimizer/projection_pushdown.rs +++ b/datafusion/core/src/physical_optimizer/projection_pushdown.rs @@ -378,7 +378,7 @@ fn try_swapping_with_coalesce_partitions( return Ok(None); } // CoalescePartitionsExec always has a single child, so zero indexing is safe. - make_with_child(projection, &projection.input().children()[0]) + make_with_child(projection, projection.input().children()[0]) .map(|e| Some(Arc::new(CoalescePartitionsExec::new(e)) as _)) } @@ -526,7 +526,7 @@ fn try_pushdown_through_union( let new_children = union .children() .into_iter() - .map(|child| make_with_child(projection, &child)) + .map(|child| make_with_child(projection, child)) .collect::>>()?; Ok(Some(Arc::new(UnionExec::new(new_children)))) @@ -813,8 +813,8 @@ fn try_swapping_with_sort_merge_join( projection_as_columns, far_right_left_col_ind, far_left_right_col_ind, - &sm_join.children()[0], - &sm_join.children()[1], + sm_join.children()[0], + sm_join.children()[1], )?; Ok(Some(Arc::new(SortMergeJoinExec::try_new( diff --git a/datafusion/core/src/physical_optimizer/sort_pushdown.rs b/datafusion/core/src/physical_optimizer/sort_pushdown.rs index c527819e7746..83531da3ca8f 100644 --- a/datafusion/core/src/physical_optimizer/sort_pushdown.rs +++ b/datafusion/core/src/physical_optimizer/sort_pushdown.rs @@ -123,7 +123,7 @@ fn pushdown_requirement_to_children( if is_window(plan) { let required_input_ordering = plan.required_input_ordering(); let request_child = required_input_ordering[0].as_deref().unwrap_or(&[]); - let child_plan = plan.children().swap_remove(0); + let child_plan = plan.children().swap_remove(0).clone(); match determine_children_requirement(parent_required, request_child, child_plan) { RequirementsCompatibility::Satisfy => { let req = (!request_child.is_empty()).then(|| request_child.to_vec()); diff --git a/datafusion/core/src/physical_optimizer/topk_aggregation.rs b/datafusion/core/src/physical_optimizer/topk_aggregation.rs index 7c0519eda3b3..b754ee75ef3e 100644 --- a/datafusion/core/src/physical_optimizer/topk_aggregation.rs +++ b/datafusion/core/src/physical_optimizer/topk_aggregation.rs @@ -88,7 +88,7 @@ impl TopKAggregation { let sort = plan.as_any().downcast_ref::()?; let children = sort.children(); - let child = children.iter().exactly_one().ok()?; + let child = children.into_iter().exactly_one().ok()?; let order = sort.properties().output_ordering()?; let order = order.iter().exactly_one().ok()?; let limit = sort.fetch()?; diff --git a/datafusion/core/src/physical_planner.rs b/datafusion/core/src/physical_planner.rs index 090b1d59d9a0..f38c34b98a46 100644 --- a/datafusion/core/src/physical_planner.rs +++ b/datafusion/core/src/physical_planner.rs @@ -2876,7 +2876,7 @@ mod tests { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/core/src/test/mod.rs b/datafusion/core/src/test/mod.rs index 1152c70d4391..133b8d87b534 100644 --- a/datafusion/core/src/test/mod.rs +++ b/datafusion/core/src/test/mod.rs @@ -426,7 +426,7 @@ impl ExecutionPlan for StatisticsExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/core/src/test_util/mod.rs b/datafusion/core/src/test_util/mod.rs index 75ef364d01fd..7aec66825de3 100644 --- a/datafusion/core/src/test_util/mod.rs +++ b/datafusion/core/src/test_util/mod.rs @@ -293,7 +293,7 @@ impl ExecutionPlan for UnboundedExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/core/src/test_util/parquet.rs b/datafusion/core/src/test_util/parquet.rs index 1d5668c7ec55..d151afc2c13f 100644 --- a/datafusion/core/src/test_util/parquet.rs +++ b/datafusion/core/src/test_util/parquet.rs @@ -195,7 +195,7 @@ impl TestParquetFile { /// /// Recursively searches for ParquetExec and returns the metrics /// on the first one it finds - pub fn parquet_metrics(plan: Arc) -> Option { + pub fn parquet_metrics(plan: &Arc) -> Option { if let Some(parquet) = plan.as_any().downcast_ref::() { return parquet.metrics(); } diff --git a/datafusion/core/tests/custom_sources_cases/mod.rs b/datafusion/core/tests/custom_sources_cases/mod.rs index 31a823f3eaf9..e8ead01d2ee4 100644 --- a/datafusion/core/tests/custom_sources_cases/mod.rs +++ b/datafusion/core/tests/custom_sources_cases/mod.rs @@ -148,7 +148,7 @@ impl ExecutionPlan for CustomExecutionPlan { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } @@ -303,6 +303,6 @@ fn contains_place_holder_exec(plan: Arc) -> bool { } else if plan.children().len() != 1 { false } else { - contains_place_holder_exec(Arc::clone(&plan.children()[0])) + contains_place_holder_exec(Arc::clone(plan.children()[0])) } } diff --git a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs index 4579fe806d6f..8c9cffcf08d1 100644 --- a/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs +++ b/datafusion/core/tests/custom_sources_cases/provider_filter_pushdown.rs @@ -101,7 +101,7 @@ impl ExecutionPlan for CustomPlan { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/core/tests/custom_sources_cases/statistics.rs b/datafusion/core/tests/custom_sources_cases/statistics.rs index 85ac47dc97fc..c7be89533f1d 100644 --- a/datafusion/core/tests/custom_sources_cases/statistics.rs +++ b/datafusion/core/tests/custom_sources_cases/statistics.rs @@ -153,7 +153,7 @@ impl ExecutionPlan for StatisticsValidation { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs b/datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs index 8df16e7944d2..1e4ef0ecb2c6 100644 --- a/datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs +++ b/datafusion/core/tests/fuzz_cases/aggregate_fuzz.rs @@ -312,10 +312,10 @@ async fn verify_ordered_aggregate(frame: &DataFrame, expected_sort: bool) { } let mut visitor = Visitor { expected_sort }; - impl TreeNodeVisitor for Visitor { + impl<'n> TreeNodeVisitor<'n> for Visitor { type Node = Arc; - fn f_down(&mut self, node: &Self::Node) -> Result { + fn f_down(&mut self, node: &'n Self::Node) -> Result { if let Some(exec) = node.as_any().downcast_ref::() { if self.expected_sort { assert!(matches!( diff --git a/datafusion/core/tests/parquet/filter_pushdown.rs b/datafusion/core/tests/parquet/filter_pushdown.rs index feb928a3a474..8c7624f07813 100644 --- a/datafusion/core/tests/parquet/filter_pushdown.rs +++ b/datafusion/core/tests/parquet/filter_pushdown.rs @@ -529,7 +529,7 @@ impl<'a> TestCase<'a> { // verify expected pushdown let metrics = - TestParquetFile::parquet_metrics(exec).expect("found parquet metrics"); + TestParquetFile::parquet_metrics(&exec).expect("found parquet metrics"); let pushdown_expected = if scan_options.pushdown_filters { self.pushdown_expected diff --git a/datafusion/core/tests/user_defined/user_defined_plan.rs b/datafusion/core/tests/user_defined/user_defined_plan.rs index 54dcffe35f62..07622e48afaf 100644 --- a/datafusion/core/tests/user_defined/user_defined_plan.rs +++ b/datafusion/core/tests/user_defined/user_defined_plan.rs @@ -472,8 +472,8 @@ impl ExecutionPlan for TopKExec { vec![Distribution::SinglePartition] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/expr/src/logical_plan/display.rs b/datafusion/expr/src/logical_plan/display.rs index 3a2ed9ffc2d8..44ced6dc5bad 100644 --- a/datafusion/expr/src/logical_plan/display.rs +++ b/datafusion/expr/src/logical_plan/display.rs @@ -58,12 +58,12 @@ impl<'a, 'b> IndentVisitor<'a, 'b> { } } -impl<'a, 'b> TreeNodeVisitor for IndentVisitor<'a, 'b> { +impl<'n, 'a, 'b> TreeNodeVisitor<'n> for IndentVisitor<'a, 'b> { type Node = LogicalPlan; fn f_down( &mut self, - plan: &LogicalPlan, + plan: &'n LogicalPlan, ) -> datafusion_common::Result { if self.indent > 0 { writeln!(self.f)?; @@ -84,7 +84,7 @@ impl<'a, 'b> TreeNodeVisitor for IndentVisitor<'a, 'b> { fn f_up( &mut self, - _plan: &LogicalPlan, + _plan: &'n LogicalPlan, ) -> datafusion_common::Result { self.indent -= 1; Ok(TreeNodeRecursion::Continue) @@ -180,12 +180,12 @@ impl<'a, 'b> GraphvizVisitor<'a, 'b> { } } -impl<'a, 'b> TreeNodeVisitor for GraphvizVisitor<'a, 'b> { +impl<'n, 'a, 'b> TreeNodeVisitor<'n> for GraphvizVisitor<'a, 'b> { type Node = LogicalPlan; fn f_down( &mut self, - plan: &LogicalPlan, + plan: &'n LogicalPlan, ) -> datafusion_common::Result { let id = self.graphviz_builder.next_id(); @@ -648,12 +648,12 @@ impl<'a, 'b> PgJsonVisitor<'a, 'b> { } } -impl<'a, 'b> TreeNodeVisitor for PgJsonVisitor<'a, 'b> { +impl<'n, 'a, 'b> TreeNodeVisitor<'n> for PgJsonVisitor<'a, 'b> { type Node = LogicalPlan; fn f_down( &mut self, - node: &LogicalPlan, + node: &'n LogicalPlan, ) -> datafusion_common::Result { let id = self.next_id; self.next_id += 1; diff --git a/datafusion/expr/src/logical_plan/plan.rs b/datafusion/expr/src/logical_plan/plan.rs index 42f3e1f163a7..243b9203854e 100644 --- a/datafusion/expr/src/logical_plan/plan.rs +++ b/datafusion/expr/src/logical_plan/plan.rs @@ -2984,10 +2984,10 @@ digraph { strings: Vec, } - impl TreeNodeVisitor for OkVisitor { + impl<'n> TreeNodeVisitor<'n> for OkVisitor { type Node = LogicalPlan; - fn f_down(&mut self, plan: &LogicalPlan) -> Result { + fn f_down(&mut self, plan: &'n LogicalPlan) -> Result { let s = match plan { LogicalPlan::Projection { .. } => "pre_visit Projection", LogicalPlan::Filter { .. } => "pre_visit Filter", @@ -3001,7 +3001,7 @@ digraph { Ok(TreeNodeRecursion::Continue) } - fn f_up(&mut self, plan: &LogicalPlan) -> Result { + fn f_up(&mut self, plan: &'n LogicalPlan) -> Result { let s = match plan { LogicalPlan::Projection { .. } => "post_visit Projection", LogicalPlan::Filter { .. } => "post_visit Filter", @@ -3067,10 +3067,10 @@ digraph { return_false_from_post_in: OptionalCounter, } - impl TreeNodeVisitor for StoppingVisitor { + impl<'n> TreeNodeVisitor<'n> for StoppingVisitor { type Node = LogicalPlan; - fn f_down(&mut self, plan: &LogicalPlan) -> Result { + fn f_down(&mut self, plan: &'n LogicalPlan) -> Result { if self.return_false_from_pre_in.dec() { return Ok(TreeNodeRecursion::Stop); } @@ -3079,7 +3079,7 @@ digraph { Ok(TreeNodeRecursion::Continue) } - fn f_up(&mut self, plan: &LogicalPlan) -> Result { + fn f_up(&mut self, plan: &'n LogicalPlan) -> Result { if self.return_false_from_post_in.dec() { return Ok(TreeNodeRecursion::Stop); } @@ -3136,10 +3136,10 @@ digraph { return_error_from_post_in: OptionalCounter, } - impl TreeNodeVisitor for ErrorVisitor { + impl<'n> TreeNodeVisitor<'n> for ErrorVisitor { type Node = LogicalPlan; - fn f_down(&mut self, plan: &LogicalPlan) -> Result { + fn f_down(&mut self, plan: &'n LogicalPlan) -> Result { if self.return_error_from_pre_in.dec() { return not_impl_err!("Error in pre_visit"); } @@ -3147,7 +3147,7 @@ digraph { self.inner.f_down(plan) } - fn f_up(&mut self, plan: &LogicalPlan) -> Result { + fn f_up(&mut self, plan: &'n LogicalPlan) -> Result { if self.return_error_from_post_in.dec() { return not_impl_err!("Error in post_visit"); } diff --git a/datafusion/expr/src/logical_plan/tree_node.rs b/datafusion/expr/src/logical_plan/tree_node.rs index ed582b6c5344..3dc7aad33c6b 100644 --- a/datafusion/expr/src/logical_plan/tree_node.rs +++ b/datafusion/expr/src/logical_plan/tree_node.rs @@ -56,7 +56,7 @@ use datafusion_common::{ }; impl TreeNode for LogicalPlan { - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result { @@ -736,10 +736,10 @@ impl LogicalPlan { /// Visits a plan similarly to [`Self::visit`], including subqueries that /// may appear in expressions such as `IN (SELECT ...)`. - pub fn visit_with_subqueries>( - &self, - visitor: &mut V, - ) -> Result { + pub fn visit_with_subqueries(&self, visitor: &mut V) -> Result + where + for<'n> V: TreeNodeVisitor<'n, Node = Self>, + { visitor .f_down(self)? .visit_children(|| { diff --git a/datafusion/expr/src/tree_node.rs b/datafusion/expr/src/tree_node.rs index 7cdcdf935405..b3ef45fa23fa 100644 --- a/datafusion/expr/src/tree_node.rs +++ b/datafusion/expr/src/tree_node.rs @@ -30,7 +30,7 @@ use datafusion_common::tree_node::{ use datafusion_common::{map_until_stop_and_collect, Result}; impl TreeNode for Expr { - fn apply_children_ref<'n, F: FnMut(&'n Self) -> Result>( + fn apply_children<'n, F: FnMut(&'n Self) -> Result>( &'n self, f: F, ) -> Result { diff --git a/datafusion/optimizer/src/common_subexpr_eliminate.rs b/datafusion/optimizer/src/common_subexpr_eliminate.rs index 3532a57f6206..174440dac316 100644 --- a/datafusion/optimizer/src/common_subexpr_eliminate.rs +++ b/datafusion/optimizer/src/common_subexpr_eliminate.rs @@ -682,10 +682,10 @@ impl ExprIdentifierVisitor<'_> { } } -impl TreeNodeVisitor for ExprIdentifierVisitor<'_> { +impl<'n> TreeNodeVisitor<'n> for ExprIdentifierVisitor<'_> { type Node = Expr; - fn f_down(&mut self, expr: &Expr) -> Result { + fn f_down(&mut self, expr: &'n Expr) -> Result { // related to https://github.com/apache/arrow-datafusion/issues/8814 // If the expr contain volatile expression or is a short-circuit expression, skip it. // TODO: propagate is_volatile state bottom-up + consider non-volatile sub-expressions for CSE @@ -704,7 +704,7 @@ impl TreeNodeVisitor for ExprIdentifierVisitor<'_> { Ok(TreeNodeRecursion::Continue) } - fn f_up(&mut self, expr: &Expr) -> Result { + fn f_up(&mut self, expr: &'n Expr) -> Result { let Some((down_index, sub_expr_id)) = self.pop_enter_mark() else { return Ok(TreeNodeRecursion::Continue); }; diff --git a/datafusion/physical-expr-common/src/expressions/column.rs b/datafusion/physical-expr-common/src/expressions/column.rs index 2cd52d6332fb..956c33d59b20 100644 --- a/datafusion/physical-expr-common/src/expressions/column.rs +++ b/datafusion/physical-expr-common/src/expressions/column.rs @@ -92,7 +92,7 @@ impl PhysicalExpr for Column { Ok(ColumnarValue::Array(batch.column(self.index).clone())) } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-expr-common/src/physical_expr.rs b/datafusion/physical-expr-common/src/physical_expr.rs index 00b3dd725dc2..fd8d07dc966d 100644 --- a/datafusion/physical-expr-common/src/physical_expr.rs +++ b/datafusion/physical-expr-common/src/physical_expr.rs @@ -65,7 +65,7 @@ pub trait PhysicalExpr: Send + Sync + Display + Debug + PartialEq { } /// Get a list of child PhysicalExpr that provide the input for this expr. - fn children(&self) -> Vec>; + fn children(&self) -> Vec<&Arc>; /// Returns a new PhysicalExpr where all children were replaced by new exprs. fn with_new_children( diff --git a/datafusion/physical-expr-common/src/tree_node.rs b/datafusion/physical-expr-common/src/tree_node.rs index 42dc6673af6a..6e5a22199f28 100644 --- a/datafusion/physical-expr-common/src/tree_node.rs +++ b/datafusion/physical-expr-common/src/tree_node.rs @@ -26,7 +26,7 @@ use datafusion_common::tree_node::{ConcreteTreeNode, DynTreeNode}; use datafusion_common::Result; impl DynTreeNode for dyn PhysicalExpr { - fn arc_children(&self) -> Vec> { + fn arc_children(&self) -> Vec<&Arc> { self.children() } @@ -70,7 +70,12 @@ impl ExprContext { impl ExprContext { pub fn new_default(plan: Arc) -> Self { - let children = plan.children().into_iter().map(Self::new_default).collect(); + let children = plan + .children() + .into_iter() + .cloned() + .map(Self::new_default) + .collect(); Self::new(plan, Default::default(), children) } } diff --git a/datafusion/physical-expr-common/src/utils.rs b/datafusion/physical-expr-common/src/utils.rs index 601d344e4aac..487aba945aa5 100644 --- a/datafusion/physical-expr-common/src/utils.rs +++ b/datafusion/physical-expr-common/src/utils.rs @@ -35,7 +35,12 @@ impl ExprPropertiesNode { /// given physical expression. This node initializes with default properties /// and recursively applies this to all child expressions. pub fn new_unknown(expr: Arc) -> Self { - let children = expr.children().into_iter().map(Self::new_unknown).collect(); + let children = expr + .children() + .into_iter() + .cloned() + .map(Self::new_unknown) + .collect(); Self { expr, data: ExprProperties::new_unknown(), diff --git a/datafusion/physical-expr/Cargo.toml b/datafusion/physical-expr/Cargo.toml index 798654206e63..ddd8b26d69d0 100644 --- a/datafusion/physical-expr/Cargo.toml +++ b/datafusion/physical-expr/Cargo.toml @@ -63,6 +63,7 @@ hashbrown = { workspace = true } hex = { version = "0.4", optional = true } indexmap = { workspace = true } itertools = { workspace = true, features = ["use_std"] } +lazy_static = "1.4.0" log = { workspace = true } paste = "^1.0" petgraph = "0.6.2" diff --git a/datafusion/physical-expr/src/equivalence/class.rs b/datafusion/physical-expr/src/equivalence/class.rs index 9ea456b0f879..b4d12e963611 100644 --- a/datafusion/physical-expr/src/equivalence/class.rs +++ b/datafusion/physical-expr/src/equivalence/class.rs @@ -374,7 +374,7 @@ impl EquivalenceGroup { } children .into_iter() - .map(|child| self.project_expr(mapping, &child)) + .map(|child| self.project_expr(mapping, child)) .collect::>>() .map(|children| expr.clone().with_new_children(children).unwrap()) } diff --git a/datafusion/physical-expr/src/expressions/binary.rs b/datafusion/physical-expr/src/expressions/binary.rs index 08f7523f92f0..98df0cba9f3e 100644 --- a/datafusion/physical-expr/src/expressions/binary.rs +++ b/datafusion/physical-expr/src/expressions/binary.rs @@ -312,8 +312,8 @@ impl PhysicalExpr for BinaryExpr { .map(ColumnarValue::Array) } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/case.rs b/datafusion/physical-expr/src/expressions/case.rs index 7b10df9ac146..30245f27f8b2 100644 --- a/datafusion/physical-expr/src/expressions/case.rs +++ b/datafusion/physical-expr/src/expressions/case.rs @@ -33,6 +33,7 @@ use datafusion_common::{exec_err, internal_err, DataFusionError, Result, ScalarV use datafusion_expr::ColumnarValue; use itertools::Itertools; +use lazy_static::lazy_static; type WhenThen = (Arc, Arc); @@ -258,6 +259,10 @@ impl CaseExpr { } } +lazy_static! { + static ref NO_OP: Arc = Arc::new(NoOp::new()); +} + impl PhysicalExpr for CaseExpr { /// Return a reference to Any that can be used for down-casting fn as_any(&self) -> &dyn Any { @@ -314,20 +319,20 @@ impl PhysicalExpr for CaseExpr { } } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { let mut children = vec![]; match &self.expr { - Some(expr) => children.push(expr.clone()), - None => children.push(Arc::new(NoOp::new())), + Some(expr) => children.push(expr), + None => children.push(&NO_OP), } self.when_then_expr.iter().for_each(|(cond, value)| { - children.push(cond.clone()); - children.push(value.clone()); + children.push(cond); + children.push(value); }); match &self.else_expr { - Some(expr) => children.push(expr.clone()), - None => children.push(Arc::new(NoOp::new())), + Some(expr) => children.push(expr), + None => children.push(&NO_OP), } children } diff --git a/datafusion/physical-expr/src/expressions/cast.rs b/datafusion/physical-expr/src/expressions/cast.rs index 79a44ac30cfc..4f940a792bb9 100644 --- a/datafusion/physical-expr/src/expressions/cast.rs +++ b/datafusion/physical-expr/src/expressions/cast.rs @@ -123,8 +123,8 @@ impl PhysicalExpr for CastExpr { value.cast_to(&self.cast_type, Some(&self.cast_options)) } - fn children(&self) -> Vec> { - vec![self.expr.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.expr] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/column.rs b/datafusion/physical-expr/src/expressions/column.rs index 634a56d1d683..f6525c7c0462 100644 --- a/datafusion/physical-expr/src/expressions/column.rs +++ b/datafusion/physical-expr/src/expressions/column.rs @@ -77,7 +77,7 @@ impl PhysicalExpr for UnKnownColumn { internal_err!("UnKnownColumn::evaluate() should not be called") } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-expr/src/expressions/in_list.rs b/datafusion/physical-expr/src/expressions/in_list.rs index 9ae4c2784ccf..dd61fc802441 100644 --- a/datafusion/physical-expr/src/expressions/in_list.rs +++ b/datafusion/physical-expr/src/expressions/in_list.rs @@ -372,10 +372,10 @@ impl PhysicalExpr for InListExpr { Ok(ColumnarValue::Array(Arc::new(r))) } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { let mut children = vec![]; - children.push(self.expr.clone()); - children.extend(self.list.clone()); + children.push(&self.expr); + children.extend(&self.list); children } diff --git a/datafusion/physical-expr/src/expressions/is_not_null.rs b/datafusion/physical-expr/src/expressions/is_not_null.rs index c5c673ec28ea..1918f0891fff 100644 --- a/datafusion/physical-expr/src/expressions/is_not_null.rs +++ b/datafusion/physical-expr/src/expressions/is_not_null.rs @@ -82,8 +82,8 @@ impl PhysicalExpr for IsNotNullExpr { } } - fn children(&self) -> Vec> { - vec![self.arg.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.arg] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/is_null.rs b/datafusion/physical-expr/src/expressions/is_null.rs index b0f70b6f0d7a..3430efcd7635 100644 --- a/datafusion/physical-expr/src/expressions/is_null.rs +++ b/datafusion/physical-expr/src/expressions/is_null.rs @@ -83,8 +83,8 @@ impl PhysicalExpr for IsNullExpr { } } - fn children(&self) -> Vec> { - vec![self.arg.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.arg] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/like.rs b/datafusion/physical-expr/src/expressions/like.rs index 6e0beeb0beea..eec347db8ed8 100644 --- a/datafusion/physical-expr/src/expressions/like.rs +++ b/datafusion/physical-expr/src/expressions/like.rs @@ -112,8 +112,8 @@ impl PhysicalExpr for LikeExpr { } } - fn children(&self) -> Vec> { - vec![self.expr.clone(), self.pattern.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.expr, &self.pattern] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/literal.rs b/datafusion/physical-expr/src/expressions/literal.rs index 371028959ab8..fcaf229af0a8 100644 --- a/datafusion/physical-expr/src/expressions/literal.rs +++ b/datafusion/physical-expr/src/expressions/literal.rs @@ -75,7 +75,7 @@ impl PhysicalExpr for Literal { Ok(ColumnarValue::Scalar(self.value.clone())) } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-expr/src/expressions/negative.rs b/datafusion/physical-expr/src/expressions/negative.rs index 62f865bd9b32..aed2675e0447 100644 --- a/datafusion/physical-expr/src/expressions/negative.rs +++ b/datafusion/physical-expr/src/expressions/negative.rs @@ -89,8 +89,8 @@ impl PhysicalExpr for NegativeExpr { } } - fn children(&self) -> Vec> { - vec![self.arg.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.arg] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/no_op.rs b/datafusion/physical-expr/src/expressions/no_op.rs index b558ccab154d..9148cb7c1c1d 100644 --- a/datafusion/physical-expr/src/expressions/no_op.rs +++ b/datafusion/physical-expr/src/expressions/no_op.rs @@ -68,7 +68,7 @@ impl PhysicalExpr for NoOp { internal_err!("NoOp::evaluate() should not be called") } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-expr/src/expressions/not.rs b/datafusion/physical-expr/src/expressions/not.rs index 1428be71cc21..9aaab0658d39 100644 --- a/datafusion/physical-expr/src/expressions/not.rs +++ b/datafusion/physical-expr/src/expressions/not.rs @@ -89,8 +89,8 @@ impl PhysicalExpr for NotExpr { } } - fn children(&self) -> Vec> { - vec![self.arg.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.arg] } fn with_new_children( diff --git a/datafusion/physical-expr/src/expressions/try_cast.rs b/datafusion/physical-expr/src/expressions/try_cast.rs index d25a904f7d6a..d31306e239bd 100644 --- a/datafusion/physical-expr/src/expressions/try_cast.rs +++ b/datafusion/physical-expr/src/expressions/try_cast.rs @@ -97,8 +97,8 @@ impl PhysicalExpr for TryCastExpr { } } - fn children(&self) -> Vec> { - vec![self.expr.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.expr] } fn with_new_children( diff --git a/datafusion/physical-expr/src/scalar_function.rs b/datafusion/physical-expr/src/scalar_function.rs index 14631caec55e..10e29b41031d 100644 --- a/datafusion/physical-expr/src/scalar_function.rs +++ b/datafusion/physical-expr/src/scalar_function.rs @@ -143,8 +143,8 @@ impl PhysicalExpr for ScalarFunctionExpr { Ok(output) } - fn children(&self) -> Vec> { - self.args.clone() + fn children(&self) -> Vec<&Arc> { + self.args.iter().collect() } fn with_new_children( diff --git a/datafusion/physical-plan/src/aggregates/mod.rs b/datafusion/physical-plan/src/aggregates/mod.rs index 21608db40d56..aa197605dc9f 100644 --- a/datafusion/physical-plan/src/aggregates/mod.rs +++ b/datafusion/physical-plan/src/aggregates/mod.rs @@ -696,8 +696,8 @@ impl ExecutionPlan for AggregateExec { vec![self.required_input_ordering.clone()] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( @@ -1640,7 +1640,7 @@ mod tests { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/analyze.rs b/datafusion/physical-plan/src/analyze.rs index c420581c4323..5b859804163b 100644 --- a/datafusion/physical-plan/src/analyze.rs +++ b/datafusion/physical-plan/src/analyze.rs @@ -124,8 +124,8 @@ impl ExecutionPlan for AnalyzeExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } /// AnalyzeExec is handled specially so this value is ignored diff --git a/datafusion/physical-plan/src/coalesce_batches.rs b/datafusion/physical-plan/src/coalesce_batches.rs index bc7c4a3d0673..804fabff71ac 100644 --- a/datafusion/physical-plan/src/coalesce_batches.rs +++ b/datafusion/physical-plan/src/coalesce_batches.rs @@ -117,8 +117,8 @@ impl ExecutionPlan for CoalesceBatchesExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn maintains_input_order(&self) -> Vec { diff --git a/datafusion/physical-plan/src/coalesce_partitions.rs b/datafusion/physical-plan/src/coalesce_partitions.rs index 1c725ce31f14..ce67cba2cd0e 100644 --- a/datafusion/physical-plan/src/coalesce_partitions.rs +++ b/datafusion/physical-plan/src/coalesce_partitions.rs @@ -102,8 +102,8 @@ impl ExecutionPlan for CoalescePartitionsExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn benefits_from_input_partitioning(&self) -> Vec { diff --git a/datafusion/physical-plan/src/display.rs b/datafusion/physical-plan/src/display.rs index ca93ce5e7b83..ed85c80251d6 100644 --- a/datafusion/physical-plan/src/display.rs +++ b/datafusion/physical-plan/src/display.rs @@ -501,7 +501,7 @@ mod tests { unimplemented!() } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/empty.rs b/datafusion/physical-plan/src/empty.rs index 33bf1668b3c9..11af0624db15 100644 --- a/datafusion/physical-plan/src/empty.rs +++ b/datafusion/physical-plan/src/empty.rs @@ -114,7 +114,7 @@ impl ExecutionPlan for EmptyExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/explain.rs b/datafusion/physical-plan/src/explain.rs index 649946993229..4b2edbf2045d 100644 --- a/datafusion/physical-plan/src/explain.rs +++ b/datafusion/physical-plan/src/explain.rs @@ -111,7 +111,7 @@ impl ExecutionPlan for ExplainExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } diff --git a/datafusion/physical-plan/src/filter.rs b/datafusion/physical-plan/src/filter.rs index 6729e3b9e603..6153dbacfbff 100644 --- a/datafusion/physical-plan/src/filter.rs +++ b/datafusion/physical-plan/src/filter.rs @@ -242,8 +242,8 @@ impl ExecutionPlan for FilterExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn maintains_input_order(&self) -> Vec { diff --git a/datafusion/physical-plan/src/insert.rs b/datafusion/physical-plan/src/insert.rs index 259db644ae0a..fa30141a1934 100644 --- a/datafusion/physical-plan/src/insert.rs +++ b/datafusion/physical-plan/src/insert.rs @@ -248,8 +248,8 @@ impl ExecutionPlan for DataSinkExec { vec![true] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/physical-plan/src/joins/cross_join.rs b/datafusion/physical-plan/src/joins/cross_join.rs index 9d1de3715f54..92443d06856a 100644 --- a/datafusion/physical-plan/src/joins/cross_join.rs +++ b/datafusion/physical-plan/src/joins/cross_join.rs @@ -207,8 +207,8 @@ impl ExecutionPlan for CrossJoinExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn metrics(&self) -> Option { diff --git a/datafusion/physical-plan/src/joins/hash_join.rs b/datafusion/physical-plan/src/joins/hash_join.rs index d3abedbe3806..e669517be400 100644 --- a/datafusion/physical-plan/src/joins/hash_join.rs +++ b/datafusion/physical-plan/src/joins/hash_join.rs @@ -683,8 +683,8 @@ impl ExecutionPlan for HashJoinExec { Self::maintains_input_order(self.join_type) } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn with_new_children( diff --git a/datafusion/physical-plan/src/joins/nested_loop_join.rs b/datafusion/physical-plan/src/joins/nested_loop_join.rs index 47e262c3c8f6..18518600ef2f 100644 --- a/datafusion/physical-plan/src/joins/nested_loop_join.rs +++ b/datafusion/physical-plan/src/joins/nested_loop_join.rs @@ -292,8 +292,8 @@ impl ExecutionPlan for NestedLoopJoinExec { ] } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn with_new_children( diff --git a/datafusion/physical-plan/src/joins/sort_merge_join.rs b/datafusion/physical-plan/src/joins/sort_merge_join.rs index 1cc7bf4700d1..ec83fe3f2af8 100644 --- a/datafusion/physical-plan/src/joins/sort_merge_join.rs +++ b/datafusion/physical-plan/src/joins/sort_merge_join.rs @@ -298,8 +298,8 @@ impl ExecutionPlan for SortMergeJoinExec { Self::maintains_input_order(self.join_type) } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn with_new_children( diff --git a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs index 9d48c2a7d408..0d902af9c6cc 100644 --- a/datafusion/physical-plan/src/joins/symmetric_hash_join.rs +++ b/datafusion/physical-plan/src/joins/symmetric_hash_join.rs @@ -427,8 +427,8 @@ impl ExecutionPlan for SymmetricHashJoinExec { ] } - fn children(&self) -> Vec> { - vec![self.left.clone(), self.right.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.left, &self.right] } fn with_new_children( diff --git a/datafusion/physical-plan/src/lib.rs b/datafusion/physical-plan/src/lib.rs index 8d8a3e71031e..739bff2cfa23 100644 --- a/datafusion/physical-plan/src/lib.rs +++ b/datafusion/physical-plan/src/lib.rs @@ -215,7 +215,7 @@ pub trait ExecutionPlan: Debug + DisplayAs + Send + Sync { /// The returned list will be empty for leaf nodes such as scans, will contain /// a single value for unary nodes, or two values for binary nodes (such as /// joins). - fn children(&self) -> Vec>; + fn children(&self) -> Vec<&Arc>; /// Returns a new `ExecutionPlan` where all existing children were replaced /// by the `children`, in order @@ -841,7 +841,7 @@ mod tests { unimplemented!() } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } @@ -900,7 +900,7 @@ mod tests { unimplemented!() } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/limit.rs b/datafusion/physical-plan/src/limit.rs index 4f8ff4c5606e..4c6d1b3674d5 100644 --- a/datafusion/physical-plan/src/limit.rs +++ b/datafusion/physical-plan/src/limit.rs @@ -124,8 +124,8 @@ impl ExecutionPlan for GlobalLimitExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn required_input_distribution(&self) -> Vec { @@ -334,8 +334,8 @@ impl ExecutionPlan for LocalLimitExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn benefits_from_input_partitioning(&self) -> Vec { diff --git a/datafusion/physical-plan/src/memory.rs b/datafusion/physical-plan/src/memory.rs index 883cdb540a9e..39ae8d551f4b 100644 --- a/datafusion/physical-plan/src/memory.rs +++ b/datafusion/physical-plan/src/memory.rs @@ -116,7 +116,7 @@ impl ExecutionPlan for MemoryExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } diff --git a/datafusion/physical-plan/src/placeholder_row.rs b/datafusion/physical-plan/src/placeholder_row.rs index c94c2b0607d7..3b10cc0ac435 100644 --- a/datafusion/physical-plan/src/placeholder_row.rs +++ b/datafusion/physical-plan/src/placeholder_row.rs @@ -132,7 +132,7 @@ impl ExecutionPlan for PlaceholderRowExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/projection.rs b/datafusion/physical-plan/src/projection.rs index f72815c01a9e..8341549340dd 100644 --- a/datafusion/physical-plan/src/projection.rs +++ b/datafusion/physical-plan/src/projection.rs @@ -193,8 +193,8 @@ impl ExecutionPlan for ProjectionExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn maintains_input_order(&self) -> Vec { diff --git a/datafusion/physical-plan/src/recursive_query.rs b/datafusion/physical-plan/src/recursive_query.rs index ed897d78f0c8..9a0b66caba31 100644 --- a/datafusion/physical-plan/src/recursive_query.rs +++ b/datafusion/physical-plan/src/recursive_query.rs @@ -120,8 +120,8 @@ impl ExecutionPlan for RecursiveQueryExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.static_term.clone(), self.recursive_term.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.static_term, &self.recursive_term] } // TODO: control these hints and see whether we can @@ -358,7 +358,9 @@ fn reset_plan_states(plan: Arc) -> Result() { Ok(Transformed::no(plan)) } else { - let new_plan = plan.clone().with_new_children(plan.children())?; + let new_plan = plan + .clone() + .with_new_children(plan.children().into_iter().cloned().collect())?; Ok(Transformed::yes(new_plan)) } }) diff --git a/datafusion/physical-plan/src/repartition/mod.rs b/datafusion/physical-plan/src/repartition/mod.rs index e31fdc6ee2c2..65f7d5070a5d 100644 --- a/datafusion/physical-plan/src/repartition/mod.rs +++ b/datafusion/physical-plan/src/repartition/mod.rs @@ -519,8 +519,8 @@ impl ExecutionPlan for RepartitionExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/physical-plan/src/sorts/partial_sort.rs b/datafusion/physical-plan/src/sorts/partial_sort.rs index d24bc5a670e5..ad5d485cffc9 100644 --- a/datafusion/physical-plan/src/sorts/partial_sort.rs +++ b/datafusion/physical-plan/src/sorts/partial_sort.rs @@ -250,8 +250,8 @@ impl ExecutionPlan for PartialSortExec { vec![false] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/physical-plan/src/sorts/sort.rs b/datafusion/physical-plan/src/sorts/sort.rs index c684748bb29a..2a4862534590 100644 --- a/datafusion/physical-plan/src/sorts/sort.rs +++ b/datafusion/physical-plan/src/sorts/sort.rs @@ -903,8 +903,8 @@ impl ExecutionPlan for SortExec { } } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn benefits_from_input_partitioning(&self) -> Vec { diff --git a/datafusion/physical-plan/src/sorts/sort_preserving_merge.rs b/datafusion/physical-plan/src/sorts/sort_preserving_merge.rs index 88c6c312b94b..8a349bd22abf 100644 --- a/datafusion/physical-plan/src/sorts/sort_preserving_merge.rs +++ b/datafusion/physical-plan/src/sorts/sort_preserving_merge.rs @@ -173,8 +173,8 @@ impl ExecutionPlan for SortPreservingMergeExec { vec![true] } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/physical-plan/src/streaming.rs b/datafusion/physical-plan/src/streaming.rs index d174e3b8b6ca..ff57adde4e2e 100644 --- a/datafusion/physical-plan/src/streaming.rs +++ b/datafusion/physical-plan/src/streaming.rs @@ -217,7 +217,7 @@ impl ExecutionPlan for StreamingTableExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/test/exec.rs b/datafusion/physical-plan/src/test/exec.rs index b4f1eac0a655..d5ad9292b49d 100644 --- a/datafusion/physical-plan/src/test/exec.rs +++ b/datafusion/physical-plan/src/test/exec.rs @@ -185,7 +185,7 @@ impl ExecutionPlan for MockExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } @@ -343,7 +343,7 @@ impl ExecutionPlan for BarrierExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { unimplemented!() } @@ -452,7 +452,7 @@ impl ExecutionPlan for ErrorExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { unimplemented!() } @@ -535,7 +535,7 @@ impl ExecutionPlan for StatisticsExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } @@ -627,7 +627,7 @@ impl ExecutionPlan for BlockingExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } @@ -768,7 +768,7 @@ impl ExecutionPlan for PanicExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { // this is a leaf node and has no children vec![] } diff --git a/datafusion/physical-plan/src/tree_node.rs b/datafusion/physical-plan/src/tree_node.rs index 46460cbb6684..45bd320d8943 100644 --- a/datafusion/physical-plan/src/tree_node.rs +++ b/datafusion/physical-plan/src/tree_node.rs @@ -26,7 +26,7 @@ use datafusion_common::tree_node::{ConcreteTreeNode, DynTreeNode}; use datafusion_common::Result; impl DynTreeNode for dyn ExecutionPlan { - fn arc_children(&self) -> Vec> { + fn arc_children(&self) -> Vec<&Arc> { self.children() } @@ -71,7 +71,12 @@ impl PlanContext { impl PlanContext { pub fn new_default(plan: Arc) -> Self { - let children = plan.children().into_iter().map(Self::new_default).collect(); + let children = plan + .children() + .into_iter() + .cloned() + .map(Self::new_default) + .collect(); Self::new(plan, Default::default(), children) } } diff --git a/datafusion/physical-plan/src/union.rs b/datafusion/physical-plan/src/union.rs index 1354644788ea..dc7d270bae25 100644 --- a/datafusion/physical-plan/src/union.rs +++ b/datafusion/physical-plan/src/union.rs @@ -196,8 +196,8 @@ impl ExecutionPlan for UnionExec { &self.cache } - fn children(&self) -> Vec> { - self.inputs.clone() + fn children(&self) -> Vec<&Arc> { + self.inputs.iter().collect() } fn maintains_input_order(&self) -> Vec { @@ -387,8 +387,8 @@ impl ExecutionPlan for InterleaveExec { &self.cache } - fn children(&self) -> Vec> { - self.inputs.clone() + fn children(&self) -> Vec<&Arc> { + self.inputs.iter().collect() } fn maintains_input_order(&self) -> Vec { diff --git a/datafusion/physical-plan/src/unnest.rs b/datafusion/physical-plan/src/unnest.rs index 06dd8230d39e..af50a307efb2 100644 --- a/datafusion/physical-plan/src/unnest.rs +++ b/datafusion/physical-plan/src/unnest.rs @@ -127,8 +127,8 @@ impl ExecutionPlan for UnnestExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn with_new_children( diff --git a/datafusion/physical-plan/src/values.rs b/datafusion/physical-plan/src/values.rs index 2aa893fd2916..4d385812d4a8 100644 --- a/datafusion/physical-plan/src/values.rs +++ b/datafusion/physical-plan/src/values.rs @@ -167,7 +167,7 @@ impl ExecutionPlan for ValuesExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs b/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs index cff91283eb6e..48f1bee59bbf 100644 --- a/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs +++ b/datafusion/physical-plan/src/windows/bounded_window_agg_exec.rs @@ -250,8 +250,8 @@ impl ExecutionPlan for BoundedWindowAggExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn required_input_ordering(&self) -> Vec>> { diff --git a/datafusion/physical-plan/src/windows/window_agg_exec.rs b/datafusion/physical-plan/src/windows/window_agg_exec.rs index 1507902c22ea..eb01da2ec094 100644 --- a/datafusion/physical-plan/src/windows/window_agg_exec.rs +++ b/datafusion/physical-plan/src/windows/window_agg_exec.rs @@ -184,8 +184,8 @@ impl ExecutionPlan for WindowAggExec { &self.cache } - fn children(&self) -> Vec> { - vec![self.input.clone()] + fn children(&self) -> Vec<&Arc> { + vec![&self.input] } fn maintains_input_order(&self) -> Vec { diff --git a/datafusion/physical-plan/src/work_table.rs b/datafusion/physical-plan/src/work_table.rs index b3c9043d4fdc..003957947fec 100644 --- a/datafusion/physical-plan/src/work_table.rs +++ b/datafusion/physical-plan/src/work_table.rs @@ -169,7 +169,7 @@ impl ExecutionPlan for WorkTableExec { &self.cache } - fn children(&self) -> Vec> { + fn children(&self) -> Vec<&Arc> { vec![] } diff --git a/datafusion/proto/src/physical_plan/mod.rs b/datafusion/proto/src/physical_plan/mod.rs index 0515ed5006aa..4c581f99b32a 100644 --- a/datafusion/proto/src/physical_plan/mod.rs +++ b/datafusion/proto/src/physical_plan/mod.rs @@ -1972,6 +1972,7 @@ impl AsExecutionPlan for protobuf::PhysicalPlanNode { let inputs: Vec = plan_clone .children() .into_iter() + .cloned() .map(|i| { protobuf::PhysicalPlanNode::try_from_physical_plan( i,