fix: handle ambiguous and non-existent local times#3865
fix: handle ambiguous and non-existent local times#3865matthewalex4 wants to merge 6 commits intoapache:mainfrom
Conversation
matthewalex4
changed the title
| LocalResult::Ambiguous(dt, _) => dt, | ||
| LocalResult::None => { | ||
| // Interpret nonexistent local time by shifting from one hour earlier. | ||
| let shift = TimeDelta::hours(1); |
There was a problem hiding this comment.
The 1-hour shift works for standard DST transitions, but some timezones have non-standard gaps (e.g., Australia/Lord_Howe has a 30-minute DST transition). Yes, this is an edge case so it may be fine to ignore, but we should at least document this as an incompatibility.
https://www.timeanddate.com/time/change/australia/lord-howe-island
There was a problem hiding this comment.
I'm handling the DST gap in #3884. Perhaps you can wait for this to pass CI and then re-use.
native/spark-expr/src/utils.rs
Outdated
|
|
||
| #[test] | ||
| fn test_timestamp_ntz_to_timestamp_handles_non_existent_time() { | ||
| let result = std::panic::catch_unwind(|| { |
There was a problem hiding this comment.
do these tests still need catch_unwind?
|
Thanks @matthewalex4. Could you also add an end-to-end Spark test, perhaps in |
Yes, have added a test in 998f975 |
3 participants
Which issue does this PR close?
Closes #3864.
Rationale for this change
timestamp_ntz_to_timestamppanics when a local time is ambiguous or non-existent due to DST transitions. Further explanation in issue description.What changes are included in this PR?
from_local_datetime(...)is handled for cases of ambiguous and non-existent local times.How are these changes tested?
Unit tests provided for both ambiguous and non-existent local times.