[SPARK-35886][SQL] CodeGenerator.getLocalInputVariableValues should handle case that expression match SubExprEliminationState but not VariableValue

[AngersZhuuuu]

What changes were proposed in this pull request?

For test case

spark.sql(
  """
    |CREATE TABLE t1 (
    |  c1 DECIMAL(18,6),
    |  c2 DECIMAL(18,6),
    |  c3 DECIMAL(18,6))
    |USING parquet;
    |""".stripMargin)

spark.sql("SELECT sum(c1 * c3) + sum(c2 * c3) FROM t1").show

failed when codegen

20:23:36.272 ERROR org.apache.spark.sql.catalyst.expressions.codegen.CodeGenerator: failed to compile: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 56, Column 6: Expression "agg_exprIsNull_2_0" is not an rvalue
org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 56, Column 6: Expression "agg_exprIsNull_2_0" is not an rvalue
	at org.codehaus.janino.UnitCompiler.compileError(UnitCompiler.java:12675)
	at org.codehaus.janino.UnitCompiler.toRvalueOrCompileException(UnitCompiler.java:7676)

It's caused by when call CodeGenerator.getLocalInputVariableValues, expression match SubExprEliminationState but is not VariableValue, then skip this whole expression, but when call splitAggregateExpressions, since we need to find all input variables by this method, this will cause we skip some inputs.
Here in this case , we put expression's children to stack too.

Why are the changes needed?

Fix Bug

Does this PR introduce any user-facing change?

No

How was this patch tested?

Added UT

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44851/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44851/

[SparkQA]

Test build #140318 has finished for PR 33082 at commit d03657c.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AngersZhuuuu]

ping @cloud-fan @maropu @wangyum @Ngone51

[SparkQA]

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44865/

[SparkQA]

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/44865/

[SparkQA]

Test build #140333 has finished for PR 33082 at commit 01e5d38.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[wangyum]

Also cc @viirya

[viirya]

you mean but not ONLY VariableValue?

[viirya]

Oh, some SubExprEliminationState contains not VariableValue but still some statements that includes other expressions?

[AngersZhuuuu]

Oh, some SubExprEliminationState contains not VariableValue but still some statements that includes other expressions?

Yes, the expression contains input var may match SubExprEliminationState and just skiped.

[viirya]

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 as co-author there. Thanks.

[viirya]

Do we have "subquery" here? Please update the description if it is not "subquery".

[viirya]

Looks reasonable, although the description and the title confuse.

[AngersZhuuuu]

Do we have "subquery" here? Please update the description if it is not "subquery".

My mistake, should be SubExprEliminationState

[AngersZhuuuu]

Looks reasonable, although the description and the title confuse.

Updated, how about current?

[AngersZhuuuu]

Close this according to #33103