diff --git a/.changeset/sql-driver-field-string-comment-fix.md b/.changeset/sql-driver-field-string-comment-fix.md new file mode 100644 index 0000000000..33bc44ee48 --- /dev/null +++ b/.changeset/sql-driver-field-string-comment-fix.md @@ -0,0 +1,43 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): correct two comments in `createColumn` that cited a `Field.string` builder that has never existed (#12593) + +Two comments in the keyed/bounded text-family branch of `createColumn` +(`signature` / `qrcode` / `richtext` / `code`) asserted a `Field.string` +builder exists and that it "has always taken knex's `varchar(255)`." It does +not exist and never has: `Field` has 37 keys and none is `string`, +`FieldType.options` (49 entries) does not list it, and +`FieldSchema.safeParse({ type: 'string', … })` fails at `[type]` — all three +reproduced fresh on this branch, plus `git log -S` confirming no commit ever +added such a builder key to `field.zod.ts`. + +The name is not arbitrary: `'string'` is knex's own column-builder method +name (`table.string(name)`), reused internally by this driver as its +*untyped* default (`field?.type || 'string'`) — a storage-side spelling that +collides with, but is not, an authoring-side one. The corrected comments now +state only checkable facts: knex's bare `table.string(name)` (no length) is +`varchar(255)`; this branch never calls it bare, it calls +`table.string(name, keyable)` with the field's own declared `maxLength` +(up to `MAX_KEYABLE_VARCHAR_CHARS`, 768 chars); and the nearest *authorable* +spelling that reaches this exact `varchar(n)` shape is `Field.text({ +maxLength: n })` on a keyed column — exactly what this switch arm already +serves. + +No code changed — `git diff` is comment-only lines inside `sql-driver.ts`. +Nothing here alters DDL, column widths, or any runtime behavior; the two +pins that already exercise this exact branch behaviorally +(`sql-driver-11565-row-byte-budget.test.ts`'s "agrees with createColumn about +every FieldType" mirror, and `sql-driver-keyed-text-mysql.test.ts`'s +"emits varchar(maxLength) for a keyed bounded field") both stay green, +unmodified. + +**Grade: `patch`, and deliberately no higher.** This is documentation +embedded in source, not an exported symbol, a spec key, or any authorable or +runtime surface — there is nothing here for a consumer to migrate. `patch` +is the correct floor for a fix that changes only what the driver's own +source *says*, matching the sibling `builtin-column-delivery-id-type.md` +changeset (#12131) that corrected the same false `Field.string` premise in +an adjacent file. Not a declared-breaking changeset, so no ADR-0087 +disposition marker applies. diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 895550f512..317f733202 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -14636,17 +14636,25 @@ export class SqlDriver implements IDataDriver { // `schema-drift.ts` already treats `varchar(field.maxLength)` as the // expected physical shape of a bounded field (its `widen_varchar` / // `narrow_varchar` ops say so in as many words); this is the emitter - // finally agreeing with the differ. `Field.string` has always taken - // knex's `varchar(255)`, so a bounded text field is now LESS arbitrary - // than its string sibling, not more. + // finally agreeing with the differ. There is no `Field.string` + // builder: none of `Field`'s keys is named `string`, `FieldType.options` + // omits it too, and `FieldSchema.safeParse({ type: 'string', … })` + // fails at `[type]` (#12593). What IS true of knex: its bare + // `table.string(name)` call (no length argument) is `varchar(255)`. + // This branch never calls it bare — `table.string(name, keyable)`, + // where `keyable` is the field's own declared `maxLength`, up to + // `MAX_KEYABLE_VARCHAR_CHARS` (768 chars, wider than knex's + // 255-char default). // // Applied on every dialect rather than under `isMysql`, deliberately: // the alternative is one declaration with two enforcement answers, so // the same app would refuse an over-length write on MySQL and accept it // on Postgres. Dialect-divergent enforcement of one declared bound is // the defect class this repo's conformance matrices exist to close, and - // a `varchar(n)` is exactly what the SQLite and Postgres columns would - // have been had the field been declared `Field.string`. + // a `varchar(n)` is exactly what the SQLite and Postgres columns land + // as for a field declared `Field.text({ maxLength: n })` on a keyed + // column — the nearest authorable spelling to this shape; there is no + // `Field.string` (#12593). // // ⚠️ Scope, both halves load-bearing: // - KEYED only. A non-indexed `Field.text({ maxLength: 65000 })` stays