From a51eb0e7360f976cccc68d24e8f664023db6ec8e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 15:13:31 +0000 Subject: [PATCH 1/2] fix(driver-sql): correct two comments in createColumn citing 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. It does not: Field has 37 keys and none is `string`, FieldType.options (49 entries) omits it, and FieldSchema.safeParse({ type: 'string', ... }) fails at [type] -- all reproduced fresh on origin/main, plus git log -S confirming no commit ever added such a builder key to field.zod.ts. Corrected prose states only checkable facts: knex's bare table.string(name) (no length) is varchar(255); this branch calls it with a length instead (table.string(name, keyable), the field's own maxLength); and the nearest authorable spelling that reaches this exact varchar(n) shape is Field.text({ maxLength: n }) on a keyed column. Comment-only change -- no DDL, column width, or runtime behavior differs. --- .../sql-driver-field-string-comment-fix.md | 43 +++++++++++++++++++ packages/drivers/driver-sql/src/sql-driver.ts | 18 +++++--- 2 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 .changeset/sql-driver-field-string-comment-fix.md 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..f948fb5424 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: `Field` has 37 keys and none of them is `string`, + // `FieldType.options` (49 entries) 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 From 01f35ddca9cc51e4e58a12a17e570deffd3789df Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 15:40:12 +0000 Subject: [PATCH 2/2] fix(driver-sql): drop drifting key/entry counts from the corrected comment (#12593) PM review on PR #12740: "Field has 37 keys" and "FieldType.options (49 entries)" are measurements of a tree that moves -- this repo adds field types, so both numbers drift and would themselves go stale, which is this card's own defect at smaller scale. Keep the stable predicates (none of Field's keys is named 'string', FieldType.options omits it too, FieldSchema.safeParse fails at [type]) and drop the parenthetical counts; (#12593) already dates the measurement for anyone who wants to re-run it. Comment-only, as before: 0 non-comment lines added or removed. --- packages/drivers/driver-sql/src/sql-driver.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index f948fb5424..317f733202 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -14637,14 +14637,14 @@ export class SqlDriver implements IDataDriver { // 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. There is no `Field.string` - // builder: `Field` has 37 keys and none of them is `string`, - // `FieldType.options` (49 entries) 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). + // 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