Address comments

Author: wangyum

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala

@@ -471,7 +471,8 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
   }
 
   private def isAlwaysFalse(exps: Seq[Expression], equalTo: Literal): Boolean = {
-    exps.forall(!EqualTo(_, equalTo).eval(EmptyRow).asInstanceOf[Boolean])
+    exps.forall(_.isInstanceOf[Literal]) &&
+      exps.forall(!EqualTo(_, equalTo).eval(EmptyRow).asInstanceOf[Boolean])
   }
 
   def apply(plan: LogicalPlan): LogicalPlan = plan transform {
@@ -528,13 +529,12 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
           e.copy(branches = branches.take(i).map(branch => (branch._1, elseValue)))
         }
 
-      case EqualTo(i @ If(_, trueValue: Literal, falseValue: Literal), right: Literal)
+      case EqualTo(i @ If(_, trueValue, falseValue), right: Literal)
           if i.deterministic && isAlwaysFalse(trueValue :: falseValue :: Nil, right) =>
         FalseLiteral
 
-      case EqualTo(c @ CaseWhen(branches, elseValue), right: Literal) if c.deterministic &&
-          (branches.map(_._2) ++ elseValue).forall(_.isInstanceOf[Literal]) &&
-          isAlwaysFalse(branches.map(_._2) ++ elseValue, right) =>
+      case EqualTo(c @ CaseWhen(branches, elseValue), right: Literal)
+          if c.deterministic && isAlwaysFalse(branches.map(_._2) ++ elseValue, right) =>
         FalseLiteral
     }
   }

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/SimplifyConditionalSuite.scala

@@ -220,22 +220,23 @@ class SimplifyConditionalSuite extends PlanTest with ExpressionEvalHelper with P
     assert(!nonDeterministic.deterministic)
     assertEquivalent(EqualTo(nonDeterministic, Literal(-1)), EqualTo(nonDeterministic, Literal(-1)))
 
-    // null check, SPARK-33798 will not change these behaviors.
-    assertEquivalent(
-      EqualTo(If(FalseLiteral, Literal(null, IntegerType), Literal(1)), Literal(1)),
-      TrueLiteral)
+    // null check, SPARK-33798 will change the following two behaviors.
     assertEquivalent(
-      EqualTo(If(TrueLiteral, Literal(null, IntegerType), Literal(1)), Literal(1)),
-      Literal(null, BooleanType))
+      EqualTo(If(a === Literal(1), Literal(null, IntegerType), Literal(1)), Literal(2)),
+      FalseLiteral)
     assertEquivalent(
-      EqualTo(If(FalseLiteral, Literal(null, IntegerType), Literal(null, IntegerType)), Literal(1)),
-      Literal(null, BooleanType))
+      EqualTo(If(a =!= Literal(1), Literal(1), Literal(2)), Literal(null, IntegerType)),
+      FalseLiteral)
 
     assertEquivalent(
-      EqualTo(If(FalseLiteral, Literal(1), Literal(2)), Literal(null, IntegerType)),
-      Literal(null, BooleanType))
+      EqualTo(If(a === Literal(1), Literal(null, IntegerType), Literal(1)), Literal(1)),
+      EqualTo(If(a === Literal(1), Literal(null, IntegerType), Literal(1)), Literal(1)))
     assertEquivalent(
-      EqualTo(If(TrueLiteral, Literal(1), Literal(2)), Literal(null, IntegerType)),
+      EqualTo(If(a =!= Literal(1), Literal(null, IntegerType), Literal(1)), Literal(1)),
+      EqualTo(If(a =!= Literal(1), Literal(null, IntegerType), Literal(1)), Literal(1)))
+    assertEquivalent(
+      EqualTo(If(a =!= Literal(1), Literal(null, IntegerType), Literal(null, IntegerType)),
+        Literal(1)),
       Literal(null, BooleanType))
   }