maropu commented on a change in pull request #32595:
URL: https://github.com/apache/spark/pull/32595#discussion_r637414764
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/EquivalentExpressions.scala
##########
@@ -128,7 +128,11 @@ class EquivalentExpressions {
// a subexpression among values doesn't need to be in conditions because
no matter which
// condition is true, it will be evaluated.
val conditions = c.branches.tail.map(_._1)
- val values = c.branches.map(_._2) ++ c.elseValue
+ val values = if (c.elseValue.nonEmpty) {
Review comment:
> However, for a statement to be in "all branches" of a CaseWhen
statement, it must also be in the elseValue.
It's better to leave a simple comment here about why we need to check
`elseValue`.
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/SubexpressionEliminationSuite.scala
##########
@@ -226,6 +226,17 @@ class SubexpressionEliminationSuite extends SparkFunSuite
with ExpressionEvalHel
val equivalence3 = new EquivalentExpressions
equivalence3.addExprTree(caseWhenExpr3)
assert(equivalence3.getAllEquivalentExprs.count(_.size == 2) == 0)
+
+ val conditions4 = (GreaterThan(add1, Literal(3)), add1) ::
+ (GreaterThan(add2, Literal(4)), add1) ::
+ (GreaterThan(add2, Literal(5)), add1) :: Nil
+
+ val caseWhenExpr4 = CaseWhen(conditions4, None)
+ val equivalence4 = new EquivalentExpressions
+ equivalence4.addExprTree(caseWhenExpr4)
+
+ // `add1` is not in the elseValue, so we can't extract it from the branches
+ assert(equivalence4.getAllEquivalentExprs.count(_.size == 2) == 0)
Review comment:
Could you put this new test in a new test unit `test("SPARK-35449: ..."?
--
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]
