diff --git a/.changeset/string-family-maxlength-varchar.md b/.changeset/string-family-maxlength-varchar.md new file mode 100644 index 0000000000..9ae926f9d0 --- /dev/null +++ b/.changeset/string-family-maxlength-varchar.md @@ -0,0 +1,40 @@ +--- +'@objectstack/driver-sql': minor +--- + +driver-sql: a string field's declared `maxLength` now shapes the column it gets + +`createColumn` mapped the string family — `string` / `email` / `url` / `phone` / +`password` — with a bare `table.string(name)`, so every column took knex's +default width of 255 and the field's own `maxLength` was never read. A field +declaring a wider bound got a narrower column, and on a dialect that enforces +`varchar` length the write was refused: measured through the driver's own +`initObjects` on MySQL 8.0.46 and Postgres 16, a 300-character value written to +a `maxLength: 1024` column came back `ER_DATA_TOO_LONG` and `22001 value too +long for type character varying(255)` respectively. `schema-drift.ts` has always +treated `varchar(field.maxLength)` as the expected physical shape, so every such +column also reported permanent drift against a table the driver had just +created. + +**This changes emitted DDL for existing declarations.** A field declaring +`maxLength` now gets `varchar(maxLength)` in both directions — wider *and* +narrower than 255. Only newly created columns are affected: `createColumn` runs +on `CREATE TABLE` and `ALTER TABLE ADD COLUMN`, never on a column that already +holds rows, so nothing is truncated and no existing column is rewritten. +Narrowing a populated column remains what it was — the `narrow_varchar` drift +op, category `destructive`, behind `os migrate apply --allow-destructive`. + +A declared bound above 16383 characters (MySQL's utf8mb4 `varchar` ceiling) +makes the column `TEXT` rather than clamping it, since a clamp would reinstate +the same defect. Fields declaring no `maxLength`, or a malformed one, keep +`varchar(255)` exactly as before. `lookup` / `user`, `autonumber`, and the +catch-all branch are deliberately unchanged — none of them stores the value the +declared bound describes. + +Two matching corrections in `schema-drift.ts`, so the differ and the emitter +agree on which declarations count: a `maxLength` that is not a positive integer +is no longer read as a bound (`maxLength: 0` planned a destructive `varchar(0)` +ALTER), and a MySQL `TEXT` column is no longer diffed as a `varchar` 65535 wide +— MySQL reports `character_maximum_length` 65535 for `TEXT` where Postgres +reports NULL, so on MySQL alone every bounded unkeyed text column had been +reporting a permanent destructive `narrow_varchar` against itself. diff --git a/content/docs/protocol/objectql/types.mdx b/content/docs/protocol/objectql/types.mdx index 21f64f7a53..a7e478351f 100644 --- a/content/docs/protocol/objectql/types.mdx +++ b/content/docs/protocol/objectql/types.mdx @@ -189,7 +189,8 @@ parser, the value is **not** lowercased, and no DNS/MX lookup is performed. Stri rules belong in a validation rule or custom validator. **Database mapping:** -- SQL driver: `VARCHAR(255)` +- SQL driver: `VARCHAR(maxLength)`, or `VARCHAR(255)` when the field declares no + `maxLength` - MongoDB: `String` **Use cases:** @@ -230,8 +231,9 @@ phone: label: Phone Number ``` -**Storage format:** the string as entered — a `VARCHAR(255)` column. The engine -does **not** normalize to E.164 and does **not** reformat for display. +**Storage format:** the string as entered — a `VARCHAR(maxLength)` column, or +`VARCHAR(255)` when the field declares no `maxLength`. The engine does **not** +normalize to E.164 and does **not** reformat for display. **Validation:** a shape check only — at least 5 characters drawn from digits and `+ ( ) - . ` and whitespace (`invalid_phone` otherwise). There is no country-code @@ -1123,7 +1125,7 @@ The column each type gets from the SQL driver, per dialect: | ObjectQL Type | PostgreSQL | MySQL | SQLite | |---------------|------------|-------|--------| | `text` / `textarea` / `html` | `TEXT` \* | `TEXT` \* | `TEXT` \* | -| `email` / `url` / `phone` | `VARCHAR(255)` | `VARCHAR(255)` | `VARCHAR(255)` | +| `email` / `url` / `phone` / `password` | `VARCHAR(maxLength)` † | `VARCHAR(maxLength)` † | `VARCHAR(maxLength)` † | | `number` / `currency` / `percent` | `REAL` | `FLOAT` | `REAL` | | `date` | `DATE` | `DATE` | `TEXT` (`YYYY-MM-DD`) | | `datetime` | `TIMESTAMPTZ` | `DATETIME(3)` | `TEXT` (canonical `…Z`) | @@ -1145,6 +1147,20 @@ declares, and so does a keyed column whose bound exceeds 768 characters — see the `text` type above for why, and for what the driver does when a keyed column cannot be bounded. +† The string family takes the field's declared `maxLength` verbatim, in both +directions — a declared 1024 is a `VARCHAR(1024)` and a declared 20 is a +`VARCHAR(20)`. A field that declares no `maxLength` keeps `VARCHAR(255)`, and so +does one whose declaration is not a positive integer. Above 16383 characters +(MySQL's utf8mb4 `VARCHAR` ceiling) the column is `TEXT` instead of being +clamped, since a clamp would refuse writes the declaration permits; the bound is +still enforced at write time by the record validator's `max_length` check. + +Note the neighbouring rows that deliberately do **not** follow this rule: +`select` / `radio` store an option's machine name, `lookup` / `master_detail` / +`tree` store the referenced record's id, and `autonumber` stores a +runtime-issued number — in none of those is the stored string the value the +field's `maxLength` describes, so all of them keep `VARCHAR(255)`. + Any field flagged `multiple: true` becomes a `JSON` column regardless of its type. Relationship columns are plain id strings with no database `FOREIGN KEY` constraint (see `lookup` above). The MongoDB driver is schemaless — it issues no diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 6457cd4eed..a163829f20 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -374,6 +374,38 @@ function enforcesVarcharLength(dialect: SqlDialectName): boolean { return dialect === 'postgres' || dialect === 'mysql'; } +/** + * Is this physical column a `varchar`/`char` — the only kind that HAS a + * declared length to compare against (#11431)? + * + * Without this the length branch below read a MySQL **TEXT** column as a + * varchar 65535 wide, because that is literally what the server reports for it. + * Measured on MySQL 8.0.46 and Postgres 16, same two columns: + * + * MySQL `text` character_maximum_length = 65535 + * Postgres `text` character_maximum_length = NULL + * both `varchar(30)` character_maximum_length = 30 + * + * So the defect was MySQL-only and invisible on Postgres. Every bounded, + * unkeyed text field — the shape `createColumn` deliberately leaves as TEXT — + * diffed as "declared 4000, column allows 65535" and produced a + * `narrow_varchar` op at severity `error`, category **destructive**, against a + * table the driver had just created and which held no rows. Measured on live + * MySQL: `sys_email`'s envelope alone accounts for seven such findings, each + * inviting `os migrate apply --allow-destructive` to rewrite a TEXT column into + * a varchar for no reason. + * + * A TEXT column refuses nothing a `maxLength` allows, so there is no + * divergence to plan an ALTER for; the bound is enforced at the write seam. + * Spelled as a substring test rather than an equality because the three + * dialects disagree on the word — Postgres says `character varying`, MySQL and + * SQLite say `varchar` — matching the predicate `introspectSchema` already + * uses for the same question. + */ +function isCharacterColumn(type: string | undefined): boolean { + return /char/i.test(String(type ?? '')); +} + /** * Diff one table's metadata fields against its physical columns and return the * set of *drift* findings. Metadata is authoritative. @@ -487,24 +519,45 @@ export function diffManagedTable(args: { } // ── varchar length (only where the dialect enforces it) ────────── + // + // `maxLength` must be a POSITIVE INTEGER to be a bound (#11431). Without + // that predicate this branch read a malformed declaration as authoritative + // and planned DDL no server will accept: `maxLength: 0` took the narrowing + // arm (`0 > col.maxLength` is false) and asked for `varchar(0)`, and + // `maxLength: 12.5` asked for `varchar(12.5)` — both reported at severity + // `error`, category `destructive`, i.e. as work `os migrate apply + // --allow-destructive` should go do. + // + // It is the same predicate the EMITTER applies + // (`SqlDriver.declaredVarcharLength`, and `keyableTextLength` before it): + // a malformed bound is treated as no bound at all, and the column keeps + // its default width. Sharing the predicate is the point — the two halves + // disagreeing about which declarations count is the defect class #11431 + // exists to close, and a differ that still honoured a malformed + // `maxLength` would have re-opened it one case to the left. + const declaredMaxLength = + typeof field.maxLength === 'number' && Number.isInteger(field.maxLength) && field.maxLength > 0 + ? field.maxLength + : undefined; if ( enforcesVarcharLength(dialect) && - typeof field.maxLength === 'number' && + declaredMaxLength !== undefined && + isCharacterColumn(col.type) && typeof col.maxLength === 'number' && - field.maxLength !== col.maxLength + declaredMaxLength !== col.maxLength ) { - if (field.maxLength > col.maxLength) { + if (declaredMaxLength > col.maxLength) { out.push({ kind: 'type_mismatch', remoteName: table, table, column: fieldName, - expected: `varchar(${field.maxLength})`, + expected: `varchar(${declaredMaxLength})`, actual: `varchar(${col.maxLength})`, severity: 'warning', category: 'safe', - op: { type: 'widen_varchar', table, column: fieldName, to: field.maxLength, from: col.maxLength }, - message: `${table}.${fieldName}: metadata allows ${field.maxLength} chars but the column caps at ${col.maxLength} — widen via "os migrate".`, + op: { type: 'widen_varchar', table, column: fieldName, to: declaredMaxLength, from: col.maxLength }, + message: `${table}.${fieldName}: metadata allows ${declaredMaxLength} chars but the column caps at ${col.maxLength} — widen via "os migrate".`, }); } else { out.push({ @@ -512,12 +565,12 @@ export function diffManagedTable(args: { remoteName: table, table, column: fieldName, - expected: `varchar(${field.maxLength})`, + expected: `varchar(${declaredMaxLength})`, actual: `varchar(${col.maxLength})`, severity: 'error', category: 'destructive', - op: { type: 'narrow_varchar', table, column: fieldName, to: field.maxLength, from: col.maxLength }, - message: `${table}.${fieldName}: metadata caps at ${field.maxLength} chars but the column allows ${col.maxLength} — narrowing may truncate. "os migrate apply --allow-destructive".`, + op: { type: 'narrow_varchar', table, column: fieldName, to: declaredMaxLength, from: col.maxLength }, + message: `${table}.${fieldName}: metadata caps at ${declaredMaxLength} chars but the column allows ${col.maxLength} — narrowing may truncate. "os migrate apply --allow-destructive".`, }); } } diff --git a/packages/drivers/driver-sql/src/sql-driver-string-maxlength-varchar.test.ts b/packages/drivers/driver-sql/src/sql-driver-string-maxlength-varchar.test.ts new file mode 100644 index 0000000000..c4e4d14efa --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-string-maxlength-varchar.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #11431 — the STRING family and its declared `maxLength`. + * + * ## The defect + * + * `createColumn` mapped `string` / `email` / `url` / `phone` / `password` with a + * bare `table.string(name)`, so every one of them took knex's invented default + * of 255 and the field's own `maxLength` was never read. Measured on the + * pre-fix tree against live MySQL 8.0.46 and Postgres 16, through the driver's + * own `initObjects`: + * + * wide_email email({ maxLength: 400 }) -> varchar(255) + * wide_url url({ maxLength: 1024 }) -> varchar(255) + * narrow_phone phone({ maxLength: 20 }) -> varchar(255) + * + * and a 300-character write into the `maxLength: 1024` column — legal under the + * declaration, and accepted by the record validator's own `max_length` check — + * was refused by both enforcing dialects: MySQL `ER_DATA_TOO_LONG`, Postgres + * `22001 value too long for type character varying(255)`. + * + * The bound was inert in BOTH directions, which is why this file pins both. + * `schema-drift.ts` has always treated `varchar(field.maxLength)` as the + * expected physical shape, so before the fix a freshly created table reported + * drift against itself — measured on that same table: two `widen_varchar` + * (warning/safe) plus one `narrow_varchar` (error/**destructive**), on a table + * with no rows in it. + * + * ## Why narrowing is pinned here and is NOT a destructive migration + * + * It reads like one, so it is worth stating with the mechanism: `createColumn` + * has exactly four call sites, and every one of them is a `CREATE TABLE` or an + * `ALTER TABLE ADD COLUMN` for a column that does not exist yet. The column it + * sizes is always EMPTY. Narrowing an existing populated `varchar(255)` is a + * different road entirely — the `narrow_varchar` drift op, category + * `destructive`, behind `os migrate apply --allow-destructive` — and this + * change does not touch it. `schema-drift.ts` is byte-identical. + * + * Opt-in for the live halves — they need real servers: + * + * OS_TEST_MYSQL_URL=mysql://root:root@127.0.0.1:3306/conformance \ + * OS_TEST_POSTGRES_URL=postgres://postgres:postgres@127.0.0.1:5432/postgres \ + * pnpm --filter @objectstack/driver-sql test + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; +import { MYSQL_CELL, PG_CELL, dialectCell, declareDialectCell } from './live-dialect-matrix.testkit.js'; + +const T = 'os11431_strings'; +const PARENT = 'os11431_parent'; + +/** + * One table holding every corner of the decision at once, so a change to it + * cannot be green in five places and wrong in the sixth. + */ +const stringObject = () => ({ + name: T, + fields: { + // ── the rule, both directions ────────────────────────────── + wide_email: { type: 'email', maxLength: 400 }, + wide_url: { type: 'url', maxLength: 1024 }, + narrow_phone: { type: 'phone', maxLength: 20 }, + narrow_password: { type: 'password', maxLength: 60 }, + // ── no declaration → knex's 255, unchanged ───────────────── + plain_email: { type: 'email' }, + // ── a malformed declaration is not a bound ───────────────── + bogus_url: { type: 'url', maxLength: 0 }, + fractional_url: { type: 'url', maxLength: 12.5 }, + // ── past the varchar ceiling → TEXT, never a clamp ───────── + huge_url: { type: 'url', maxLength: 100000 }, + // ── the three families deliberately left at 255 ──────────── + a_lookup: { type: 'lookup', maxLength: 20, reference_to: PARENT }, + a_user: { type: 'user', maxLength: 30 }, + an_autonumber: { type: 'autonumber', maxLength: 8 }, + a_secret: { type: 'secret', maxLength: 4000 }, + a_select: { type: 'select', maxLength: 4000, options: ['a', 'b'] }, + }, +}); + +const parentObject = () => ({ name: PARENT, fields: { name: { type: 'text', maxLength: 64 } } }); + +/** `type(maxLength)` per column, normalised across the three dialects. */ +async function columnShapes(driver: any, table: string): Promise> { + const info: Record = await driver.knex(table).columnInfo(); + const out: Record = {}; + for (const [k, v] of Object.entries(info)) { + const t = String((v as any)?.type ?? '').toLowerCase(); + const n = (v as any)?.maxLength; + out[k] = /char/.test(t) && n ? `varchar(${n})` : t; + } + return out; +} + +// ── The emission rule, on a dialect every runner has ─────────────────────── + +describe('string-family columns take their declared maxLength (#11431)', () => { + let driver: SqlDriver; + + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + }); + + it('emits varchar(maxLength) in BOTH directions, and 255 without a declaration', async () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + await driver.initObjects([parentObject(), stringObject()]); + const shapes = await columnShapes(driver as any, T); + + // Wider than knex's default — the reported defect. + expect(shapes.wide_email).toBe('varchar(400)'); + expect(shapes.wide_url).toBe('varchar(1024)'); + // Narrower — the same defect's other half. Empty column, nothing to truncate. + expect(shapes.narrow_phone).toBe('varchar(20)'); + expect(shapes.narrow_password).toBe('varchar(60)'); + // Undeclared stays exactly where it was. + expect(shapes.plain_email).toBe('varchar(255)'); + }); + + it('treats a malformed maxLength as no declaration rather than guessing', async () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + await driver.initObjects([parentObject(), stringObject()]); + const shapes = await columnShapes(driver as any, T); + // `varchar(0)` and `varchar(12.5)` are not DDL; neither is a bound. + expect(shapes.bogus_url).toBe('varchar(255)'); + expect(shapes.fractional_url).toBe('varchar(255)'); + }); + + it('falls back to TEXT past the varchar ceiling instead of clamping', async () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + await driver.initObjects([parentObject(), stringObject()]); + const shapes = await columnShapes(driver as any, T); + // ⛔ NOT varchar(16383). Clamping would reinstate the very defect being + // fixed — a column narrower than the declaration, refusing legal writes. + expect(shapes.huge_url).toBe('text'); + }); + + it('leaves lookup / user / autonumber / catch-all at 255 — they do not store the declared value', async () => { + driver = new SqlDriver(dialectCell('sqlite').config()); + await driver.initObjects([parentObject(), stringObject()]); + const shapes = await columnShapes(driver as any, T); + // A lookup holds the referenced row's ID, not the declared value: a + // platform id is 26 characters, so `varchar(20)` could hold none of them. + expect(shapes.a_lookup).toBe('varchar(255)'); + expect(shapes.a_user).toBe('varchar(255)'); + // Runtime-issued; `maxLength` has no write-time counterpart on this type. + expect(shapes.an_autonumber).toBe('varchar(255)'); + // `secret` persists an opaque sys_secret ref; `select` a machine name. + expect(shapes.a_secret).toBe('varchar(255)'); + expect(shapes.a_select).toBe('varchar(255)'); + }); +}); + +// ── The halves only a length-ENFORCING server can measure ────────────────── + +for (const cell of [MYSQL_CELL, PG_CELL]) { + declareDialectCell(cell, 'string-family maxLength (#11431)', (c) => { + describe(`string-family maxLength on ${c.label} (#11431)`, () => { + let driver: SqlDriver; + + afterEach(async () => { + for (const t of [T, PARENT]) await driver?.execute(`drop table if exists ${t}`).catch(() => {}); + await driver?.disconnect().catch(() => {}); + }); + + it('accepts a write the declaration allows — the whole defect, in one assertion', async () => { + driver = new SqlDriver(c.config()); + for (const t of [T, PARENT]) await driver.execute(`drop table if exists ${t}`).catch(() => {}); + await driver.initObjects([parentObject(), stringObject()]); + + const shapes = await columnShapes(driver as any, T); + expect(shapes.wide_url).toBe('varchar(1024)'); + expect(shapes.narrow_phone).toBe('varchar(20)'); + + // Pre-fix this was REFUSED — `ER_DATA_TOO_LONG` on MySQL, `22001` on + // Postgres — for a value the declaration plainly permits. + await driver.create(T, { id: 'r1', wide_url: 'u'.repeat(300) }); + + // Read the length back from the SERVER rather than through the + // driver's deserializer: what is being asserted is that the column + // really holds 300 characters, and `char_length` is the database's own + // answer to that on both dialects. + const res: any = await driver.execute( + `select char_length(wide_url) as n from ${T} where id = 'r1'`, + ); + const rows: any[] = Array.isArray(res) && Array.isArray(res[0]) ? res[0] : (res?.rows ?? res); + expect(Number(rows[0]?.n ?? rows[0]?.N)).toBe(300); + }); + + it('reports no varchar drift against a table it just created', async () => { + driver = new SqlDriver(c.config()); + for (const t of [T, PARENT]) await driver.execute(`drop table if exists ${t}`).catch(() => {}); + await driver.initObjects([parentObject(), stringObject()]); + + const drift: any[] = await (driver as any).detectTableDrift(T, stringObject().fields, []); + const varcharDrift = drift.filter( + (d) => d.op?.type === 'widen_varchar' || d.op?.type === 'narrow_varchar', + ); + + // The emitter and the differ finally agree about the string family. + // Pre-fix this table reported two `widen_varchar` AND a + // `narrow_varchar` (error/destructive) against itself, with no rows in + // it. What remains is only the families this card deliberately left + // alone, so the assertion names them rather than expecting zero. + expect(varcharDrift.map((d) => d.column).sort()).toEqual( + ['a_lookup', 'a_secret', 'a_select', 'a_user', 'an_autonumber'].sort(), + ); + for (const col of ['wide_email', 'wide_url', 'narrow_phone', 'narrow_password', 'plain_email']) { + expect(varcharDrift.map((d) => d.column)).not.toContain(col); + } + + // `huge_url` is TEXT, and the differ is right to say nothing about it: + // it compares a numeric `col.maxLength`, which a TEXT column does not + // report. A TEXT column refuses nothing the declaration allows, so + // there is no divergence to plan an ALTER for. + expect(varcharDrift.map((d) => d.column)).not.toContain('huge_url'); + + // ⛔ The malformed pair, and the reason `schema-drift.ts` is in this + // diff at all. On the pre-fix tree BOTH of these reported + // `narrow_varchar` at severity `error` / category `destructive` — + // `maxLength: 0` takes the narrowing arm because `0 > 255` is false — + // so `os migrate apply --allow-destructive` was being asked to run + // `varchar(0)` and `varchar(12.5)`, DDL no server accepts. The emitter + // treats a malformed bound as no bound; the differ now applies the + // same predicate, which is the whole point of the fix. + expect(varcharDrift.map((d) => d.column)).not.toContain('bogus_url'); + expect(varcharDrift.map((d) => d.column)).not.toContain('fractional_url'); + }); + + it('still refuses a value past the DECLARED bound — the bound binds, it did not merely move', async () => { + driver = new SqlDriver(c.config()); + for (const t of [T, PARENT]) await driver.execute(`drop table if exists ${t}`).catch(() => {}); + await driver.initObjects([parentObject(), stringObject()]); + + // ⛔ The negative half. `varchar(20)` must really be 20 — a fix that + // widened everything to silence the symptom would pass every + // assertion above and fail this one. + await expect( + driver.create(T, { id: 'r2', narrow_phone: 'p'.repeat(300) }), + ).rejects.toThrow(/too long|ER_DATA_TOO_LONG|22001/i); + }); + }); + }); +} diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index fa767ee7c8..13034707a1 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -12596,6 +12596,87 @@ export class SqlDriver implements IDataDriver { * Either way the column stays TEXT and, on MySQL, the index over it is * refused with the diagnostic {@link explainUnkeyableTextColumn} writes. */ + /** + * knex's `table.string(name)` default, spelled out rather than inherited + * (#11431). It is the width a string-family field with no usable + * `maxLength` keeps — unchanged behaviour, and deliberately so: a field that + * declares no bound has not asked for one, and inventing a narrower column + * would impose a truncation boundary its author never wrote. + */ + protected static readonly DEFAULT_STRING_VARCHAR_CHARS = 255; + + /** + * The widest `varchar(n)` a column may declare, across every dialect this + * driver speaks (#11431). + * + * Measured, not read off a doc page. MySQL 8.0.46, utf8mb4/InnoDB: + * `varchar(16383)` creates and `varchar(16384)` is refused with + * `ERROR 1074 Column length too big for column 'c' (max = 16383); use BLOB + * or TEXT instead` — 65535 bytes ÷ 4 bytes per utf8mb4 character. Postgres + * 16 accepts up to `varchar(10485760)` and refuses `10485761` with + * `length for type varchar cannot exceed 10485760`; SQLite records the + * declared type verbatim and enforces nothing. + * + * ⚠️ ONE ceiling for every dialect, deliberately, and it is the LOWEST of + * the three. The alternative is one declaration with three physical shapes, + * so the same app would take a `maxLength: 100000` url as a bounded column + * on Postgres and refuse to create the table at all on MySQL — the + * dialect-divergent enforcement this file's conformance matrices exist to + * close, and the same call {@link keyableTextLength}'s neighbour already + * made for the text family. + * + * ⚠️ What this constant is NOT: a guarantee the table will create. MySQL + * also caps the SUM of a row's declared byte widths at 65535 (measured: + * 15 × `varchar(1024)` creates, 16 × `varchar(1024)` is refused with + * `ERROR 1118 Row size too large`), a limit no per-column decision can see. + * Postgres has no such limit (it TOASTs; 40 × `varchar(4096)` creates + * cleanly). No platform object comes near it — the widest declared bound on + * a string-family field in this repo is 2000, and no object carries more + * than six such fields — but an authored app can, and the server's own + * refusal is the only thing that reports it today. + */ + protected static readonly MAX_VARCHAR_CHARS = 16383; + + /** + * The `varchar(n)` a STRING-family column should take, or `null` to make it + * TEXT instead (#11431). + * + * Three outcomes, each a deliberate answer rather than a fallback: + * + * - **no usable declaration** — `maxLength` absent, or not a positive + * integer — keeps {@link DEFAULT_STRING_VARCHAR_CHARS}. Unchanged + * behaviour for every field that never declared a bound, which is the + * overwhelming majority of them. + * - **a declaration this dialect can express** returns it verbatim, in + * BOTH directions. Wider than 255 is the reported defect; narrower is + * the same defect's other half — `maxLength: 20` took `varchar(255)` + * too, so the bound bound in neither direction. + * - **a declaration wider than {@link MAX_VARCHAR_CHARS}** returns `null` + * and the column becomes TEXT. Emitting `varchar(100000)` would be DDL + * MySQL refuses outright, and clamping it to the ceiling would reinstate + * exactly the defect being fixed — a column narrower than the + * declaration, refusing writes the declaration allows. TEXT refuses + * nothing the author declared; the bound is still enforced, at the + * write seam where `maxLength` enforcement actually lives (the record + * validator's `max_length` check), with a field-named ADR-0112 envelope + * instead of a raw `ER_DATA_TOO_LONG`. + * + * ⚠️ Deliberately mirrors {@link keyableTextLength} without sharing code + * with it. The two families answer different questions — that one asks + * "can this KEY?" and returns `null` for an unbounded field, this one asks + * "how wide is this column?" and returns 255 — and #11374's remaining half + * may still reshape the text side. A shared helper would couple a settled + * decision to an unsettled one. + */ + protected declaredVarcharLength(field: any): number | null { + const declared = (field as { maxLength?: unknown }).maxLength; + const n = typeof declared === 'string' ? Number(declared) : declared; + if (typeof n !== 'number' || !Number.isInteger(n) || n <= 0) { + return SqlDriver.DEFAULT_STRING_VARCHAR_CHARS; + } + return n > SqlDriver.MAX_VARCHAR_CHARS ? null : n; + } + protected keyableTextLength(field: any): number | null { const declared = (field as { maxLength?: unknown }).maxLength; const n = typeof declared === 'string' ? Number(declared) : declared; @@ -12677,9 +12758,42 @@ export class SqlDriver implements IDataDriver { case 'email': case 'url': case 'phone': - case 'password': - col = table.string(name); + case 'password': { + // #11431: the string family takes its declared `maxLength`, instead of + // knex's invented default of 255. + // + // The bound is the field's OWN — nothing is guessed. `schema-drift.ts` + // has always treated `varchar(field.maxLength)` as the EXPECTED + // physical shape of a bounded field (`widen_varchar` / + // `narrow_varchar` say so in as many words), so before this branch the + // differ and the emitter disagreed about every column the driver + // itself had just created. Measured on live MySQL 8.0.46 and Postgres + // 16 on the pre-fix tree: a `maxLength: 400` email, a `maxLength: 1024` + // url and a `maxLength: 20` phone all landed as `varchar(255)`, and the + // same table reported two `widen_varchar` (warning/safe) plus one + // `narrow_varchar` (error/DESTRUCTIVE) findings against itself, on a + // table with no rows in it. + // + // The write half is the reported defect. A 300-character value — + // legal under the declaration, and accepted by the record validator's + // own `max_length` check — was refused by BOTH enforcing dialects: + // MySQL `ER_DATA_TOO_LONG` under `STRICT_TRANS_TABLES`, Postgres + // `22001 value too long for type character varying(255)`. + // + // ⚠️ Both directions, and the narrowing one is NOT a destructive + // migration — a point worth stating because it reads like one: + // `createColumn` runs on `CREATE TABLE` and on `ALTER TABLE ADD + // COLUMN`, so the column it sizes is always EMPTY. Narrowing an + // EXISTING `varchar(255)` that holds rows stays exactly where it was + // before this change: the `narrow_varchar` drift op, category + // `destructive`, behind `os migrate apply --allow-destructive`. + // Nothing here plans an ALTER on an existing column, and the differ is + // untouched. What this removes is the permanent, self-inflicted drift + // report a fresh table used to raise against itself. + const declared = this.declaredVarcharLength(field); + col = declared === null ? table.text(name) : table.string(name, declared); break; + } case 'text': case 'textarea': case 'html': @@ -12786,6 +12900,19 @@ export class SqlDriver implements IDataDriver { // primitive — it shares this exact DDL path so reads/$expand/FK stay uniform. case 'lookup': case 'user': + // ⛔ #11431 deliberately STOPS at the string family and does not reach + // here, even though these branches spell the same `table.string(name)`. + // A lookup column does not hold the declared value — it holds the + // REFERENCED ROW'S ID, whose width is the parent table's `id` column, + // not anything this field declared. Measured on MySQL 8.0.46: the FK + // itself is content with mismatched widths (`varchar(20)` child → + // `varchar(255)` parent `id` creates cleanly, and Postgres 16 accepts + // it too), so the type system gives no warning — but the first write + // is refused, because a platform id is 26 characters and + // `INSERT … VALUES ('01JQ8XKZ9M4N7P2R5T6V8W0Y3B')` into `varchar(20)` + // is `ERROR 1406 Data too long`. Honouring `maxLength` here would make + // the column structurally incapable of holding ANY id — a strictly + // worse defect than the one #11431 fixes. col = table.string(name); if (field.reference_to) { table.foreign(name).references('id').inTable(field.reference_to); @@ -12796,6 +12923,17 @@ export class SqlDriver implements IDataDriver { break; case 'auto_number': case 'autonumber': + // ⛔ Also out of #11431's scope, for a different reason than `lookup` + // above: the value is issued by the RUNTIME from its sequence, not + // supplied by a caller. `maxLength` has no write-time counterpart on + // this type — the record validator's `max_length` check covers the + // textual types only, and an autonumber never reaches it at all + // (runtime-owned types are excluded from its door) — so binding the + // DDL to it would create a refusal with nothing declaring it: the + // platform's own generated record number rejected by a column, with no + // author to hand the error to. Widening `maxLength` to a second + // meaning ("how wide is the generated number") is a spec decision, not + // a driver one. col = table.string(name); break; case 'formula': @@ -12806,6 +12944,16 @@ export class SqlDriver implements IDataDriver { // (the read-side deserializer) can never drift — the drift between them // is exactly what let array-valued fields reach the binder un-serialized // (#field-zoo). Everything else is a plain string. + // + // ⛔ The third branch #11431 leaves alone. This is the CATCH-ALL, and + // what lands in it is precisely the set of types whose stored value is + // NOT the declared value: `secret` persists an opaque `sys_secret` + // ref rather than the credential it was given (ADR-0100), and + // `select` / `radio` / `checkboxes` / `code` / `tree` store option + // machine names or ids. Sizing any of those from the author's + // `maxLength` would size the wrong string. A type that genuinely wants + // the bound belongs in the string-family case above, named — never + // acquired by falling through to here. col = JSON_COLUMN_TYPES.has(type) ? table.json(name) : table.string(name); }