refactor(driver-sql): the sql-driver codemod retires 15 facade-gate exception rows (knex calls, spelled return types, comments in parameters, accessors, computed defaults, the logger default) (#30147 phase 2a)

packages/drivers/driver-sql/src/sql-driver-class-head.ts

@@ -2,9 +2,10 @@
 
 /**
  * [#29520 codemod] The "Class head: dialect probes, logger, configuration"
- * section of `SqlDriver`: the bodies of `emissionClients`, `clientSpelling`,
- * `aliasIdentifierSql`, `dateBucketSql`, `knexConfigFor`, `withConnectBound`,
- * `withUtcSession`, `withMysqlCalendarDayAsText`,
+ * section of `SqlDriver`: the bodies of `logger`, `emissionClients`,
+ * `clientSpelling`, `insertReturnsStoredRows`, `aliasIdentifierSql`,
+ * `dateGranularityCapabilities`, `dateBucketSql`, `knexConfigFor`,
+ * `withConnectBound`, `withUtcSession`, `withMysqlCalendarDayAsText`,
  * `withPostgresCalendarDayAsText`, `installQueryTiming`, `assertSchemaMutable`,
  * `assertDialectEmits` moved out of `sql-driver.ts` whole by
  * `scripts/codemod-split-sql-driver.mjs`.
@@ -60,6 +61,8 @@ export interface ClassHeadHost {
   assertDialectEmits: SqlDriverClass['assertDialectEmits'];
   buildDateBucketExpr: SqlDriverClass['buildDateBucketExpr'];
   readonly config: SqlDriverClass['config'];
+  readonly isMysql: SqlDriverClass['isMysql'];
+  readonly isPostgres: SqlDriverClass['isPostgres'];
   readonly isSqlite: SqlDriverClass['isSqlite'];
   readonly knex: SqlDriverClass['knex'];
   openedEmptyInMemory: SqlDriverClass['openedEmptyInMemory'];
@@ -68,6 +71,14 @@ export interface ClassHeadHost {
 
 /** The bodies of the "Class head: dialect probes, logger, configuration" section of `SqlDriver`: a stateless facet, called by the class's stubs. */
 export class ClassHeadFacet {
+  /** The initializer of `SqlDriver#logger`; the field's declaration there is its contract. */
+  static logger(): SqlDriverClass['logger'] {
+    return {
+    warn: (msg, meta) => console.warn(msg, meta ?? ''),
+    error: (msg, meta) => console.error(msg, meta ?? ''),
+  };
+  }
+
   /** The body of `SqlDriver#emissionClients`; the method's docblock there is its contract. */
   static emissionClients(): string[] {
     return [
@@ -90,11 +101,37 @@ export class ClassHeadFacet {
     return typeof c === 'string' ? c : '';
   }
 
+  /** The body of `SqlDriver#insertReturnsStoredRows`; the method's docblock there is its contract. */
+  static insertReturnsStoredRows(this: Pick<ClassHeadHost, 'isPostgres' | 'isSqlite'>): boolean {
+    return this.isSqlite || this.isPostgres;
+  }
+
   /** The body of `SqlDriver#aliasIdentifierSql`; the method's docblock there is its contract. */
   static aliasIdentifierSql(this: Pick<ClassHeadHost, 'knex'>, alias: string): string {
     return (this.knex.client as { wrapIdentifier(value: string): string }).wrapIdentifier(String(alias));
   }
 
+  /** The body of `SqlDriver#dateGranularityCapabilities`; the method's docblock there is its contract. */
+  static dateGranularityCapabilities(this: Pick<ClassHeadHost, 'isMysql' | 'isPostgres' | 'isSqlite'>): Record<string, boolean> {
+    if (this.isPostgres) {
+      return { day: true, month: true, quarter: true, year: true, week: true };
+    }
+    if (this.isMysql) {
+      return { day: true, month: true, quarter: true, year: true, week: true };
+    }
+    if (this.isSqlite) {
+      // [#21595] All five, `week` included. The ISO week is computed by the
+      // Thursday rule through date modifiers (the `week` arm of
+      // `buildDateBucketExpr`), never `strftime('%V')` / `'%G'`: those arrived
+      // in SQLite 3.46, and libSQL (`driver-turso`) bundles an older engine.
+      // Measured: better-sqlite3 13.0.3 runs 3.53.4, `@libsql/client` 0.18.0
+      // runs 3.45.1 (where `%V` answers NULL), sql.js 1.14.2
+      // (`driver-sqlite-wasm`) runs 3.49.1. The expression runs on all three.
+      return { day: true, month: true, quarter: true, year: true, week: true };
+    }
+    return {};
+  }
+
   /** The body of `SqlDriver#dateBucketSql`; the method's docblock there is its contract. */
   static dateBucketSql(this: Pick<ClassHeadHost, 'buildDateBucketExpr' | 'knex'>, objectName: string, field: string, granularity: DateGranularityValue): string | null {
     const bucket = this.buildDateBucketExpr(field, granularity, objectName);

packages/drivers/driver-sql/src/sql-driver-column-creation.ts

@@ -3,8 +3,9 @@
 /**
  * [#29520 codemod] The "Column creation helper" section of `SqlDriver`: the
  * bodies of `declaredVarcharLength`, `keyableTextLength`,
- * `hashShadowColumnFor`, `createHashShadowUniqueIndex`,
- * `hashShadowColumnState`, `dropOrphanedHashShadowColumn`,
+ * `explainUnkeyableTextColumn`, `hashShadowColumnFor`,
+ * `createHashShadowUniqueIndex`, `hashShadowColumnState`,
+ * `dropOrphanedHashShadowColumn`, `explainHashShadowDuplicate`,
  * `decorateHashShadowDuplicate`, `hashShadowSourceColumns`,
  * `varcharPackLength`, `rowWidthProfile`, `schemaBytesPerChar`,
  * `explainRowSizeOverflow`, `rethrowWithRowSizeExplanation`, `jsonColumn` moved
@@ -55,11 +56,14 @@ import type { SqlDriver as SqlDriverClass } from './sql-driver.js';
  * assigns.
  */
 export interface ColumnCreationHost {
+  declaredFieldsFor: SqlDriverClass['declaredFieldsFor'];
   explainHashShadowDuplicate: SqlDriverClass['explainHashShadowDuplicate'];
   explainRowSizeOverflow: SqlDriverClass['explainRowSizeOverflow'];
   hashShadowColumnState: SqlDriverClass['hashShadowColumnState'];
+  hashShadowSourceColumns: SqlDriverClass['hashShadowSourceColumns'];
   readonly isMysql: SqlDriverClass['isMysql'];
   readonly isSqlite: SqlDriverClass['isSqlite'];
+  keyableTextLength: SqlDriverClass['keyableTextLength'];
   readonly knex: SqlDriverClass['knex'];
   logDurabilityFailure: SqlDriverClass['logDurabilityFailure'];
   readonly logger: SqlDriverClass['logger'];
@@ -89,6 +93,101 @@ export class ColumnCreationFacet {
     return n;
   }
 
+  /** The body of `SqlDriver#explainUnkeyableTextColumn`; the method's docblock there is its contract. */
+  static async explainUnkeyableTextColumn(this: Pick<ColumnCreationHost, 'declaredFieldsFor' | 'keyableTextLength' | 'knex'>, tableName: string, indexName: string, columns: string[], cause: unknown): Promise<string | null> {
+    const code = (cause as { code?: string } | undefined)?.code;
+    if (code !== 'ER_BLOB_KEY_WITHOUT_LENGTH' && code !== 'ER_TOO_LONG_KEY') return null;
+    let offenders: string[] = [];
+    try {
+      const info: Record<string, { type?: string }> = await this.knex(tableName).columnInfo();
+      offenders = columns.filter((c) => /text|blob/i.test(String(info?.[c]?.type ?? '')));
+    } catch {
+      // Introspection is a nicety here; the advice below holds without it.
+    }
+    const named =
+      offenders.length > 0
+        ? `Column(s) ${offenders.map((c) => `"${c}"`).join(', ')} are stored as TEXT`
+        : 'One or more of its key columns is stored as TEXT';
+
+    // Commit ebcc34e89: the SAME refusal has two causes, and the message below is true of
+    // only one of them. On CREATE the field really declares no usable bound, so
+    // "declare `maxLength`" is the fix. On an UPGRADE the bound IS declared —
+    // the additive sync never rewrites a column's type (#3728), so a column
+    // created before the declaration stays TEXT forever — and that same sentence
+    // tells the operator to do what they already did, once per boot, in
+    // production, which reads as the release they just deployed being broken.
+    //
+    // The discriminator is per COLUMN and needs BOTH halves, which is why it is
+    // computed here rather than inferred from the error code: a bounded
+    // declaration alone does not imply a stale column, because `ER_TOO_LONG_KEY`
+    // can refuse a COMPOSITE whose parts are each individually keyable (see
+    // {@link MAX_KEYABLE_VARCHAR_CHARS} — the bound is per KEY, not per column).
+    // So the branch fires only where the physical column is TEXT *and*
+    // {@link keyableTextLength} says a fresh create would have emitted
+    // `varchar(n)` for it. When `columnInfo()` fails, `offenders` is empty and
+    // this map stays empty, so the CREATE-path message stands — the honest
+    // reading, since without introspection the two causes are indistinguishable.
+    //
+    // `declaredFieldsFor` returns `undefined` for a table this driver does not
+    // own the column set of, and misses a SHARD table (registered under the base
+    // name). Both degrade to the message below rather than guessing.
+    const declaredFields = this.declaredFieldsFor(tableName);
+    const staleBounds = new Map<string, number>();
+    for (const c of offenders) {
+      const field = declaredFields?.[c];
+      const bound = field ? this.keyableTextLength(field) : null;
+      if (bound !== null) staleBounds.set(c, bound);
+    }
+
+    if (staleBounds.size > 0) {
+      const staleNamed = [...staleBounds]
+        .map(([c, n]) => `"${c}" (declares \`maxLength: ${n}\`)`)
+        .join(', ');
+      const [firstColumn, firstBound] = [...staleBounds][0];
+      // Named in full, deliberately. The mirror case in `schema-drift.ts` (the
+      // unbounded field over a stale `varchar`) already had to derive this, and
+      // the two MySQL `MODIFY` hazards it names are recorded twice more in this
+      // file, on the datetime and time widenings: MODIFY does NOT repeat a
+      // `NOT NULL`, and it DROPS a `DEFAULT` it does not restate. An operator
+      // who follows an abbreviated version of this remedy ends up worse off than
+      // the no-op they started with — a column that lost its default.
+      const others =
+        offenders.length > staleBounds.size
+          ? ` Column(s) ${offenders
+              .filter((c) => !staleBounds.has(c))
+              .map((c) => `"${c}"`)
+              .join(', ')} in the same key declare no usable bound and DO need \`maxLength\` ` +
+            `declared on the field.`
+          : '';
+      return (
+        `[sql-driver] cannot create index '${indexName}' on "${tableName}" — MySQL refuses a TEXT/BLOB ` +
+        `column in a key without a key length. Column(s) ${staleNamed} are stored as TEXT even though the ` +
+        `field ALREADY declares a usable \`maxLength\` — the column was created before that bound was ` +
+        `declared, and the additive sync never rewrites a column's type, so re-declaring \`maxLength\` ` +
+        `changes nothing here. REMEDY: convert the column(s) to varchar(n) by hand, with a backup ` +
+        `taken first, restating the FULL column definition on MySQL (MODIFY drops a NOT NULL or DEFAULT you ` +
+        `do not repeat) — e.g. ALTER TABLE \`${tableName}\` MODIFY \`${firstColumn}\` ` +
+        `varchar(${firstBound}) plus that column's existing NOT NULL / DEFAULT clauses, restated verbatim; ` +
+        `— and let the next boot create this index. ObjectStack does NOT rewrite the column for you: that ` +
+        `ALTER needs an exclusive metadata lock on the table, which makes it a destructive, hard-to-roll-back ` +
+        `action and a deliberate manual floor rather than a boot-time one.${others} The table exists but this ` +
+        `index does NOT, so any uniqueness it declared is currently unenforced. A prefix index is deliberately ` +
+        `not substituted: on a UNIQUE index it constrains the prefix rather than the value, and rejects two ` +
+        `different values that share one.`
+      );
+    }
+
+    return (
+      `[sql-driver] cannot create index '${indexName}' on "${tableName}" — MySQL refuses a TEXT/BLOB ` +
+      `column in a key without a key length. ${named} because the field declares no \`maxLength\` (or one ` +
+      `wider than ${SqlDriver.MAX_KEYABLE_VARCHAR_CHARS} characters, the most a utf8mb4 key part can hold). ` +
+      `Declare \`maxLength\` on the field(s) so the column is emitted as varchar(n) and can be keyed. ` +
+      `The table exists but this index does NOT, so any uniqueness it declared is currently ` +
+      `unenforced. A prefix index is deliberately not substituted: on a UNIQUE index it constrains the ` +
+      `prefix rather than the value, and rejects two different values that share one.`
+    );
+  }
+
   /** The body of `SqlDriver#hashShadowColumnFor`; the method's docblock there is its contract. */
   static hashShadowColumnFor(indexName: string): string {
     // Commit cd1348802: DELEGATES rather than re-deriving. The differ has to look for
@@ -277,6 +376,85 @@ export class ColumnCreationFacet {
     return true;
   }
 
+  /** The body of `SqlDriver#explainHashShadowDuplicate`; the method's docblock there is its contract. */
+  static async explainHashShadowDuplicate(this: Pick<ColumnCreationHost, 'hashShadowSourceColumns' | 'isMysql' | 'knex'>, tableName: string, values: Record<string, unknown>, cause: unknown): Promise<string | null> {
+    if (!this.isMysql) return null;
+    if ((cause as { code?: string } | undefined)?.code !== 'ER_DUP_ENTRY') return null;
+    const message = String((cause as { message?: string } | undefined)?.message ?? '');
+    // MySQL names the key as `table.index` (8.0) or `index` (5.7) — take the
+    // last path segment either way.
+    const keyed = /for key '([^']+)'/.exec(message);
+    if (!keyed) return null;
+    const indexName = keyed[1]!.split('.').pop()!;
+    const shadow = SqlDriver.hashShadowColumnFor(indexName);
+    let shadowExists = false;
+    try {
+      shadowExists = await this.knex.schema.hasColumn(tableName, shadow);
+    } catch {
+      return null;
+    }
+    if (!shadowExists) return null;
+    const sources = await this.hashShadowSourceColumns(tableName, indexName);
+    if (sources.length === 0) return null;
+    // Only the source columns the failing write actually supplied; a partial
+    // update cannot be re-selected on columns it never mentioned. A NULL-safe
+    // key part (commit df1c75c4b) is exempt: the enforced key COALESCEs an absent or
+    // NULL value into the '__global__' bucket, so its key part is knowable
+    // without the write mentioning the column.
+    if (
+      !sources.every(
+        (s) => s.nullSafe || Object.prototype.hasOwnProperty.call(values, s.column),
+      )
+    ) {
+      return null;
+    }
+    let existing = 0;
+    try {
+      // Re-select by the key the index ENFORCES, not by raw equality (commit df1c75c4b):
+      // a NULL-safe part compares through the same COALESCE the generation
+      // expression carries (NULL and absent both land in the global bucket),
+      // and plain parts use `<=>` — MySQL's NULL-safe equality; this whole
+      // route is inside `isMysql`. A bare `.where({col: null})` would compare
+      // `= NULL`, match nothing, and misreport a genuine NULL-organization
+      // duplicate as a HASH COLLISION.
+      let q = this.knex(tableName);
+      for (const s of sources) {
+        q = s.nullSafe
+          ? q.whereRaw(`COALESCE(??, '${GLOBAL_TENANT}') = COALESCE(?, '${GLOBAL_TENANT}')`, [
+              s.column,
+              (values[s.column] ?? null) as any,
+            ])
+          : q.whereRaw('?? <=> ?', [s.column, (values[s.column] ?? null) as any]);
+      }
+      const rows = await q.limit(1);
+      existing = rows.length;
+    } catch {
+      return null;
+    }
+    // Describe the key parts as enforced — NULL-safe parts in their COALESCE
+    // spelling — so the message names the actual constraint (commit df1c75c4b).
+    const described = sources
+      .map((s) => (s.nullSafe ? `COALESCE(${s.column}, '${GLOBAL_TENANT}')` : s.column))
+      .join(', ');
+    if (existing > 0) {
+      // The ordinary case: a real duplicate. Say so in the declared terms
+      // rather than leaving MySQL's binary digest as the only explanation.
+      return (
+        `[sql-driver] duplicate value for the UNIQUE constraint '${indexName}' on "${tableName}" ` +
+        `(${described}). The constraint is physically carried by a hash-shadow column, so the ` +
+        `server's own message quotes a binary digest instead of the value.`
+      );
+    }
+    return (
+      `[sql-driver] HASH COLLISION on the shadow-carried UNIQUE index '${indexName}' on "${tableName}" ` +
+      `(${described}): the write was rejected as a duplicate, but NO existing row carries these ` +
+      `values. Uniqueness on this index is enforced over a SHA-256 of them, so two different ` +
+      `values produced the same digest. This is expected at a rate near 10^-59 for a billion rows — if ` +
+      `you are reading this, please report it with the values above; the write itself is legitimate and ` +
+      `is being refused.`
+    );
+  }
+
   /** The body of `SqlDriver#decorateHashShadowDuplicate`; the method's docblock there is its contract. */
   static async decorateHashShadowDuplicate(this: Pick<ColumnCreationHost, 'explainHashShadowDuplicate' | 'logDurabilityFailure'>, tableName: string, values: Record<string, unknown>, error: unknown): Promise<unknown> {
     let explanation: string | null = null;