Github user mgaido91 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20637#discussion_r212589702
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionEvalHelper.scala
 ---
    @@ -223,8 +223,9 @@ trait ExpressionEvalHelper extends 
GeneratorDrivenPropertyChecks with PlanTestBa
               }
             } else {
               val lit = InternalRow(expected, expected)
    +          val dtAsNullable = expression.dataType.asNullable
    --- End diff --
    
    Mmh, but a test using a non-nullable expression returning `null` should 
fail IMHO. So if without this there are test failures, I think those tests are 
wrong and/or we are hiding potential bugs in the codebase, right? Are there 
many test failures without this change?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to