rich7420 commented on code in PR #5735:
URL: https://github.com/apache/datafusion-comet/pull/5735#discussion_r3950323795
##########
native/spark-expr/src/conversion_funcs/temporal.rs:
##########
@@ -40,49 +42,46 @@ pub(crate) fn cast_date_to_timestamp(
cast_options: &SparkCastOptions,
target_tz: &Option<Arc<str>>,
) -> SparkResult<ArrayRef> {
+ if target_tz.is_none() {
+ // TIMESTAMP_NTZ ignores session TZ. Like Spark's daysToMicros, reject
overflow.
+ return Ok(cast_with_options(
+ array_ref.as_ref(),
+ &DataType::Timestamp(TimeUnit::Microsecond, None),
+ &DEFAULT_CAST_OPTIONS,
Review Comment:
Could we use `safe: true` for `EvalMode::Try`, while keeping Legacy and ANSI
strict?
For day count `106751992`, Spark 4.1.3 `TRY_CAST` returns NULL, but this
head's `spark_cast` returns `ArithmeticOverflow`. I verified these separately
through Spark SQL and the native entry point.
Could you add a TRY-mode test mixing valid, overflowing and null dates?
--
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]
