[GitHub] [spark] LuciferYang commented on a change in pull request #35890: [SPARK-38585][SQL] Remove the constant arg `forceCopy` for the private method `mapChildren` in `TreeNode`

[GitBox <gi...@apache.org>]

LuciferYang commented on a change in pull request #35890:
URL: https://github.com/apache/spark/pull/35890#discussion_r829663269



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
##########
@@ -610,24 +610,15 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product with Tre
 
   /**
    * Returns a copy of this node where `f` has been applied to all the nodes in `children`.
+   * This method force making a copy of the nodes even if no child has been changed.
    * @param f The transform function to be applied on applicable `TreeNode` elements.
-   * @param forceCopy Whether to force making a copy of the nodes even if no child has been changed.
    */
-  private def mapChildren(
-      f: BaseType => BaseType,
-      forceCopy: Boolean): BaseType = {
-    var changed = false
+  private def mapChildrenWithForceCopy(f: BaseType => BaseType): BaseType = {

Review comment:
       Should we move this method into `clone()` method? It is now only used by `TreeNode.clone`
   
   




-- 
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: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org