[GitHub] [spark] viirya commented on a change in pull request #33082: [SPARK-35886][SQL] CodeGenerator.getLocalInputVariableValues should handle case that expression match SubExprEliminationState but not VariableValue

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

viirya commented on a change in pull request #33082:
URL: https://github.com/apache/spark/pull/33082#discussion_r659221432



##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala
##########
@@ -1766,19 +1766,20 @@ object CodeGenerator extends Logging {
       argSet += JavaCode.variable(ctx.INPUT_ROW, classOf[InternalRow])
     }
 
+    val stack = mutable.Stack[Expression](expr)
+
     // Collects local variables from a given `expr` tree
-    val collectLocalVariable = (ev: ExprValue) => ev match {
+    val collectLocalVariable = (ev: ExprValue, e: Expression) => ev match {
       case vv: VariableValue => argSet += vv
-      case _ =>
+      case _ => stack.pushAll(e.children)
     }
 
-    val stack = mutable.Stack[Expression](expr)
     while (stack.nonEmpty) {
       stack.pop() match {
         case e if subExprs.contains(e) =>
           val SubExprEliminationState(isNull, value) = subExprs(e)
-          collectLocalVariable(value)
-          collectLocalVariable(isNull)
+          collectLocalVariable(value, e)
+          collectLocalVariable(isNull, e)

Review comment:
       Hmm, after second look, it looks not root cause. A subexpr should be evaluated before using it. 
   
   So technically all its children expressions are not needed.
   
   I found the root cause. `PromotePrecision` overwrites `Expression.genCode` where is subexpression replace happens. So `PromotePrecision` skips subexpression elimination. I will submit another PR and include your test there. Thanks.




-- 
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