sigmod commented on a change in pull request #32060:
URL: https://github.com/apache/spark/pull/32060#discussion_r609408036



##########
File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/QueryPlan.scala
##########
@@ -80,28 +105,57 @@ abstract class QueryPlan[PlanType <: QueryPlan[PlanType]]
    * transformExpressionsDown or transformExpressionsUp should be used.
    *
    * @param rule the rule to be applied to every expression in this operator.
+   * @param cond  a Lambda expression to prune tree traversals. If 
`cond.apply` returns false
+   *              on an expression T, skips processing T and its subtree; 
otherwise, processes
+   *              T and its subtree recursively.
+   * @param ruleId is a unique Id for `rule` to prune unnecessary tree 
traversals. When it is
+   *        RuleId.UnknownId, no pruning happens. Otherwise, if `rule`(with id 
`ruleId`) has been
+   *        marked as in effective on an expression T, skips processing T and 
its subtree. Do not
+   *        pass it if the rule is not purely functional and reads a varying 
initial state for
+   *        different invocations.
    */
-  def transformExpressions(rule: PartialFunction[Expression, Expression]): 
this.type = {
-    transformExpressionsDown(rule)
+  def transformExpressions(rule: PartialFunction[Expression, Expression],

Review comment:
       I tried to overload transform functions, but ran into the following 
issue:
   
   - the overload does work with Scala 2.13, but doesn't work with Scala 2.11 
nor the default Scala version Spark build uses.  Basically, the overload fails 
existing call sites like `e transform {....}`. To repro what happens, we can do 
the following in a Scala console:
   
           scala>  object X { def f(i: Int) = i+1 ; def f(g: String=> String) = 
g("a") }
                         defined object X
   
           scala>  X.f {case s => s + "b"}
                        <console>:13: error: missing parameter type for 
expanded function
                        The argument types of an anonymous function must be 
fully known. (SLS 8.5)
                        Expected type was: ?
                         X.f {case s => s + "b"}
   
   - also, I found that style guide mentioned to not use multiple parameter 
lists: https://github.com/databricks/scala-style-guide#multiple-parameter-lists
   
   Let me know if you have a better thought.




-- 
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to