MaxGekk commented on code in PR #56677:
URL: https://github.com/apache/spark/pull/56677#discussion_r3462951607
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala:
##########
@@ -158,6 +158,14 @@ object Cast extends QueryErrorsBase {
case (_: TimeType, _: TimeType) => true
case (_: TimeType, _: IntegralType) => true
+ // TIME(p) <-> TIMESTAMP_NTZ(q), q in [6, 9] (precision 6 is the micro
TimestampNTZType,
+ // [7, 9] is TimestampNTZNanosType). Restricted to the NTZ family on
purpose: TIMESTAMP_LTZ
+ // is not a valid counterpart for these casts.
+ case (_: TimeType, TimestampNTZType) => true
Review Comment:
Following up on this — I ended up reverting the four `=> false` arms.
The catch is that `canANSIStoreAssign` is also the gate for
`AnsiTypeCoercion.implicitCast`:
```scala
case (_, target) if !target.isInstanceOf[TypeCollection] =>
val concreteType = target.defaultConcreteType
if (Cast.canANSIStoreAssign(inType, concreteType)) Some(concreteType) else
None
```
So blocking the pair there doesn't just stop the `INSERT` store assignment —
it also removes ANSI implicit coercion of `TIME -> TIMESTAMP_NTZ` and breaks
the existing `SPARK-56152: implicit type cast - TimeType` test, which asserts
`TimeType` coerces to all `datetimeTypes` (incl. `TimestampNTZType`). CI caught
it on `AnsiTypeCoercionSuite` ("Failed to cast TimeType(6) to
TimestampNTZType").
The explicit-only stance is still enforced on the paths that would actually
surprise users: `findWiderDateTimeType` returns `None` for any `TimeType` pair
(so `UNION`/`coalesce`/`CASE` are rejected — covered by the negative-coercion
guard), and `canUpCast` is `false`. Store assignment of `TIME -> TIMESTAMP_NTZ`
follows the same `CURRENT_DATE` semantics as the explicit cast and matches the
pre-existing `TimeType` coercion contract.
Tightening store assignment specifically would mean either changing the
`SPARK-56152` `TimeType` coercion contract (broader than this PR), or splitting
the store-assignment gate from `implicitCast` (a cross-cutting refactor). I'd
prefer to keep store assignment as-is in this PR and handle any tightening
separately if we decide we want it. WDYT?
--
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]
