[SPARK-32721][SQL][FOLLOWUP] Simplify if clauses with null and boolean

Author: sunchao

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

@@ -465,6 +465,8 @@ object SimplifyConditionals extends Rule[LogicalPlan] with PredicateHelper {
         if cond.deterministic && trueValue.semanticEquals(falseValue) => trueValue
       case If(cond, l @ Literal(null, _), FalseLiteral) if !cond.nullable => And(cond, l)
       case If(cond, l @ Literal(null, _), TrueLiteral) if !cond.nullable => Or(Not(cond), l)
+      case If(cond, FalseLiteral, l @ Literal(null, _)) if !cond.nullable => And(Not(cond), l)
+      case If(cond, TrueLiteral, l @ Literal(null, _)) if !cond.nullable => Or(cond, l)
 
       case e @ CaseWhen(branches, elseValue) if branches.exists(x => falseOrNullLiteral(x._1)) =>
         // If there are branches that are always false, remove them.

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

@@ -166,29 +166,37 @@ class SimplifyConditionalSuite extends PlanTest with ExpressionEvalHelper with P
     )
   }
 
-  test("simplify if when then clause is null and else clause is boolean") {
+  test("simplify if when one clause is null and another is boolean") {
     val p = IsNull('a)
     val nullLiteral = Literal(null, BooleanType)
     assertEquivalent(If(p, nullLiteral, FalseLiteral), And(p, nullLiteral))
     assertEquivalent(If(p, nullLiteral, TrueLiteral), Or(IsNotNull('a), nullLiteral))
+    assertEquivalent(If(p, FalseLiteral, nullLiteral), And(IsNotNull('a), nullLiteral))
+    assertEquivalent(If(p, TrueLiteral, nullLiteral), Or(IsNull('a), nullLiteral))
 
     // the rule should not apply to nullable predicate
     Seq(TrueLiteral, FalseLiteral).foreach { b =>
       assertEquivalent(If(GreaterThan('a, 42), nullLiteral, b),
         If(GreaterThan('a, 42), nullLiteral, b))
+      assertEquivalent(If(GreaterThan('a, 42), b, nullLiteral),
+        If(GreaterThan('a, 42), b, nullLiteral))
     }
 
     // check evaluation also
     Seq(TrueLiteral, FalseLiteral).foreach { b =>
       checkEvaluation(If(b, nullLiteral, FalseLiteral), And(b, nullLiteral).eval(EmptyRow))
       checkEvaluation(If(b, nullLiteral, TrueLiteral), Or(Not(b), nullLiteral).eval(EmptyRow))
+      checkEvaluation(If(b, FalseLiteral, nullLiteral), And(Not(b), nullLiteral).eval(EmptyRow))
+      checkEvaluation(If(b, TrueLiteral, nullLiteral), Or(b, nullLiteral).eval(EmptyRow))
     }
 
     // should have no effect on expressions with nullable if condition
     assert((Factorial(5) > 100L).nullable)
     Seq(TrueLiteral, FalseLiteral).foreach { b =>
       checkEvaluation(If(Factorial(5) > 100L, nullLiteral, b),
         If(Factorial(5) > 100L, nullLiteral, b).eval(EmptyRow))
+      checkEvaluation(If(Factorial(5) > 100L, b, nullLiteral),
+        If(Factorial(5) > 100L, b, nullLiteral).eval(EmptyRow))
     }
   }
 }