jiangxt2 commented on code in PR #58028:
URL: https://github.com/apache/spark/pull/58028#discussion_r4002689107
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala:
##########
@@ -1022,7 +1022,18 @@ case class Cast(
(d.toBigDecimal * MICROS_PER_SECOND).longValue
}
private[this] def doubleToTimestamp(d: Double): Any = {
Review Comment:
You're right that this is not unique to TIMESTAMP. Float/Double to INT has
the same class of silent-saturation issue, and I agree that these cases should
be fixed as well.
I think this class of issue is worth fixing because an out-of-range input
can become a non-null, valid-looking result, allowing incorrect data to
propagate without an overflow signal.
I am starting with the TIMESTAMP path as a focused first fix, not because it
is unique, but because it is a self-contained conversion family with dedicated
interpreted and codegen paths. The analogous numeric casts use shared generic
branches across multiple source and target types, so I will track them,
together with other timestamp-conversion paths such as timestamp_seconds, in
follow-up JIRAs and PRs for independent review, testing, and backporting.
I recognize that ANSI mode is enabled by default and that changing non-ANSI
behavior has a compatibility cost for existing users. I believe this targeted
correction is still appropriate for the supported non-ANSI path, and I have
documented the user-visible change in the migration guide. If you prefer
preserving the legacy non-ANSI behavior, please let me know and I can revisit
that compatibility decision.
--
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]
