weiting-chen commented on code in PR #13248:
URL: https://github.com/apache/gluten/pull/13248#discussion_r4271688294
##########
gluten-ut/spark42/src/test/scala/org/apache/spark/sql/sources/GlutenInsertSuite.scala:
##########
@@ -591,6 +593,181 @@ class GlutenInsertSuite
}
}
}
+
+ // Spark's SPARK-55716 tests expect a
SparkRuntimeException(NOT_NULL_ASSERT_VIOLATION).
+ // Velox assert_not_null throws a VeloxUserError instead, which fails the
query with a
+ // SparkException.
+ private def assertNotNullViolation(f: => Unit): Unit = {
+ val exception = intercept[SparkException](f)
+ val messages = Iterator
+ .iterate[Throwable](exception)(_.getCause)
+ .takeWhile(_ != null)
+ .flatMap(e => Option(e.getMessage))
+
+ assert(messages.exists(_.contains("Null value appeared in non-nullable
field")))
+ }
+
+ testGluten("SPARK-55716: V1 INSERT rejects null into NOT NULL column for
file sources") {
+ Seq("parquet", "orc", "json").foreach {
+ format =>
+ withSQLConf(SQLConf.FILE_SOURCE_INSERT_ENFORCE_NOT_NULL.key -> "true")
{
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT NOT NULL, s STRING NOT NULL) USING
$format")
+ assertNotNullViolation {
+ sql("INSERT INTO t VALUES(null, 'a')")
+ }
+ assertNotNullViolation {
+ sql("INSERT INTO t VALUES(1, null)")
+ }
+ sql("INSERT INTO t VALUES(1, 'a')")
+ checkAnswer(spark.table("t"), Seq(Row(1, "a")))
+ }
+ }
+ }
+ }
+
+ testGluten("SPARK-55716: V1 INSERT NOT NULL enforcement respects
storeAssignmentPolicy") {
+ Seq("parquet", "orc").foreach {
+ format =>
+ // ANSI mode (default): rejects null
+ withSQLConf(
+ SQLConf.FILE_SOURCE_INSERT_ENFORCE_NOT_NULL.key -> "true",
+ SQLConf.STORE_ASSIGNMENT_POLICY.key ->
SQLConf.StoreAssignmentPolicy.ANSI.toString) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT NOT NULL) USING $format")
+ assertNotNullViolation {
+ sql("INSERT INTO t VALUES(null)")
+ }
+ }
+ }
+ // STRICT mode: also rejects null (fails at analysis with type
mismatch)
+ withSQLConf(
+ SQLConf.FILE_SOURCE_INSERT_ENFORCE_NOT_NULL.key -> "true",
+ SQLConf.STORE_ASSIGNMENT_POLICY.key ->
SQLConf.StoreAssignmentPolicy.STRICT.toString) {
+ withTable("t") {
+ sql(s"CREATE TABLE t(i INT NOT NULL) USING $format")
+ intercept[AnalysisException] {
+ sql("INSERT INTO t VALUES(null)")
+ }
+ }
+ }
+ // LEGACY mode: allows null (no AssertNotNull injected)
+ withSQLConf(
+ SQLConf.STORE_ASSIGNMENT_POLICY.key ->
SQLConf.StoreAssignmentPolicy.LEGACY.toString) {
Review Comment:
**Optional: keep NOT NULL enforcement enabled when testing the LEGACY
exemption**
**Target Location:**
`gluten-ut/spark42/src/test/scala/org/apache/spark/sql/sources/GlutenInsertSuite.scala:654-662`.
**Problem:** The LEGACY branch changes only `storeAssignmentPolicy`. The
previous `withSQLConf` scopes have already restored
`FILE_SOURCE_INSERT_ENFORCE_NOT_NULL` to its default `false`, so this branch
admits null because enforcement is off regardless of whether the LEGACY
exemption works. It therefore does not isolate the policy behavior named by the
test. This is a **nonblocking gap inherited from the upstream test**, not a
current enforcement defect. Confidence: high.
**Evidence:**
```scala
// LEGACY mode: allows null (no AssertNotNull injected)
withSQLConf(
SQLConf.STORE_ASSIGNMENT_POLICY.key ->
SQLConf.StoreAssignmentPolicy.LEGACY.toString) {
```
The adjacent ANSI and STRICT branches explicitly enable the enforcement
gate; this branch does not. The later gate-disabled branch already covers
admitting null with enforcement off.
**Suggested Fix:**
```scala
withSQLConf(
SQLConf.FILE_SOURCE_INSERT_ENFORCE_NOT_NULL.key -> "true",
SQLConf.STORE_ASSIGNMENT_POLICY.key ->
SQLConf.StoreAssignmentPolicy.LEGACY.toString) {
withTable("t") {
sql(s"CREATE TABLE t(i INT NOT NULL) USING $format")
sql("INSERT INTO t VALUES(null)")
checkAnswer(spark.table("t"), Seq(Row(null)))
}
}
```
This tests `enforcement=true + LEGACY` against the existing `true +
ANSI/STRICT` rejection cases. In the separate gate-disabled branch, setting the
policy explicitly to ANSI would also make its negative control independent of
ambient policy. Rerun this policy test for both file formats after the change.
--
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]
