[GitHub] [arrow-datafusion] alamb commented on a diff in pull request #5761: Minor: Avoid an unecessary contruction in `map_children` some extra plan construction

["alamb (via GitHub)" <gi...@apache.org>]

alamb commented on code in PR #5761:
URL: https://github.com/apache/arrow-datafusion/pull/5761#discussion_r1150987073


##########
datafusion/expr/src/tree_node/plan.rs:
##########
@@ -108,11 +108,20 @@ impl TreeNode for LogicalPlan {
     where
         F: FnMut(Self) -> Result<Self>,
     {
-        let children = self.inputs().into_iter().cloned().collect::<Vec<_>>();
-        if !children.is_empty() {
-            let new_children: Result<Vec<_>> =
-                children.into_iter().map(transform).collect();
-            self.with_new_inputs(new_children?.as_slice())
+        let old_children = self.inputs();
+        let new_children = old_children
+            .iter()
+            .map(|&c| c.clone())
+            .map(transform)
+            .collect::<Result<Vec<_>>>()?;
+
+        // if any changes made, make a new child
+        if old_children

Review Comment:
   As @mingmwang  says
   
   https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.any
   
   "An empty iterator returns false."



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org