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

    https://github.com/apache/spark/pull/20746#discussion_r172455421
  
    --- Diff: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
 ---
    @@ -84,4 +85,23 @@ class ObjectExpressionsSuite extends SparkFunSuite with 
ExpressionEvalHelper {
           checkEvaluation(wrapObject, expected, 
InternalRow.fromSeq(Seq(input)))
         }
       }
    +
    +  test("SPARK-23594 GetExternalRowField should support interpreted 
execution") {
    +    val inputObject = BoundReference(0, ObjectType(classOf[Row]), nullable 
= true)
    +    val getRowField = GetExternalRowField(inputObject, index = 0, 
fieldName = "c0")
    +    Seq((Row(1), 1), (Row(3), 3)).foreach { case (input, expected) =>
    +      checkEvaluation(getRowField, expected, 
InternalRow.fromSeq(Seq(input)))
    +    }
    +
    +    // If an input row or a field are null, a runtime exception will be 
thrown
    +    val errMsg1 = intercept[RuntimeException] {
    +      evaluate(getRowField, InternalRow.fromSeq(Seq(null)))
    --- End diff --
    
    Minor comment: This only tests the interpreted code path. Maybe we should 
add something to the `ExpressionEvalHelper` harness to test all paths in case 
of failure. Let me know if you want to pick this up, we can also spin that off 
into a separate PR.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to