Skip to content

Comments

Use DateTime<Utc> instead of NaiveDateTime#4392

Open
tgeoghegan wants to merge 1 commit intomainfrom
timg/chrono-datetime-utc-everywhere
Open

Use DateTime<Utc> instead of NaiveDateTime#4392
tgeoghegan wants to merge 1 commit intomainfrom
timg/chrono-datetime-utc-everywhere

Conversation

@tgeoghegan
Copy link
Contributor

@tgeoghegan tgeoghegan commented Feb 24, 2026

Replace all usages of NaiveDateTime with DateTime<Utc>, which will mean one less set of conversions we'll need to or from janus_messages::Time.

The scariest thing about this is the implications for the database. Previously we used NaiveDateTime because then we got free postgres_types::{FromSql, ToSql} implementations converting between NaiveDateTime and SQL TIMESTAMP. postgres_types also provides such trait implementations so that we can easily go between DateTime<Utc> and SQL TIMESTAMP WITH TIME ZONE. It turns out to be quite easy to change the schema so that all TIMESTAMP columns are now TIMESTAMP WITH TIME ZONE and all TSRANGEs (timestamp ranges) are now TSTZRANGE (timestamp with timezone range).

Keep in mind also the majority of these TIMESTAMP WITH TIME ZONE and TSTZRANGE columns will go away as we progress through #4206, becoming BIGINT or INT8RANGE to represent ranges of DAP timestamps.

Interestingly, a TIMESTAMP WITH TIME ZONE is no bigger on disk (1, emphasis mine):

For timestamp with time zone values, an input string that includes an
explicit time zone will be converted to UTC (Universal Coordinated
Time) using the appropriate offset for that time zone. If no time zone
is stated in the input string, then it is assumed to be in the time
zone indicated by the system's TimeZone parameter, and is converted
to UTC using the offset for the timezone zone. In either case, the
value is stored internally as UTC, and the originally stated or
assumed time zone is not retained.

The risky thing here is the handling of timestamps without explicit timezone information, but by making sure to use DateTime<Utc> everywhere, we can reasonably guarantee that we never handle times in other time zones (bar a couple of tests where I had to explicitly make timestamp strings UTC by adding "+00").

@tgeoghegan tgeoghegan requested a review from a team as a code owner February 24, 2026 00:05
@tgeoghegan tgeoghegan added the allow-changed-migrations Override the ci-migrations check to allow migrations that have changed. label Feb 24, 2026
@tgeoghegan
Copy link
Contributor Author

While doing this I toyed with changing certain interfaces in Datastore to take chrono::TimeDelta instead of std::time::Duration. I think that's the right move eventually: std::time::Duration is unsigned, which means you do have to think a little bit every time you convert one to the other. But for now, since my objective is to focus on the datastore layer and only venture out into the higher layers when forced to, I don't want to mess too much with things like the aggregation and collection job drivers, so they can hang onto their std::time::Duration fields for now.

Replace all usages of `NaiveDateTime` with `DateTime<Utc>`, which will
mean one less set of conversions we'll need to or from
`janus_messages::Time`.

The scariest thing about this is the implications for the database.
Previously we used `NaiveDateTime` because then we got free
`postgres_types::{FromSql, ToSql}` implementations converting between
`NaiveDateTime` and SQL `TIMESTAMP`. `postgres_types` also provides such
trait implementations so that we can easily go between `DateTime<Utc>`
and SQL `TIMESTAMP WITH TIME ZONE`. It turns out to be quite easy to
change the schema so that all `TIMESTAMP` columns are now `TIMESTAMP
WITH TIME ZONE` and all `TSRANGE`s (timestamp ranges) are now
`TSTZRANGE` (timestamp with timezone range).

Interestingly, a `TIMESTAMP WITH TIME ZONE` is no bigger on disk ([1],
emphasis mine):

> For timestamp with time zone values, an input string that includes an
> explicit time zone will be converted to UTC (Universal Coordinated
> Time) using the appropriate offset for that time zone. If no time zone
> is stated in the input string, then it is assumed to be in the time
> zone indicated by the system's TimeZone parameter, and is converted
> to UTC using the offset for the timezone zone. *In either case, the
> value is stored internally as UTC, and the originally stated or
> assumed time zone is not retained.*

The risky thing here is the handling of timestamps without explicit
timezone information, but by making sure to use `DateTime<Utc>`
everywhere, we can reasonably guarantee that we never handle times in
other time zones (bar a couple of tests where I had to explicitly make
timestamp strings UTC by adding "+00").

[1]: https://www.postgresql.org/docs/current/datatype-datetime.html#DATATYPE-DATETIME-INPUT-TIME-STAMPS
@tgeoghegan tgeoghegan force-pushed the timg/chrono-datetime-utc-everywhere branch from 2b69d5d to 2289df2 Compare February 24, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

allow-changed-migrations Override the ci-migrations check to allow migrations that have changed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant