addressed review comments

apache · Dec 1, 2016 · 737b21f · 737b21f
1 parent 93d4354
commit 737b21f
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 8 deletions.
diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/ScalaReflection.scala
@@ -591,13 +591,9 @@ object ScalaReflection extends ScalaReflection {
               "cannot be used as field name\n" + walkedTypePath.mkString("\n"))
           }
 
-          // we know primitive type takes only non-null, or
-          // we can infer correct nullability for struct's fieldValue by a guard
-          // using If(IsNull()) at the last line of this section
-          val returnNonNull = fieldType.typeSymbol.asClass.isPrimitive ||
-            definedByConstructorParams(fieldType)
-          val fieldValue = Invoke(inputObject, fieldName, dataTypeFor(fieldType),
-            returnNullable = !returnNonNull)
+          val fieldValue = Invoke(
+            AssertNotNull(inputObject, walkedTypePath), fieldName, dataTypeFor(fieldType),
+            returnNullable = !fieldType.typeSymbol.asClass.isPrimitive)
           val clsName = getClassNameFromType(fieldType)
           val newPath = s"""- field (class: "$clsName", name: "$fieldName")""" +: walkedTypePath
           expressions.Literal(fieldName) :: serializerFor(fieldValue, fieldType, newPath) :: Nil

diff --git a/...talyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala b/...talyst/src/test/scala/org/apache/spark/sql/catalyst/encoders/ExpressionEncoderSuite.scala
@@ -300,6 +300,11 @@ class ExpressionEncoderSuite extends PlanTest with AnalysisTest {
   encodeDecodeTest(
     ReferenceValueClass(ReferenceValueClass.Container(1)), "reference value class")
 
+  encodeDecodeTest(Option(31), "option of int")
+  encodeDecodeTest(Option.empty[Int], "empty option of int")
+  encodeDecodeTest(Option("abc"), "option of string")
+  encodeDecodeTest(Option.empty[String], "empty option of string")
+
   productTest(("UDT", new ExamplePoint(0.1, 0.2)))
 
   test("nullable of encoder schema") {
@@ -340,7 +345,7 @@ class ExpressionEncoderSuite extends PlanTest with AnalysisTest {
 
   test("nullable of encoder serializer") {
     def checkNullable[T: Encoder](nullable: Boolean): Unit = {
-      assert(encoderFor[T].serializer.map(_.nullable) === nullable)
+      assert(encoderFor[T].serializer.forall(_.nullable === nullable))
     }
 
     // test for flat encoders