maryannxue commented on a change in pull request #24706: [SPARK-23128][SQL] A
new approach to do adaptive execution in Spark SQL
URL: https://github.com/apache/spark/pull/24706#discussion_r293907198
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
##########
@@ -317,76 +317,92 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]]
extends Product {
*/
def mapChildren(f: BaseType => BaseType): BaseType = {
if (children.nonEmpty) {
- var changed = false
- def mapChild(child: Any): Any = child match {
- case arg: TreeNode[_] if containsChild(arg) =>
- val newChild = f(arg.asInstanceOf[BaseType])
- if (!(newChild fastEquals arg)) {
- changed = true
- newChild
- } else {
- arg
- }
- case tuple@(arg1: TreeNode[_], arg2: TreeNode[_]) =>
- val newChild1 = if (containsChild(arg1)) {
- f(arg1.asInstanceOf[BaseType])
- } else {
- arg1.asInstanceOf[BaseType]
- }
+ mapProductElements(f, applyToAll = false)
+ } else {
+ this
+ }
+ }
- val newChild2 = if (containsChild(arg2)) {
- f(arg2.asInstanceOf[BaseType])
- } else {
- arg2.asInstanceOf[BaseType]
- }
+ /**
+ * Returns a copy of this node where `f` has been applied to all applicable
`TreeNode` elements
+ * in the productIterator.
+ * @param f the transform function to be applied on applicable `TreeNode`
elements.
+ * @param applyToAll If true, the transform function will be applied to all
`TreeNode` elements
+ * even for non-child elements; otherwise, the function
will only be applied
+ * on children nodes. Also, when this is true, a copy of
this node will be
+ * returned even if no elements have been changed.
+ */
+ private def mapProductElements(
+ f: BaseType => BaseType,
+ applyToAll: Boolean): BaseType = {
+ var changed = false
- if (!(newChild1 fastEquals arg1) || !(newChild2 fastEquals arg2)) {
- changed = true
- (newChild1, newChild2)
- } else {
- tuple
- }
- case other => other
- }
+ def mapChild(child: Any): Any = child match {
+ case arg: TreeNode[_] if applyToAll || containsChild(arg) =>
Review comment:
I double checked and there's no "fake LeafNode" in the logical plan space.
So I removed the "applyToAll" from the condition for transforming the elements
and renamed it to "forceCopy". I've also changed the method name back to
"mapChildren" since it's only for children nodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
