ReviewOS

also looking at this

stacks/bun-query-builder

docs(mysql): say that timestamptz is naive there, and warn once

#1180
Merged glennmichael123 wants to merge docs/mysql-timestamptz-warning into main
2 files +60 -1
8 days ago

Refs #1179. The documentation-and-warning option, not a change to the mapping.

A model declaring timestamptz gets a genuinely aware column on Postgres and a naive DATETIME on MySQL, and nothing said so.

declaredPostgresMySQL
datetimetimestamp (naive)DATETIME (naive)
timestamptztimestamptz (aware)DATETIME (naive)

Why warn rather than fix

The mapping is not wrong. MySQL's only aware type is TIMESTAMP, and on live 8.4.5 it will not hold a far-future date:

INSERT ... DATETIME  '2045-06-01 00:00:00'  -> stored
INSERT ... TIMESTAMP '2045-06-01 00:00:00'  -> ERROR 1292 Incorrect datetime value

timestamptz columns tend to be exactly the far-future ones (expires_at, period ends), so DATETIME is the safer storage. Mapping to TIMESTAMP would trade a semantic gap for a Y2038 ceiling on the columns most likely to exceed it.

Refusing the declaration would also be wrong: it is reasonable, and the storage is sound. Values round-trip, because temporalLiteral writes UTC wall-clock digits and MySQL returns them unconverted. What is lost is the server enforcing it — the column is an instant by convention, and the convention breaks for a raw NOW(), another service, or a dump restored from a timezone-aware source.

So: make the loss visible, leave the behaviour alone.

What this adds

  • A warning at DDL time, once per column per process, modelled on warnOnceBareSqlFragment. Only timestamptz triggers it; datetime is silent.
  • A ### timestamptz is not timezone-aware on MySQL section under Timezones and Types in docs/advanced/dialects.md.

Both record the two consequences I measured:

  • Regenerating model source from a MySQL database rewrites the attribute to datetime. It does not cause migration churn — canonicalStorageType maps both declarations to DATETIME, so a reconcile compares equal. (I had this wrong in #1179 initially and corrected it there.)
  • auditSchemaDrift has to excuse a timestamptz model over a live DATETIME, since that is what a correct migration produces. It still flags the dangerous direction, a live MySQL TIMESTAMP under a naive model — #1178.

Verification

timestamptz -> datetime      [warns]
timestamptz -> datetime      [same column again, silent]
datetime    -> datetime      [silent]

Full suite 2447 pass, 26 skip, 0 fail; tsc and pickier clean.

🤖 Generated with Claude Code

2 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.