fix: an unbounded string column is text on Postgres, not varchar(255)

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/bun-query-builder/src/drivers/postgres.tsmodified+8-1

Changes to packages/bun-query-builder/src/drivers/postgres.ts

		⋯103 linesShow the 103 lines above this hunk@@ -104,7 +104,14 @@ export class PostgresDriver implements DialectDriver {
104	104
105	105	private getColumnType(column: ColumnPlan): string {
106	106	switch (column.type) {
107		- case 'string': return `varchar(${column.maxLength ?? 255})`
	107	+ // An unbounded `schema.string()` is `text`, not `varchar(255)`. The 255
	108	+ // default is a MySQL idiom: there, TEXT cannot be indexed without a
	109	+ // prefix length, so a bounded varchar is worth defaulting to. Postgres
	110	+ // stores `text` and `varchar(n)` identically and indexes both, so the
	111	+ // default bought nothing and cost correctness - a column the model never
	112	+ // bounded rejected its own writes with "value too long for type
	113	+ // character varying(255)". A declared `.max(n)` is still honoured.
	114	+ case 'string': return column.maxLength === undefined ? 'text' : `varchar(${column.maxLength})`
108	115	case 'text': return 'text'
109	116	case 'boolean': return 'boolean'
110	117	case 'integer': return 'integer'

packages/bun-query-builder/src/migrations.tsmodified+26-4

Changes to packages/bun-query-builder/src/migrations.ts

		⋯1,614 linesShow the 1,614 lines above this hunk@@ -1615,8 +1615,14 @@ export function canonicalStorageType(col: ColumnPlan, dialect: SupportedDialect)
1615	1615	// invisible: both are `string`, the diff saw nothing, and the column stayed
1616	1616	// varchar(50) while the validator accepted 100 characters - every longer
1617	1617	// write then failed with "value too long for type character varying(50)".
	1618	+ // Must agree with PostgresDriver.getColumnType: an unbounded string is
	1619	+ // `text` there, and a live `text` column introspects back as `type: 'text'`.
	1620	+ // Reporting `varchar(255)` for the model side would diff against that
	1621	+ // forever, re-ALTERing an unchanged column on every reconciliation.
1618	1622	if (col.type === 'string')
1619		- return `varchar(${col.maxLength ?? 255})`
	1623	+ return dialect === 'postgres' && col.maxLength === undefined
	1624	+ ? 'text'
	1625	+ : `varchar(${col.maxLength ?? 255})`
1620	1626	// Likewise a decimal's precision and scale, which used to be a hard-coded
1621	1627	// (10,2) nothing could change.
1622	1628	if (col.type === 'decimal')
		⋯1 lineShow the 1 line above this hunk@@ -1624,6 +1630,22 @@ export function canonicalStorageType(col: ColumnPlan, dialect: SupportedDialect)
1624	1630	return col.type
1625	1631	}
1626	1632
	1633	+/**
	1634	+ * How many characters a string column accepts.
	1635	+ *
	1636	+ * "No bound declared" is not one number across dialects: on Postgres it is
	1637	+ * `text`, which has no limit, and on MySQL it is `varchar(255)`, because
	1638	+ * MySQL TEXT cannot be indexed without a prefix length. Folding both into 255
	1639	+ * made dropping a bound on Postgres look like a narrowing to 255 when it is
	1640	+ * really a widening to `text`, which would have flagged a safe ALTER
	1641	+ * destructive. A caller that passes no dialect keeps the single old default.
	1642	+ */
	1643	+function stringCharacterBound(col: ColumnPlan, dialect?: SupportedDialect): number {
	1644	+ if (col.maxLength !== undefined)
	1645	+ return col.maxLength
	1646	+ return dialect === 'postgres' ? Number.POSITIVE_INFINITY : 255
	1647	+}
	1648	+
1627	1649	/**
1628	1650	* Whether a change keeps the physical type but tightens a bound on it, so
1629	1651	* existing values may no longer fit (a varchar(100) becoming varchar(50), a
		⋯1 lineShow the 1 line above this hunk@@ -1631,9 +1653,9 @@ export function canonicalStorageType(col: ColumnPlan, dialect: SupportedDialect)
1631	1653	* Postgres and strict MySQL refuse the ALTER when a row is too long; MySQL in
1632	1654	* a permissive mode truncates. Either way it is not a free change.
1633	1655	*/
1634		-export function isNarrowingChange(from: ColumnPlan, to: ColumnPlan): boolean {
	1656	+export function isNarrowingChange(from: ColumnPlan, to: ColumnPlan, dialect?: SupportedDialect): boolean {
1635	1657	if (from.type === 'string' && to.type === 'string')
1636		- return (to.maxLength ?? 255) < (from.maxLength ?? 255)
	1658	+ return stringCharacterBound(to, dialect) < stringCharacterBound(from, dialect)
1637	1659	if (from.type === 'decimal' && to.type === 'decimal') {
1638	1660	// Lossless only when neither the fractional digits nor the integer
1639	1661	// digits (precision - scale) shrink.
		⋯785 linesShow the 785 lines above this hunk@@ -2425,7 +2447,7 @@ export function generateDiffOperations(previous: MigrationPlan | undefined, next
2425	2447	kind: 'modify_column',
2426	2448	table: curr.table,
2427	2449	column: colName,
2428		- destructive: (prevCol.type !== currCol.type && !isLosslessTypeChange(prevCol.type, currCol.type)) || isNarrowingChange(prevCol, currCol),
	2450	+ destructive: (prevCol.type !== currCol.type && !isLosslessTypeChange(prevCol.type, currCol.type)) || isNarrowingChange(prevCol, currCol, dialect),
2429	2451	sql: modifyColumnStatement,
2430	2452	})
2431	2453	info(`-- Detected column change: ${curr.table}.${colName} (${prevCol.type} -> ${currCol.type})`)

packages/bun-query-builder/test/migrations.attributes.test.tsmodified+8-4

Changes to packages/bun-query-builder/test/migrations.attributes.test.ts

		⋯1,626 linesShow the 1,626 lines above this hunk@@ -1627,8 +1627,10 @@ describe('migrations - complex model attributes', () => {
1627	1627
1628	1628	describe('dialect-specific SQL generation', () => {
1629	1629	it('generates correct PostgreSQL types', () => {
1630		- // Note: validation rule { name: 'text' } maps to 'string' type (varchar(255))
1631		- // The 'text' column type is only inferred from long default strings (>255 chars)
	1630	+ // An unbounded string is `text` on Postgres; only a declared `.max(n)`
	1631	+ // produces `varchar(n)`. This test used to pin varchar(255) for `str`,
	1632	+ // which was the defect: a column the model never bounded rejected its
	1633	+ // own writes at 256 characters.
1632	1634	const longString = 'x'.repeat(300)
1633	1635	const models = defineModels({
1634	1636	AllTypes: {
		⋯2 linesShow the 2 lines above this hunk@@ -1637,6 +1639,7 @@ describe('migrations - complex model attributes', () => {
1637	1639	attributes: {
1638	1640	id: { validation: { rule: {} } },
1639	1641	str: { validation: { rule: { name: 'string' } } },
	1642	+ code: { validation: { rule: { name: 'string', rules: [{ name: 'max', params: { max: 40 } }] } } },
1640	1643	txt: { default: longString, validation: { rule: {} } }, // Use long default to get text type
1641	1644	int_col: { validation: { rule: { name: 'integer' } } },
1642	1645	big: { validation: { rule: { name: 'bigint' } } },
		⋯11 linesShow the 11 lines above this hunk@@ -1654,8 +1657,9 @@ describe('migrations - complex model attributes', () => {
1654	1657	const plan = buildMigrationPlan(models as any, { dialect: 'postgres' })
1655	1658	const sql = generateSql(plan).join('\n').toLowerCase()
1656	1659
1657		- expect(sql).toContain('varchar')
1658		- expect(sql).toContain('text') // From long default string
	1660	+ expect(sql).toContain('"str" text') // unbounded: no varchar(255) default
	1661	+ expect(sql).toContain('"code" varchar(40)') // bounded: the declared max
	1662	+ expect(sql).toContain('"txt" text') // From long default string
1659	1663	expect(sql).toContain('integer')
1660	1664	expect(sql).toContain('bigint')
1661	1665	expect(sql).toContain('timestamp')

packages/bun-query-builder/test/migrations.postgres-varchar-length.test.tsmodified+35-3

Changes to packages/bun-query-builder/test/migrations.postgres-varchar-length.test.ts

		⋯24 linesShow the 24 lines above this hunk@@ -25,7 +25,22 @@ const handle = (plan: MigrationPlan): ColumnPlan => plan.tables[0].columns.find(
25	25	describe('postgres varchar length', () => {
26	26	it('carries the bound into the canonical storage type', () => {
27	27	expect(canonicalStorageType(handle(modelPlan(50)), 'postgres')).toBe('varchar(50)')
28		- expect(canonicalStorageType(handle(modelPlan()), 'postgres')).toBe('varchar(255)')
	28	+ })
	29	+
	30	+ // An unbounded column is `text` here, so the model side has to say `text`
	31	+ // too. A live `text` column introspects back as `type: 'text'`; reporting
	32	+ // `varchar(255)` for the model would diff against that on every run and
	33	+ // re-ALTER a column nobody touched.
	34	+ it('an unbounded string is text, and agrees with what the driver emits', () => {
	35	+ expect(canonicalStorageType(handle(modelPlan()), 'postgres')).toBe('text')
	36	+ expect(canonicalStorageType({ name: 'handle', type: 'text' } as ColumnPlan, 'postgres')).toBe('text')
	37	+ })
	38	+
	39	+ // MySQL keeps the 255 default: TEXT there cannot be indexed without a
	40	+ // prefix length, so the bound is worth having.
	41	+ it('leaves the MySQL default alone', () => {
	42	+ expect(canonicalStorageType(handle(modelPlan()), 'mysql')).toBe('VARCHAR(255)')
	43	+ expect(canonicalStorageType(handle(modelPlan(50)), 'mysql')).toBe('VARCHAR(50)')
29	44	})
30	45
31	46	it('widening max(50) -> max(100) emits ALTER COLUMN ... TYPE varchar(100), non-destructively', () => {
		⋯16 linesShow the 16 lines above this hunk@@ -48,9 +63,26 @@ describe('postgres varchar length', () => {
48	63	expect(operations).toEqual([])
49	64	})
50	65
51		- it('an unbounded string and varchar(255) are the same column', () => {
	66	+ // These used to be the same column, which is what made an unbounded
	67	+ // `schema.string()` reject its own writes at 256 characters. They are now
	68	+ // different, and dropping the bound widens to text rather than narrowing.
	69	+ it('dropping a bound widens varchar(255) to text, non-destructively', () => {
	70	+ const { operations } = generateDiffOperations(modelPlan(255), modelPlan())
	71	+ const change = operations.find(op => op.kind === 'modify_column' && op.column === 'handle')
	72	+ expect(change).toBeDefined()
	73	+ expect(change!.sql).toContain('ALTER COLUMN "handle" TYPE text')
	74	+ expect(change!.destructive).toBe(false)
	75	+ })
	76	+
	77	+ it('adding a bound to an unbounded column is destructive, since text may not fit', () => {
52	78	const { operations } = generateDiffOperations(modelPlan(), modelPlan(255))
53		- expect(operations).toEqual([])
	79	+ const change = operations.find(op => op.kind === 'modify_column' && op.column === 'handle')
	80	+ expect(change!.sql).toContain('TYPE varchar(255)')
	81	+ expect(change!.destructive).toBe(true)
	82	+ })
	83	+
	84	+ it('an unchanged unbounded column produces no change', () => {
	85	+ expect(generateDiffOperations(modelPlan(), modelPlan()).operations).toEqual([])
54	86	})
55	87
56	88	it('introspection reads the live bound so a live diff compares lengths too', () => {

packages/bun-query-builder/test/varchar-width.test.tsmodified+12-3

Changes to packages/bun-query-builder/test/varchar-width.test.ts

		⋯4 linesShow the 4 lines above this hunk@@ -5,6 +5,11 @@ import { buildMigrationPlan } from '../src/migrations'
5	5	// A bounded string column (`.max(n)`) should become `varchar(n)` instead of the
6	6	// default `varchar(255)`, so tight columns don't over-reserve. Indexable
7	7	// bounded identifiers may exceed 255; content-sized values become `text`.
	8	+//
	9	+// An UNBOUNDED string splits by dialect, which is the contrast the two DDL
	10	+// tests below draw: `varchar(255)` on MySQL, where TEXT cannot be indexed
	11	+// without a prefix length, and `text` on Postgres, which stores and indexes
	12	+// both the same way and so gains nothing from a bound the model never set.
8	13	const mkStr = (max?: number) => ({
9	14	validation: { rule: { name: 'string', rules: max ? [{ name: 'max', params: { max } }] : [] } },
10	15	})
		⋯11 linesShow the 11 lines above this hunk@@ -22,7 +27,7 @@ const models = {
22	27	id: mkStr(32),
23	28	country: mkStr(2),
24	29	mailbox: mkStr(320),
25		- path: mkStr(), // unbounded → varchar(255)
	30	+ path: mkStr(), // unbounded → varchar(255) on MySQL, text on Postgres
26	31	bio: mkStr(5000), // > 255 → text
27	32	payload: mkTypedStr(16_000), // explicit type must not hide validator width
28	33	},
		⋯27 linesShow the 27 lines above this hunk@@ -56,11 +61,15 @@ describe('varchar width from .max()', () => {
56	61	expect(ddl).toContain('`payload` text')
57	62	})
58	63
59		- it('emits varchar(n) in Postgres DDL', () => {
	64	+ it('emits varchar(n) in Postgres DDL, and text for an unbounded column', () => {
60	65	const ddl = getDialectDriver('postgres' as any).createTable(plan('postgres'))
61	66	expect(ddl).toContain('"id" varchar(32)')
62	67	expect(ddl).toContain('"country" varchar(2)')
63	68	expect(ddl).toContain('"mailbox" varchar(320)')
64		- expect(ddl).toContain('"path" varchar(255)')
	69	+ // Not varchar(255): nothing bounded this column, and a Postgres varchar
	70	+ // rejects its own writes past the bound while text would have taken them.
	71	+ expect(ddl).toContain('"path" text')
	72	+ expect(ddl).toContain('"bio" text')
	73	+ expect(ddl).toContain('"payload" text')
65	74	})
66	75	})