cloud-fan commented on a change in pull request #25581: [SPARK-28495][SQL]
Introduce ANSI store assignment policy for table insertion
URL: https://github.com/apache/spark/pull/25581#discussion_r317942895
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeWriteCompatibilitySuite.scala
##########
@@ -297,87 +508,10 @@ class DataTypeWriteCompatibilitySuite extends
SparkFunSuite {
"Should allow map of int written to map of long column")
}
- test("Check types with multiple errors") {
- val readType = StructType(Seq(
- StructField("a", ArrayType(DoubleType, containsNull = false)),
- StructField("arr_of_structs", ArrayType(point2, containsNull = false)),
- StructField("bad_nested_type", ArrayType(StringType)),
- StructField("m", MapType(LongType, FloatType, valueContainsNull =
false)),
- StructField("map_of_structs", MapType(StringType, point3,
valueContainsNull = false)),
- StructField("x", IntegerType, nullable = false),
- StructField("missing1", StringType, nullable = false),
- StructField("missing2", StringType)
- ))
-
- val missingMiddleField = StructType(Seq(
- StructField("x", FloatType, nullable = false),
- StructField("z", FloatType, nullable = false)))
-
- val writeType = StructType(Seq(
- StructField("a", ArrayType(StringType)),
- StructField("arr_of_structs", ArrayType(point3)),
- StructField("bad_nested_type", point3),
- StructField("m", MapType(DoubleType, DoubleType)),
- StructField("map_of_structs", MapType(StringType, missingMiddleField)),
- StructField("y", LongType)
- ))
-
- assertNumErrors(writeType, readType, "top", "Should catch 14 errors", 14)
{ errs =>
- assert(errs(0).contains("'top.a.element'"), "Should identify bad type")
- assert(errs(0).contains("Cannot safely cast"))
- assert(errs(0).contains("StringType to DoubleType"))
-
- assert(errs(1).contains("'top.a'"), "Should identify bad type")
- assert(errs(1).contains("Cannot write nullable elements to array of
non-nulls"))
-
- assert(errs(2).contains("'top.arr_of_structs.element'"), "Should
identify bad type")
- assert(errs(2).contains("'z'"), "Should identify bad field")
- assert(errs(2).contains("Cannot write extra fields to struct"))
-
- assert(errs(3).contains("'top.arr_of_structs'"), "Should identify bad
type")
- assert(errs(3).contains("Cannot write nullable elements to array of
non-nulls"))
-
- assert(errs(4).contains("'top.bad_nested_type'"), "Should identify bad
type")
- assert(errs(4).contains("is incompatible with"))
-
- assert(errs(5).contains("'top.m.key'"), "Should identify bad type")
- assert(errs(5).contains("Cannot safely cast"))
- assert(errs(5).contains("DoubleType to LongType"))
-
- assert(errs(6).contains("'top.m.value'"), "Should identify bad type")
- assert(errs(6).contains("Cannot safely cast"))
- assert(errs(6).contains("DoubleType to FloatType"))
-
- assert(errs(7).contains("'top.m'"), "Should identify bad type")
- assert(errs(7).contains("Cannot write nullable values to map of
non-nulls"))
-
- assert(errs(8).contains("'top.map_of_structs.value'"), "Should identify
bad type")
- assert(errs(8).contains("expected 'y', found 'z'"), "Should detect name
mismatch")
- assert(errs(8).contains("field name does not match"), "Should identify
name problem")
-
- assert(errs(9).contains("'top.map_of_structs.value'"), "Should identify
bad type")
- assert(errs(9).contains("'z'"), "Should identify missing field")
- assert(errs(9).contains("missing fields"), "Should detect missing field")
-
- assert(errs(10).contains("'top.map_of_structs'"), "Should identify bad
type")
- assert(errs(10).contains("Cannot write nullable values to map of
non-nulls"))
-
- assert(errs(11).contains("'top.x'"), "Should identify bad type")
- assert(errs(11).contains("Cannot safely cast"))
- assert(errs(11).contains("LongType to IntegerType"))
-
- assert(errs(12).contains("'top'"), "Should identify bad type")
- assert(errs(12).contains("expected 'x', found 'y'"), "Should detect name
mismatch")
- assert(errs(12).contains("field name does not match"), "Should identify
name problem")
-
- assert(errs(13).contains("'top'"), "Should identify bad type")
- assert(errs(13).contains("'missing1'"), "Should identify missing field")
- assert(errs(13).contains("missing fields"), "Should detect missing
field")
- }
- }
-
// Helper functions
+ protected def storeAssignmentPolicy: StoreAssignmentPolicy.Value
+
def assertAllowed(
Review comment:
it looks to me that, we can make the code simpler if this method takes the
policy parameter. e.g.
```
test("Check atomic types: write allowed only when casting is safe") {
atomicTypes.foreach { w =>
atomicTypes.foreach { r =>
if (Cast.canUpCast(w, r)) {
assertAllowed(Policy.STRICT, ...)
} else {
assertSingleError(Policy.STRICT, ...)
}
if (Cast.canANSIStoreAssign(w, r)) ...
}
}
}
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
