maropu commented on a change in pull request #33212:
URL: https://github.com/apache/spark/pull/33212#discussion_r667665610



##########
File path: 
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/json/JacksonParserSuite.scala
##########
@@ -54,4 +55,64 @@ class JacksonParserSuite extends SparkFunSuite {
       filters = Seq(EqualTo("d", 3.14)),
       expected = Seq(InternalRow(1, 3.14)))
   }
+
+  test("35912: nullability with different schema nullable setting") {
+    val missingFieldInput = """{"c1":1}"""
+    val nullValueInput = """{"c1": 1, "c2": null}"""
+
+    def assertAction(nullable: Boolean, input: String)(action: => Unit): Unit 
= {
+      if (nullable) {
+        action
+      } else {
+        val msg = intercept[IllegalSchemaArgumentException] {
+          action
+        }.message
+        val expected = if (input == missingFieldInput) {
+          "field c2 is not nullable but it's missing in one record."
+        } else {
+          s"field c2 is not nullable but the parsed value is null."
+        }
+        assert(msg.contains(expected))
+      }
+    }
+
+    Seq(true, false).foreach { nullable =>
+      val schema = StructType(Seq(
+        StructField("c1", IntegerType),
+        StructField("c2", IntegerType, nullable = nullable)
+      ))
+      val expected = Seq(InternalRow(1, null))
+      Seq(missingFieldInput, nullValueInput).foreach { input =>
+        assertAction(nullable, input) {
+          check(input = input, schema = schema, filters = Seq.empty, expected 
= expected)
+        }
+      }
+    }
+
+    // filter by not exist field and filter by null value field.
+    Seq(true, false).foreach { nullable =>
+      val schema = StructType(Seq(
+        StructField("c1", IntegerType),
+        StructField("c2", IntegerType, nullable = nullable)
+      ))
+      Seq(missingFieldInput, nullValueInput).foreach { input =>
+        assertAction(nullable, input) {
+          check(input = input, schema = schema, filters = Seq(EqualTo("c2", 
1)),
+            expected = Seq.empty)
+        }
+        assertAction(nullable, input) {
+          check(input = input, schema = schema, filters = Seq(EqualTo("c2", 
0)),
+            expected = Seq.empty)
+        }
+        assertAction(nullable, input) {
+          check(input = input, schema = schema, filters = Seq(IsNotNull("c2")),
+            expected = Seq.empty)
+        }
+        assertAction(nullable, input) {
+          check(input = input, schema = schema, filters = Seq(IsNull("c2")),
+            expected = Seq(InternalRow(1, null)))
+        }
+      }
+    }

Review comment:
       nit: could you move this test part into a new test unit? I feel the 
current test is a bit long.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



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

Reply via email to