gengliangwang commented on code in PR #58632:
URL: https://github.com/apache/spark/pull/58632#discussion_r4077662579
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/types/DataTypeUtils.scala:
##########
@@ -199,7 +202,7 @@ object DataTypeUtils {
case (_: NullType, _) if storeAssignmentPolicy == ANSI => true
case (w: AtomicType, r: AtomicType) if storeAssignmentPolicy == ANSI =>
- if (!Cast.canANSIStoreAssign(w, r)) {
+ if (!Cast.canANSIStoreAssign(w, r) && ansiStoreAssignmentCastCheck ==
AT_ANALYSIS) {
Review Comment:
**[P1] Reject numeric-to-timestamp overflows instead of silently saturating**
`AT_RUNTIME` now admits `LONG`/`DECIMAL -> TIMESTAMP`, but the inserted ANSI
cast is not an overflow check for these pairs. `Cast.castToTimestamp` uses
`TimeUnit.SECONDS.toMicros` for longs, which saturates, and
`BigDecimal.longValue` for decimals, which truncates or wraps.
`TableOutputResolver.canCauseCastOverflow` only wraps targets containing
integral or decimal types, so `TIMESTAMP` is unguarded. A relaxed write can
therefore silently store a different timestamp instead of failing as this mode
promises. Please keep these pairs rejected until the cast is exact, or add an
exact runtime overflow check.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/types/DataTypeUtils.scala:
##########
@@ -213,12 +216,18 @@ object DataTypeUtils {
// If write-side data type is a user-defined type, check with its
underlying data type.
case (w, r) if w.isInstanceOf[UserDefinedType[_]] &&
!r.isInstanceOf[UserDefinedType[_]] =>
canWrite(tableName, w.asInstanceOf[UserDefinedType[_]].sqlType, r,
byName, resolver,
- context, storeAssignmentPolicy, addError)
+ context, storeAssignmentPolicy, addError,
ansiStoreAssignmentCastCheck)
// If read-side data type is a user-defined type, check with its
underlying data type.
case (w, r) if r.isInstanceOf[UserDefinedType[_]] &&
!w.isInstanceOf[UserDefinedType[_]] =>
canWrite(tableName, w, r.asInstanceOf[UserDefinedType[_]].sqlType,
byName, resolver,
- context, storeAssignmentPolicy, addError)
+ context, storeAssignmentPolicy, addError,
ansiStoreAssignmentCastCheck)
+
+ // AT_RUNTIME defers to the inserted ANSI cast, which fails on malformed
or overflowing
+ // values at runtime. Clearly invalid conversion are still rejected
either in an earlier
+ // branch of this match clause, or in checkAnalysis.
+ case (_, _) if storeAssignmentPolicy == ANSI &&
ansiStoreAssignmentCastCheck == AT_RUNTIME =>
Review Comment:
**[P1] Enforce nested nullability for variant-to-complex casts**
This fallback admits top-level `VARIANT -> ARRAY/MAP/STRUCT`, but
`TableOutputResolver` casts the whole value and checks only whether that outer
value is null. `VariantGet.cast` returns null for variant-null elements and
values and leaves absent struct fields null; the `Cast` nevertheless advertises
the target's `containsNull = false`, `valueContainsNull = false`, or
non-nullable fields, so downstream projection trusts those constraints. For
example, a variant object missing `a` can be written to `STRUCT<a: INT NOT
NULL>`, and `[null]` can reach `ARRAY<INT NOT NULL>`, without the required
nested check. Please reject these target shapes or cast through a nullable
shape and apply recursive `AssertNotNull` checks.
--
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]
