Use timestamps with timezones #10604
Conversation
It generally appears to be recommended to use `timestamptz` (timestamp with time zone) instead of `timestamp` (timestamp without time zone), unless the saved timestamp explicitly should not have a timezone. Since we always treat these timestamps as UTC though, we should be using `timestamptz` instead. This commit accordingly also switches `NaiveDateTime` for `DateTime<Utc>` in most cases in our codebase. This change also allowed us to remove the `rfc3339` serde helper module, since we no longer need the `DateTime<Utc> -> NaiveDateTime` conversion performed by this module.
Turbo87
added
C-internal 🔧
| .set(api_tokens::last_used_at.eq(now.nullable())) | ||
| .set(api_tokens::last_used_at.eq(now.into_sql::<Timestamptz>().nullable())) |
There was a problem hiding this comment.
this is unfortunately necessary due to a quirk in diesel. see diesel-rs/diesel#1514 (comment).
| @@ -0,0 +1,45 @@ | |||
| SET timezone = 'UTC'; | |||
There was a problem hiding this comment.
this part is important since Postgres would otherwise perform a full table rewrite. if we set the timezone to UTC, Postgres can take a shortcut and doesn't have to rewrite any data.
There was a problem hiding this comment.
Yeah, this is interesting, which I also wanted to point out :)
Here is a SO link1 for reference that provides more details about this.
Footnotes
There was a problem hiding this comment.
migration started on staging at 2025-02-18T08:28:22.618774Z
migration ended on staging at 2025-02-18T08:28:22.653333Z
35ms seems reasonable IMHO :)
eth3lbert
left a comment
eth3lbert
left a comment
There was a problem hiding this comment.
I'm good with this, especially since it won't cause a full table rewrite :D
2 participants
It generally appears to be recommended to use
timestamptz(timestamp with time zone) instead oftimestamp(timestamp without time zone), unless the saved timestamp explicitly should not have a timezone. Since we always treat these timestamps as UTC though, we should be usingtimestamptzinstead.This commit accordingly also switches
NaiveDateTimeforDateTime<Utc>in most cases in our codebase. This change also allowed us to remove therfc3339serde helper module, since we no longer need theDateTime<Utc> -> NaiveDateTimeconversion performed by this module.