Skip to content

Commit eab52a7

Browse files
committed
fix(cli): generated migrations emit the character column driver-sql creates (#16091)
Both migration generators capped a `text` field at VARCHAR(255) while `driver-sql` creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. #15521's ruling names this card and settles its direction -- the generator follows the driver, as #15040 already did for the `id` column in this same file. Driven on a private PostgreSQL 16.13 cluster, all three producers run from one object and their columns read back out of `information_schema.columns`. The sweep found nine divergent columns of 26 probed, not one: text driver text gen varchar(255) both formats text+max driver text gen varchar(255) maxLength must NOT size it email+max driver varchar(400) gen varchar(255) maxLength was never read url driver varchar(255) sql varchar(2048) invented width phone driver varchar(255) sql varchar(50) invented width color driver varchar(255) sql varchar(7) invented width All of them now follow `createColumn`'s three arms. The text family is unbounded, because that arm branches on KEYED and a generated migration emits no index; its declared bound is enforced at the write seam, not by the column. The string family takes `declaredVarcharLength`'s answer -- the declaration verbatim in both directions, knex's 255 without one, and TEXT above the varchar ceiling rather than a clamp to it. The catch-all keeps the default width and ignores a declaration, because its stored value is an option code or another row's id rather than the declared string. Driven again afterwards: 0 of 26 columns diverge, and the 300-character write is accepted in all three tables exactly where the platform accepts it and refused in all three exactly where the platform refuses it. `generate-string-family-width.pin.test.ts` asserts that agreement against the driver's own source -- arm membership read from `createColumn`'s case labels, widths read from its own constants -- so a driver that moves fails there instead of leaving the generators quietly wrong. Three existing pin files move with it: two used `text`'s old VARCHAR(255) as a stand-in for the driver's default string column, and one asserted column ordering by searching for a `table.string` call that is now a `table.text` call. Scope is PostgreSQL, the only dialect `--format sql` claims (#15521). The FILE_REFERENCE_TYPES divergence stays recorded and unresolved (#15041). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01D47qPfEWVPmhguWgBZCi5N
1 parent dacb73f commit eab52a7

6 files changed

Lines changed: 703 additions & 15 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@objectstack/cli": patch
3+
---
4+
5+
`os generate migration` now emits the character column `driver-sql` actually creates, in both the TypeScript and the SQL format.
6+
7+
A `text` field took `VARCHAR(255)` from both generators while the platform creates an unbounded `text` column for it, so a 300-character value the platform stores was refused by every generated table with `value too long for type character varying(255)`. Enumerating the whole character-column family found the same disagreement in eight more places: `url` and `phone` and `color` carried widths the generators invented (2048, 50 and 7 against the platform's 255), and neither generator read a field's declared `maxLength` at all, so a `maxLength: 400` email was `varchar(400)` on the platform and `varchar(255)` in the migration generated for it.
8+
9+
All of them now follow the platform's own three answers: the text family is unbounded (its declared bound is enforced at the write seam, not by the column), the string family takes its declared `maxLength` — verbatim in both directions, and TEXT rather than a clamp when it exceeds what a `varchar` can express — and the remaining string-valued types keep the default width and ignore a declaration, because their stored value is an option code or another row's id rather than the declared string.
10+
11+
This scopes to PostgreSQL, which is the only dialect `os generate migration --format sql` claims.

packages/cli/src/commands/generate-builtin-id-column.pin.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,13 @@ describe('the builtin id column both migration generators emit (#15040)', () =>
226226
expect(sql.indexOf('"id"')).toBeLessThan(sql.indexOf('"title"'));
227227
const ts = generateMigrationTs(CONFIG as Record<string, unknown>);
228228
expect(ts).toContain("await db.schema.createTable('account'");
229-
expect(ts.indexOf("table.string('id')")).toBeLessThan(ts.indexOf("table.string('title')"));
229+
// #16091 — matched on the field NAME rather than on its column method. The
230+
// assertion is about ORDER (the primary key comes first), and a reader keyed
231+
// to `table.string` silently became `indexOf(…) === -1` the moment `title`,
232+
// a `text` field, moved to `table.text` — which reads as a passing
233+
// "less than" only until you notice what it is less than.
234+
expect(ts.indexOf("table.string('id')")).toBeLessThan(ts.indexOf("('title')"));
235+
expect(ts.indexOf("('title')"), 'the title column vanished from the output').toBeGreaterThan(0);
230236
// Each generator carries exactly ONE hardcoded id line — the shape that let
231237
// these two disagree with the driver in the first place, and the reason a
232238
// fix to one of them can silently leave the other behind. Counted over the

packages/cli/src/commands/generate-field-type-vocabulary.pin.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,18 @@ describe('#14828 — the SQL answers are the platform’s, not this file’s inv
567567
expect(tsInterfaceType('number')).toBe('number');
568568
// The driver's own answer for the headline member, read where it lives.
569569
expect(createColumnArm('autonumber')).toContain('table.string(name)');
570-
expect(sqlColumn('autonumber')).toBe(sqlColumn('text'));
570+
// #16091 — compared against `lookup`, not against `text`. Both were
571+
// `VARCHAR(255)` when this line was written, which made `text` a usable
572+
// stand-in for "the driver's default string column"; it is not one any
573+
// more. `createColumn` gives `text` its text-family arm (an unbounded TEXT
574+
// for every unkeyed column) and gives `lookup` the same bare
575+
// `table.string(name)` it gives `autonumber` — asserted here, from the
576+
// driver, so the comparator cannot silently become a different question again.
577+
expect(createColumnArm('lookup')).toContain('table.string(name)');
578+
expect(sqlColumn('autonumber')).toBe(sqlColumn('lookup'));
579+
// Anti-vacuity: the comparator is a real, DIFFERENT answer from the
580+
// text family's, so this equality is a measurement rather than a tautology.
581+
expect(sqlColumn('autonumber')).not.toBe(sqlColumn('text'));
571582
expect(tsColumn('autonumber')).toBe("table.string('f_autonumber')");
572583
});
573584

packages/cli/src/commands/generate-multiple-json-column.pin.test.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,8 +165,14 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces',
165165
// control cannot be satisfied by one column shape for everything either.
166166
expect(sqlColumn('single_lookup')).toBe('VARCHAR(255)');
167167
expect(tsColumn('single_lookup')).toBe("table.string('single_lookup')");
168-
expect(sqlColumn('single_text')).toBe('VARCHAR(255)');
169-
expect(tsColumn('single_text')).toBe("table.string('single_text')");
168+
// #16091 — `text` is an unbounded TEXT column now, which is what
169+
// `createColumn`'s text-family arm builds for every unkeyed column. The
170+
// control is unweakened by that for exactly the reason the `lookup` note
171+
// above gives: what it discriminates is scalar-vs-JSON, and TEXT is scalar.
172+
expect(sqlColumn('single_text')).toBe('TEXT');
173+
expect(tsColumn('single_text')).toBe("table.text('single_text')");
174+
// …and it still discriminates: the scalar answer is not the JSON one.
175+
expect(sqlColumn('single_text')).not.toBe(sqlColumn('multi_text'));
170176
expect(sqlColumn('single_file')).toBe('VARCHAR(2048)');
171177
expect(tsInterfaceType('single_lookup')).toBe('string');
172178
});
@@ -285,8 +291,8 @@ describe('#14829 — `multiple: true` is one answer across all three surfaces',
285291
// this card does NOT touch is present in both outputs. Without it, "does
286292
// not contain" would pass on an empty string.
287293
expect(other).toContain('CREATE TABLE IF NOT EXISTS "probe" (');
288-
expect(other).toContain('"t" VARCHAR(255)');
289-
expect(otherTs).toContain("table.string('t')");
294+
expect(other).toContain('"t" TEXT');
295+
expect(otherTs).toContain("table.text('t')");
290296

291297
// A RENDERED string (prefix + counter + suffix), never an integer sequence.
292298
expect(other).toContain('"a" VARCHAR(255)');

0 commit comments

Comments
 (0)