Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daylight savings time doesn't appear to be taken into account after the year 2099 #155

Closed
ethan-readyset opened this issue Mar 29, 2024 · 6 comments

Comments

@ethan-readyset
Copy link

Background
When testing with the America/New_York time zone, I'm seeing some surprising results when it comes to daylight savings times. Since 2007 in the United States, daylight savings time begins on the second Sunday in March at 02:00 local time. My testing shows that, for the years between 2007 and 2099 (inclusive), the following passes:

let date = chrono::NaiveDate::from_ymd_opt(YEAR, 3, SECOND_SUNDAY).unwrap();
let naive_dt = NaiveDateTime::new(date, time);
assert!(TIMEZONE.from_local_datetime(&naive_dt).single().is_none());

Adding one day to the original naive_dt produces a timestamp with timezone EDT, which is the America/New_York timezone when daylight savings is active.

However, beginning with year 2100 for datetimes on the second Sunday in March at 02:00, TIMEZONE.from_local_datetime(&naive_dt).single() returns Some and produces a datetime with the timezone EST, which is the America/New_York timezone when daylight savings is not active. Timestamps after this one but before the end of daylight savings within the same year continue to have EST timezones, even though I'd expect them to be EDT.

Is this a known limitation of chrono-tz, or am I just misunderstanding how daylight savings time should work?

Minimal Reproducible Example

use chrono::{DateTime, Days, Months, NaiveDateTime, NaiveTime, TimeZone};
use chrono_tz::{OffsetComponents, Tz};

// Daylight savings always starts exactly at 02:00 local time in most time zones in the United
// States
const TIMEZONE: Tz = Tz::America__New_York;

fn main() {
    let time = NaiveTime::from_hms_opt(2, 0, 0).unwrap();

    // 2099-03-08 is the second Sunday of the month, so this time is not representable
    let date = chrono::NaiveDate::from_ymd_opt(2099, 3, 8).unwrap();
    let naive_dt = NaiveDateTime::new(date, time);
    assert!(TIMEZONE.from_local_datetime(&naive_dt).single().is_none());

    // Adding a day produces a timestamp with EDT and DST offset of 1 hour, which is correct
    let naive_dt = NaiveDateTime::new(date.succ_opt().unwrap(), time);
    let next_day = TIMEZONE.from_local_datetime(&naive_dt).single().unwrap();
    assert_timezone_and_offset(&next_day, "EDT", 3600);

    // 2100-03-14 is the second Sunday and thus the start of DST, so this should be None as it is
    // above. However, it's not, and we can unwrap() it
    let date = chrono::NaiveDate::from_ymd_opt(2100, 3, 14).unwrap();
    let naive_dt = NaiveDateTime::new(date, time);
    let dt = TIMEZONE.from_local_datetime(&naive_dt).single().unwrap();
    // The timezone is EST and the DST offset is zero
    assert_timezone_and_offset(&dt, "EST", 0);

    // Adding a day to the date still produces a timestamp with EST and no DST offset
    let next_day = dt.checked_add_days(Days::new(1)).unwrap();
    assert_timezone_and_offset(&next_day, "EST", 0);

    // Adding a month to the date still produces a timestamp with EST and no DST offset, even
    // though we should be well into EDT by now
    let next_month = dt.checked_add_months(Months::new(1)).unwrap();
    assert_timezone_and_offset(&next_month, "EST", 0);
}

fn assert_timezone_and_offset(dt: &DateTime<Tz>, tz: &str, offset: i64) {
    assert_eq!(dt.format("%Z").to_string(), tz);
    assert_eq!(dt.offset().dst_offset().num_seconds(), offset);
}

Versions
rustc: 1.70.0-nightly
chrono: 0.4.37
chrono-tz: 0.8.6

@ethan-readyset
Copy link
Author

@djc
Copy link
Member

djc commented Mar 29, 2024

Ugh, sorry for that -- we should probably reconsider that choice.

@westy92
Copy link
Contributor

westy92 commented Mar 30, 2024

This is likely the cause of: #135.

@djc
Copy link
Member

djc commented Apr 4, 2024

Maybe we should expose some API from chrono (which has a parser internally) so we can use that?

@pitdicker
Copy link
Contributor

This are two things that I hope to work on, but there are plenty of things to do first for the coming months.
Chrono already has most of the code to use the system time zone database, we just don't expose it yet.

And I've got the basics down for a rewrite of this crate that uses rules instead of a large table of transition dates. That would reduce the size added to the binary by ca. 90% and allow us to handle dates without an arbitrary cutoff point such as 2099.

@pitdicker
Copy link
Contributor

Closing as a duplicate of #135. Thank you @westy92 for investigating!

@pitdicker pitdicker closed this as not planned Won't fix, can't repro, duplicate, stale Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants