MaxGekk commented on code in PR #56888:
URL: https://github.com/apache/spark/pull/56888#discussion_r3497458589
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala:
##########
@@ -689,7 +689,11 @@ case class Cast(
override protected def withNewChildInternal(newChild: Expression): Cast =
copy(child = newChild)
- final override def nodePatternsInternal(): Seq[TreePattern] = Seq(CAST)
+ final override def nodePatternsInternal(): Seq[TreePattern] = dataType match
{
+ case _: TimestampNTZType | _: TimestampNTZNanosType |
Review Comment:
Non-blocking (maintainability): `CAST_TO_TIMESTAMP` must stay set on a
*superset* of the targets that `Cast.isTimeToTimestampNTZ` /
`isTimeToTimestampLTZ` accept — otherwise `ComputeCurrentTime` silently stops
reaching a rewrite it should perform (a `CURRENT_DATE`-derived TIME->TIMESTAMP
left un-stabilized), and no current test would catch the drift since they're
two independent match lists. Consider a short comment here cross-referencing
those guards (and the rule).
Worth also recording *why this keys on the target `dataType` and not
`child.dataType`*: node patterns are computed eagerly at construction before
the child is resolved, so reading `child.dataType` throws (the `OuterReference`
/ `CREATE FUNCTION ... RETURNS TABLE` case removed in 51136ecb5e2). A future
attempt to narrow the bit by also matching the source type would reintroduce
that break — the target type is the only safe key.
--
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]
