ReviewOS

stacks/bun-query-builder

timestamptz is indistinguishable from datetime on MySQL, so it does not round-trip and the drift auditor must excuse it

#1179
Closed glennmichael123 opened this 8 days ago ·
8 days ago

drivers/mysql.ts:56 maps a declared timestamptz to DATETIME, the same type datetime maps to. The two declarations are indistinguishable once a MySQL schema exists.

Split out from #1176, where it forced a carve-out in the drift auditor (#1178).

First, what this is NOT

I expected to find silent data corruption and measured instead that there is none for data bqb writes. temporalLiteral (client.ts:368) sends a Date to MySQL as toISOString() digits — UTC wall-clock — and a DATETIME returns them unconverted. UTC in, UTC out, self-consistent.

It is also not an obviously wrong mapping. MySQL's aware type is TIMESTAMP, and on live MySQL 8.4.5 it will not hold a date past 2038:

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

expires_at is exactly the column that wants a far-future date, so "just map timestamptz to TIMESTAMP" trades a semantic gap for a Y2038 range limit. The current mapping looks like a deliberate choice.

The behavioural difference it gives up, same server, same row:

SET time_zone = '+00:00';  INSERT '2026-01-01 12:00:00' into both
read at +00:00   DATETIME 2026-01-01 12:00:00   TIMESTAMP 2026-01-01 12:00:00
read at +08:00   DATETIME 2026-01-01 12:00:00   TIMESTAMP 2026-01-01 20:00:00

What the actual costs are

1. It does not round-trip. Postgres keeps the declaration, MySQL loses it:

declaredcolumnintrospects as
Postgrestimestamptztimestamptztimestamptz
MySQLtimestamptzDATETIMEdatetime

Verified via sqlTypeToNormalized:

datetime                   -> datetime
timestamp                  -> datetime
timestamp with time zone   -> timestamptz
timestamptz                -> timestamptz

So reconcile-from-database rewrites a timestamptz model to datetime on MySQL. That is the same shape as the bug the comment at introspect-db.ts:221-226 says was fixed for Postgres — where collapsing the two made every reconcile propose a destructive type change for every such column, on every run.

2. It forced a carve-out in the drift auditor. #1178 has to excuse declared-timestamptz-over-live-DATETIME on MySQL, because that pairing is what a correct migration produces. The exclusion is sound today only because bqb never emits MySQL TIMESTAMP; it is a rule that exists to paper over this mapping.

3. Nothing says so. A model declaring timestamptz gets Postgres semantics on Postgres and naive-UTC-by-convention on MySQL, with no warning, no documentation, and no error. The convention holds only while every writer is bqb — a raw NOW(), another service, or a dump restored from a tz-aware source all break it, and the auditor cannot see it.

Options

Roughly in order of how much I would trust them:

  1. Document the mapping and leave it. Cheapest, keeps the 2038 headroom, fixes nothing about round-tripping.
  2. Make the loss explicit — warn (or refuse) when a model declares timestamptz under a MySQL-family dialect, so the semantic gap is a decision rather than a surprise.
  3. Keep the declaration round-trippable so reconcile stops flipping the model. The information has to live somewhere MySQL preserves; a column comment is the usual trick and is introspectable.
  4. Map to TIMESTAMP. Real aware semantics, and a 2038 ceiling on exactly the columns most likely to exceed it. I would not do this without opt-in.

Happy to implement whichever you prefer. I have not touched the mapping.

Verified on

MySQL 8.4.5, live. A throwaway database was created for the measurements and dropped afterwards.

8 days ago

Correcting cost #1 above — I overstated it.

I implied this reproduces the Postgres churn described at introspect-db.ts:221-226. It does not. canonicalStorageType collapses both declarations to the same MySQL type, so reconcile compares equal and proposes nothing:

mysql     timestamptz -> DATETIME     | datetime -> DATETIME      | equal: true
postgres  timestamptz -> timestamptz  | datetime -> datetime      | equal: false

That is exactly why Postgres churned and MySQL does not: there, the two sides differ and every run proposed a destructive change; here they are the same string.

So the round-trip loss is narrower than I wrote. It shows up when model source is regenerated from the database (generateModelSource), where a timestamptz attribute comes back as datetime — a silent edit to a checked-in file, not repeated migration churn.

That lowers the severity enough that I would now drop options 3 and 4 and treat this as documentation plus an explicit warning.

8 days ago

Closing as resolved-by-documentation. #1180 merged (dcd8e26b): docs/advanced/dialects.md now states the trade, and declaring timestamptz under a MySQL dialect warns once per column at DDL time.

Not changing the mapping, for the reasons that came out of measuring it:

  • DATETIME is the correct default. TIMESTAMP rejecting 2045-06-01 is it behaving as specified — 2038-01-19 against DATETIME's 9999 — and timestamptz columns are disproportionately the far-future ones.
  • What is given up is the server enforcing the instant, not the values. Everything this library writes is UTC wall-clock via temporalLiteral, and MySQL returns it unconverted.
  • A TIMESTAMP opt-in would multiply the cases #1178's dialect-threaded classifier has to get right, on the dialect where awareness is hardest to reason about, for a feature with no current consumer.

The two costs are recorded rather than fixed: regenerating model source from a MySQL database rewrites the attribute to datetime (no migration churn — both map to the same storage type), and auditSchemaDrift excuses that pairing while still flagging the dangerous direction.

Reopen if someone needs real aware semantics on MySQL and their dates stay before 2038. The documented trade is the placeholder for that conversation.

Sign in to comment on this issue.