sunchao commented on code in PR #7219:
URL: https://github.com/apache/datafusion-comet/pull/7219#discussion_r4395374736
##########
native/spark-expr/src/utils.rs:
##########
@@ -427,6 +464,55 @@ mod tests {
);
}
+ fn ntz_micros(values: Vec<i64>) -> ArrayRef {
+ Arc::new(TimestampMicrosecondArray::from(values))
+ }
+
+ #[test]
+ fn test_timestamp_ntz_to_timestamp_handles_values_beyond_chrono_range() {
+ // Spark converts the whole i64 range; chrono stops at years -262144
and 262143.
+ let output =
+ timestamp_ntz_to_timestamp(ntz_micros(vec![i64::MAX, i64::MIN,
0]), "UTC", None)
+ .unwrap();
+ let output = as_primitive_array::<TimestampMicrosecondType>(&output);
+ assert_eq!(output.values(), &[i64::MAX, i64::MIN, 0]);
+
+ // A fixed offset is applied unchanged: +02:00 local is two hours
earlier in UTC.
+ let two_hours = 2 * 3_600 * 1_000_000;
+ let output =
+ timestamp_ntz_to_timestamp(ntz_micros(vec![i64::MAX]), "+02:00",
None).unwrap();
+ assert_eq!(
+ as_primitive_array::<TimestampMicrosecondType>(&output).value(0),
+ i64::MAX - two_hours
+ );
+ }
+
+ #[test]
+ fn
test_timestamp_ntz_to_timestamp_beyond_chrono_range_matches_shifted_offset() {
Review Comment:
I reproduced the July case and think the newly accepted input range needs a
guard before this lands. For `TIMESTAMP_NTZ '+280000-07-15 12:00:00'` in
`America/Los_Angeles`, the input is `8773796318400000000` local microseconds.
Spark's `DateTimeUtils.convertTz` returns `8773796343600000000` using `-07:00`.
The head's conversion returns `8773796347200000000` using `-08:00`, one hour
late. The base conversion returns the chrono-range error for this input.
I agree that the underlying DST limitation predates this PR. The change here
is that this new branch now succeeds with the wrong instant for inputs that
previously failed. Moving the lookup to year `+242000` keeps it beyond
chrono-tz's recurring-transition horizon. Could we use a correct offset lookup
or Spark fallback for these newly accepted cases, or limit the extension to
cases where parity is established, and restore the omitted July row? The older
in-range timezone issue can remain separate.
Validation was an isolated harness containing the unmodified head/base
conversion functions, with the lockfile's chrono `0.4.45` and chrono-tz
`0.10.4`, plus a direct Spark `3.5.9` `DateTimeUtils.convertTz` call. The
harness used cached Arrow `59.2.0-openai` rather than this branch's `59.3.1`.
This is not a full Spark/Comet SQL run.
--
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]
