also looking at this
fix(drift): give timezone awareness its own type family
#1178
2 files
+135
-7
| @@ -55,7 +55,7 @@ export interface SchemaDrift { | ||
| 55 | 55 | * the ones that are. What it does separate is the pairs that silently lose |
| 56 | 56 | * data — text from bounded varchar, and fractional from whole. |
| 57 | 57 | */ |
| 58 | export type TypeFamily = 'text' | 'varchar' | 'integer' | 'fractional' | 'boolean' | 'date' | 'json' | 'binary' | 'other' | |
| 58 | export type TypeFamily = 'text' | 'varchar' | 'integer' | 'fractional' | 'boolean' | 'date' | 'timestamptz' | 'json' | 'binary' | 'other' | |
| 59 | 59 | |
| 60 | 60 | /** The family a model's declared type belongs to. */ |
| 61 | 61 | export function familyOfDeclared(type: NormalizedColumnType): TypeFamily { |
| @@ -76,8 +76,11 @@ export function familyOfDeclared(type: NormalizedColumnType): TypeFamily { | ||
| 76 | 76 | return 'boolean' |
| 77 | 77 | case 'date': |
| 78 | 78 | case 'datetime': |
| 79 | case 'timestamptz': | |
| 80 | 79 | return 'date' |
| 80 | // Awareness is its own family: a naive value written into an aware column | |
| 81 | // is read back as a different instant, which no widening rule covers. | |
| 82 | case 'timestamptz': | |
| 83 | return 'timestamptz' | |
| 81 | 84 | case 'json': |
| 82 | 85 | return 'json' |
| 83 | 86 | default: |
| @@ -85,8 +88,18 @@ export function familyOfDeclared(type: NormalizedColumnType): TypeFamily { | ||
| 85 | 88 | } |
| 86 | 89 | } |
| 87 | 90 | |
| 88 | /** The family a raw SQL type reported by a database belongs to. */ | |
| 89 | export function familyOfSqlType(sqlType: string): TypeFamily { | |
| 91 | /** | |
| 92 | * The family a raw SQL type reported by a database belongs to. | |
| 93 | * | |
| 94 | * `dialect` matters only for timestamps, and only for the bare word | |
| 95 | * `timestamp`, whose awareness is not a property of the spelling: Postgres | |
| 96 | * `timestamp` is naive, while MySQL's `TIMESTAMP` is converted to and from the | |
| 97 | * session timezone on every read and write and `DATETIME` is the naive one. So | |
| 98 | * the same four letters mean opposite things, and classifying them without | |
| 99 | * knowing the server gets one dialect wrong whichever way it guesses. Omitted, | |
| 100 | * it reads `timestamp` as naive, which is correct everywhere but MySQL. | |
| 101 | */ | |
| 102 | export function familyOfSqlType(sqlType: string, dialect?: SupportedDialect): TypeFamily { | |
| 90 | 103 | const type = String(sqlType ?? '').toLowerCase().trim() |
| 91 | 104 | |
| 92 | 105 | // Order matters. `tinyint(1)` is MySQL's boolean and must not read as an |
| @@ -100,6 +113,22 @@ export function familyOfSqlType(sqlType: string): TypeFamily { | ||
| 100 | 113 | if (/^(?:bytea|blob|binary|varbinary)\b/.test(type)) |
| 101 | 114 | return 'binary' |
| 102 | 115 | |
| 116 | // Before the generic date branch, which would otherwise swallow these. | |
| 117 | // | |
| 118 | // Tested against the whole string rather than a leading word, matching | |
| 119 | // `normalizePlanType` in introspect-db: Postgres reports an aware column as | |
| 120 | // `timestamp with time zone`, and `timestamp(6) with time zone` when it | |
| 121 | // carries a precision. The bare `timestamptz` spelling needs its own test | |
| 122 | // because `\b` cannot match between `timestamp` and `tz`, so the generic | |
| 123 | // branch misses it and it used to fall through to `other` - where the | |
| 124 | // unknown-is-not-drift rule passed it silently. | |
| 125 | if (/\bwith time zone\b/.test(type) || /^timestamptz\b/.test(type)) | |
| 126 | return 'timestamptz' | |
| 127 | ||
| 128 | // MySQL's aware type. `datetime` is its naive one and falls through below. | |
| 129 | if (isMysqlLike(dialect) && /^timestamp\b/.test(type)) | |
| 130 | return 'timestamptz' | |
| 131 | ||
| 103 | 132 | if (/^(?:date|time|timestamp|datetime)\b/.test(type)) |
| 104 | 133 | return 'date' |
| 105 | 134 | |
| @@ -141,7 +170,13 @@ export function satisfies(declared: TypeFamily, actual: TypeFamily, dialect?: Su | ||
| 141 | 170 | if (dialect === 'sqlite') { |
| 142 | 171 | if (declared === 'boolean' && actual === 'integer') |
| 143 | 172 | return true |
| 144 | if ((declared === 'date' || declared === 'json') && actual === 'text') | |
| 173 | if ((declared === 'date' || declared === 'timestamptz' || declared === 'json') && actual === 'text') | |
| 174 | return true | |
| 175 | // SQLite has no timezone-aware type and performs no session conversion, so | |
| 176 | // the distinction the `timestamptz` family exists to catch cannot arise | |
| 177 | // here. The driver stores every date family as TEXT; a column declared | |
| 178 | // `timestamptz` by some other tool is still just text. | |
| 179 | if ((declared === 'date' && actual === 'timestamptz') || (declared === 'timestamptz' && actual === 'date')) | |
| 145 | 180 | return true |
| 146 | 181 | } |
| 147 | 182 | |
| @@ -155,6 +190,21 @@ export function satisfies(declared: TypeFamily, actual: TypeFamily, dialect?: Su | ||
| 155 | 190 | if (isMysqlLike(dialect) && declared === 'boolean' && actual === 'integer') |
| 156 | 191 | return true |
| 157 | 192 | |
| 193 | /* | |
| 194 | * MySQL has no aware type a model can ask for: the driver maps BOTH | |
| 195 | * `datetime` and `timestamptz` to `DATETIME` (drivers/mysql.ts), so a model | |
| 196 | * declaring `timestamptz` gets a naive column from a correct migration. If | |
| 197 | * that read as drift, every MySQL schema bqb itself created would be dirty - | |
| 198 | * the same false positive as the boolean rule above. | |
| 199 | * | |
| 200 | * The opposite direction is NOT excused. A live MySQL `TIMESTAMP` under a | |
| 201 | * naive model is session-converted on every read, and bqb never emits one, | |
| 202 | * so it can only have come from elsewhere. That is real drift and the | |
| 203 | * dangerous half. | |
| 204 | */ | |
| 205 | if (isMysqlLike(dialect) && declared === 'timestamptz' && actual === 'date') | |
| 206 | return true | |
| 207 | ||
| 158 | 208 | // A bounded string fits in an unbounded one. |
| 159 | 209 | if (declared === 'varchar' && actual === 'text') |
| 160 | 210 | return true |
| @@ -212,7 +262,7 @@ export async function auditSchemaDrift( | ||
| 212 | 262 | continue |
| 213 | 263 | } |
| 214 | 264 | |
| 215 | const actual = familyOfSqlType(liveColumn.sqlType) | |
| 265 | const actual = familyOfSqlType(liveColumn.sqlType, dialect) | |
| 216 | 266 | |
| 217 | 267 | if (!satisfies(declared, actual, dialect)) { |
| 218 | 268 | typeMismatches.push({ |
| @@ -47,7 +47,8 @@ describe('familyOfSqlType', () => { | ||
| 47 | 47 | ['boolean', 'boolean'], |
| 48 | 48 | ['bool', 'boolean'], |
| 49 | 49 | ['jsonb', 'json'], |
| 50 | ['timestamp with time zone', 'date'], | |
| 50 | ['timestamp with time zone', 'timestamptz'], | |
| 51 | ['timestamp without time zone', 'date'], | |
| 51 | 52 | ['date', 'date'], |
| 52 | 53 | ['bytea', 'binary'], |
| 53 | 54 | ])('reads %s as %s', (sqlType, expected) => { |
| @@ -185,3 +186,80 @@ describe('formatSchemaDrift', () => { | ||
| 185 | 186 | expect(report.split('\n').length).toBeLessThan(25) |
| 186 | 187 | }) |
| 187 | 188 | }) |
| 189 | ||
| 190 | /** | |
| 191 | * Naive-vs-aware timestamps — stacksjs/bun-query-builder#1176. | |
| 192 | * | |
| 193 | * Both families used to collapse into `date`, so a live `timestamptz` under a | |
| 194 | * model declaring `datetime` reported clean in both directions, on every | |
| 195 | * dialect. It is the one drift class that silently changes every value the | |
| 196 | * application writes: a naive UTC literal stored in an aware column is read | |
| 197 | * back shifted by the session offset. Three Stacks applications ran 8 hours | |
| 198 | * off with the auditor calling those schemas clean on every migrate. | |
| 199 | */ | |
| 200 | describe('timestamp awareness (#1176)', () => { | |
| 201 | it('separates the two families at the declaration', () => { | |
| 202 | expect(familyOfDeclared('timestamptz')).toBe('timestamptz') | |
| 203 | expect(familyOfDeclared('datetime')).toBe('date') | |
| 204 | expect(familyOfDeclared('date')).toBe('date') | |
| 205 | }) | |
| 206 | ||
| 207 | it('reads every spelling Postgres and MySQL report', () => { | |
| 208 | // Measured against live servers, not assumed: Postgres information_schema | |
| 209 | // says `timestamp with/without time zone`, MySQL says `timestamp`/`datetime`. | |
| 210 | expect(familyOfSqlType('timestamp with time zone')).toBe('timestamptz') | |
| 211 | expect(familyOfSqlType('timestamp(6) with time zone')).toBe('timestamptz') | |
| 212 | expect(familyOfSqlType('timestamp without time zone')).toBe('date') | |
| 213 | expect(familyOfSqlType('timestamp')).toBe('date') | |
| 214 | expect(familyOfSqlType('datetime')).toBe('date') | |
| 215 | }) | |
| 216 | ||
| 217 | it('reads the bare timestamptz spelling, which `\\b` cannot reach', () => { | |
| 218 | // `\b` matches nothing between `timestamp` and `tz`, so the generic date | |
| 219 | // branch misses this and it used to fall through to `other` — where the | |
| 220 | // unknown-is-not-drift rule passed it without comment. SQLite returns the | |
| 221 | // declared text from PRAGMA, so this is the spelling it reports. | |
| 222 | expect(familyOfSqlType('timestamptz')).toBe('timestamptz') | |
| 223 | }) | |
| 224 | ||
| 225 | it('reads a bare MySQL TIMESTAMP as aware, unlike Postgres', () => { | |
| 226 | // The same four letters mean opposite things. MySQL converts TIMESTAMP to | |
| 227 | // and from the session timezone; DATETIME is its naive type. | |
| 228 | expect(familyOfSqlType('timestamp', 'mysql')).toBe('timestamptz') | |
| 229 | expect(familyOfSqlType('datetime', 'mysql')).toBe('date') | |
| 230 | expect(familyOfSqlType('timestamp', 'postgres')).toBe('date') | |
| 231 | }) | |
| 232 | ||
| 233 | it('refuses both directions, because neither is a widening', () => { | |
| 234 | // The bug this issue is about: a naive model over an aware column. | |
| 235 | expect(satisfies('date', 'timestamptz', 'postgres')).toBe(false) | |
| 236 | // And the reverse, which loses the offset instead of inventing one. | |
| 237 | expect(satisfies('timestamptz', 'date', 'postgres')).toBe(false) | |
| 238 | expect(satisfies('timestamptz', 'timestamptz', 'postgres')).toBe(true) | |
| 239 | expect(satisfies('date', 'date', 'postgres')).toBe(true) | |
| 240 | }) | |
| 241 | ||
| 242 | it('still flags the dangerous direction on MySQL', () => { | |
| 243 | // bqb never emits MySQL TIMESTAMP, so a live one under a naive model came | |
| 244 | // from somewhere else and is session-converted on every read. | |
| 245 | expect(satisfies('date', 'timestamptz', 'mysql')).toBe(false) | |
| 246 | }) | |
| 247 | ||
| 248 | it('excuses declared timestamptz over a MySQL DATETIME', () => { | |
| 249 | // drivers/mysql.ts maps BOTH `datetime` and `timestamptz` to DATETIME, so | |
| 250 | // a correct migration produces exactly this pairing. Flagging it would | |
| 251 | // make every MySQL schema bqb created report drift — the same false | |
| 252 | // positive the boolean/tinyint rule exists to prevent. | |
| 253 | expect(satisfies('timestamptz', 'date', 'mysql')).toBe(true) | |
| 254 | expect(satisfies('timestamptz', 'date', 'singlestore')).toBe(true) | |
| 255 | }) | |
| 256 | ||
| 257 | it('stays quiet on SQLite, which has no awareness to disagree about', () => { | |
| 258 | // Every date family is stored as TEXT and no session conversion happens, | |
| 259 | // so the distinction cannot arise. Reporting it would make every clean | |
| 260 | // SQLite database look broken. | |
| 261 | expect(satisfies('timestamptz', 'text', 'sqlite')).toBe(true) | |
| 262 | expect(satisfies('date', 'timestamptz', 'sqlite')).toBe(true) | |
| 263 | expect(satisfies('timestamptz', 'date', 'sqlite')).toBe(true) | |
| 264 | }) | |
| 265 | }) | |