From 2d917648381fa47be6726e0b6558f2ed37ef705e Mon Sep 17 00:00:00 2001 From: os-steve Date: Tue, 18 Aug 2026 00:15:22 +0000 Subject: [PATCH 1/2] fix(metadata-protocol): compile the seed-tenancy migration statements for the connected dialect (#9381) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj --- .../migrate/duplicates.pre-repair.test.ts | 2 +- packages/metadata-protocol/package.json | 1 + packages/metadata-protocol/src/index.ts | 2 + .../seed-tenancy-backfill.live-mysql.test.ts | 217 +++++++++++++++ .../migrations/seed-tenancy-backfill.test.ts | 113 +++++++- .../src/migrations/seed-tenancy-backfill.ts | 246 ++++++++++++++---- packages/metadata-protocol/src/plugin.ts | 4 +- packages/runtime/src/app-plugin.ts | 4 +- ...nancy-autonumber-split.integration.test.ts | 16 +- pnpm-lock.yaml | 3 + 10 files changed, 544 insertions(+), 64 deletions(-) create mode 100644 packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts diff --git a/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts b/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts index 5689189352..2a892f1139 100644 --- a/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts +++ b/packages/cli/src/commands/migrate/duplicates.pre-repair.test.ts @@ -131,7 +131,7 @@ describe('#8928 — why it must run BEFORE the #8686 backfill', () => { ]); // The real repair, run exactly as a boot would run it. - const repair = await backfillSeedTenancy(exec); + const repair = await backfillSeedTenancy({ exec, client: 'better-sqlite3' }); expect(repair.status).toBe('applied'); const afterRepair = await report(); diff --git a/packages/metadata-protocol/package.json b/packages/metadata-protocol/package.json index 25f03b01f1..cebe49dea5 100644 --- a/packages/metadata-protocol/package.json +++ b/packages/metadata-protocol/package.json @@ -43,6 +43,7 @@ }, "devDependencies": { "@types/node": "^26.1.2", + "mysql2": "^3.23.1", "tsup": "^8.5.1", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/metadata-protocol/src/index.ts b/packages/metadata-protocol/src/index.ts index ce15630fb6..fda41d661f 100644 --- a/packages/metadata-protocol/src/index.ts +++ b/packages/metadata-protocol/src/index.ts @@ -79,6 +79,7 @@ export type { // twice (maintainer ruling 2026-08-15: contract option 1, stored data shape 2). export { backfillSeedTenancy, + resolveSeedTenancySeam, resolveSeedTenancyExec, normalizeRows, buildSequencesPresenceSql, @@ -93,6 +94,7 @@ export { ORGANIZATION_FIELD, ORGANIZATION_TABLE, } from './migrations/seed-tenancy-backfill.js'; +export type { SeedTenancySeam } from './migrations/seed-tenancy-backfill.js'; export type { SeedTenancyExec, SeedTenancyLogger, diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts new file mode 100644 index 0000000000..75bd638afc --- /dev/null +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.live-mysql.test.ts @@ -0,0 +1,217 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9381 — the seed-tenancy backfill's statements, RUN on a live MySQL. + * + * ## Why this file exists rather than one more text pin + * + * The defect it guards is invisible on the two dialects the rest of the suite + * runs on. `"identifier"` is correct ANSI on SQLite and PostgreSQL, so a + * SQLite-only or Postgres-only test passes with the bug fully present. MySQL is + * the only dialect that can fail — it does not run with `ANSI_QUOTES`, so `"x"` + * is a STRING LITERAL there — and the module's own header names MySQL as a + * supported backend. A migration whose statements cannot parse on a backend the + * module claims is declared ≠ enforced, and the reason it never surfaced is that + * every call site swallows a migration failure into a warning by design: on + * MySQL the symptom was a skipped repair in the boot log, not an error. + * + * ## Non-vacuity + * + * The suite ASSERTS the server is not running with `ANSI_QUOTES` before it + * asserts anything else. On a server that had it, these statements would parse + * with the bug present and a green run would mean nothing — the same + * vacuous-pass hole `live-dialect-matrix.testkit.ts` closes for the timezone + * axis, and the exact condition #9381's premise step had to rule out. + * + * ## Provisioning + * + * Needs `OS_TEST_MYSQL_URL` (same variable the driver-sql live matrix uses) and + * reports a named SKIP without one — never a silent pass. A runner that knows it + * provisioned the server sets `OS_EXPECT_LIVE_DIALECT_MATRIX=1`, which turns the + * missing URL into a failure so a dropped `env:` line cannot quietly return this + * seam to no coverage at all. + * + * Everything runs in its OWN database (`os_metadata_protocol_9381`), created on + * the spot, because two of the three tables this migration touches have fixed + * platform names (`_objectstack_sequences`, `sys_organization`) that other live + * suites also use. + */ + +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import mysql from 'mysql2/promise'; +import { + backfillSeedTenancy, + buildCollisionProbeSql, + buildCounterMergeSql, + buildGlobalCounterDeleteSql, + buildOrganizationProbeSql, + buildSequencesPresenceSql, + buildSplitProbeSql, + buildStampSql, + GLOBAL_TENANT, + SEQUENCES_TABLE, + type SeedTenancySeam, +} from './seed-tenancy-backfill.js'; + +const MYSQL_URL = process.env.OS_TEST_MYSQL_URL; +const EXPECT_LIVE = process.env.OS_EXPECT_LIVE_DIALECT_MATRIX === '1'; +const DB = 'os_metadata_protocol_9381'; +const OBJECT = 'os9381_case'; +const FIELD = 'case_number'; + +if (!MYSQL_URL && EXPECT_LIVE) { + describe('#9381 live MySQL', () => { + it('OS_TEST_MYSQL_URL must be set — this runner declared it provisioned a server', () => { + throw new Error( + 'OS_EXPECT_LIVE_DIALECT_MATRIX=1 without OS_TEST_MYSQL_URL: the live MySQL cell for ' + + 'the metadata-protocol migrations would have been skipped, returning #9381 to zero ' + + 'coverage on the only dialect that can exhibit it.', + ); + }); + }); +} + +describe.skipIf(!MYSQL_URL)('#9381 seed-tenancy backfill on a LIVE MySQL', () => { + let conn: mysql.Connection; + let seam: SeedTenancySeam; + + beforeAll(async () => { + const bootstrap = await mysql.createConnection(MYSQL_URL!); + await bootstrap.query(`CREATE DATABASE IF NOT EXISTS \`${DB}\``); + await bootstrap.end(); + + conn = await mysql.createConnection(`${MYSQL_URL}`); + await conn.query(`USE \`${DB}\``); + // The one session SET `SqlDriver` itself performs on a mysql connection. + await conn.query(`SET time_zone = '+00:00'`); + + seam = { + exec: (sql: string, params?: unknown[]) => conn.query(sql, params ?? []), + client: 'mysql2', + }; + }); + + afterAll(async () => { + if (!conn) return; + await conn.query(`DROP DATABASE IF EXISTS \`${DB}\``); + await conn.end(); + }); + + const seedFixture = async (): Promise => { + await conn.query(`DROP TABLE IF EXISTS \`${SEQUENCES_TABLE}\``); + await conn.query(`DROP TABLE IF EXISTS \`${OBJECT}\``); + await conn.query(`DROP TABLE IF EXISTS \`sys_organization\``); + // Column names spelled the way the driver's own `createSequencesTable` + // spells them; `last_value` is quoted here for the same reason the migration + // has to quote it (see the reserved-word assertion below). + await conn.query( + `CREATE TABLE \`${SEQUENCES_TABLE}\` (` + + '`key_hash` VARCHAR(64), `object` VARCHAR(64), `tenant_id` VARCHAR(64), ' + + '`field` VARCHAR(64), `scope` VARCHAR(255) NOT NULL DEFAULT \'\', ' + + '`last_value` INT, `updated_at` DATETIME(3))', + ); + await conn.query( + `CREATE TABLE \`${OBJECT}\` (` + + '`id` VARCHAR(64), `case_number` VARCHAR(64), `organization_id` VARCHAR(64))', + ); + await conn.query('CREATE TABLE `sys_organization` (`id` VARCHAR(64))'); + await conn.query("INSERT INTO `sys_organization` (`id`) VALUES ('org_live')"); + await conn.query( + `INSERT INTO \`${SEQUENCES_TABLE}\` (\`key_hash\`, \`object\`, \`tenant_id\`, \`field\`, \`last_value\`) ` + + `VALUES ('h_global', '${OBJECT}', '${GLOBAL_TENANT}', '${FIELD}', 38), ` + + `('h_org', '${OBJECT}', 'org_live', '${FIELD}', 4)`, + ); + // The card's own repro: seeded rows carry NULL, API rows carry the org, and + // CASE-00001/2 were minted on BOTH sides. + await conn.query( + `INSERT INTO \`${OBJECT}\` (\`id\`, \`case_number\`, \`organization_id\`) VALUES ` + + "('s1','CASE-00001',NULL),('s2','CASE-00002',NULL),('s3','CASE-00003',NULL)," + + "('a1','CASE-00001','org_live'),('a2','CASE-00002','org_live')", + ); + }; + + it('the server is NOT running with ANSI_QUOTES — without this the run proves nothing', async () => { + const [rows] = await conn.query('SELECT @@session.sql_mode AS sql_mode, VERSION() AS version'); + const mode = String((rows as Array<{ sql_mode: string }>)[0]!.sql_mode); + // Printed so the CI log carries the measurement, not just the verdict. + // eslint-disable-next-line no-console + console.log( + `[#9381] live MySQL ${(rows as Array<{ version: string }>)[0]!.version} sql_mode=${mode}`, + ); + expect(mode).not.toContain('ANSI_QUOTES'); + }); + + it('every statement the migration builds PARSES and runs on MySQL', async () => { + await seedFixture(); + const client = 'mysql2'; + const statements: Array<[string, string, unknown[]]> = [ + ['presence probe', buildSequencesPresenceSql(client), []], + ['split probe', buildSplitProbeSql(client), [GLOBAL_TENANT, GLOBAL_TENANT]], + ['organization probe', buildOrganizationProbeSql(client), []], + ['collision probe', buildCollisionProbeSql(OBJECT, FIELD, client), []], + ['stamp', buildStampSql(OBJECT, [FIELD], client), ['org_live']], + ['counter merge', buildCounterMergeSql(client), [38, OBJECT, FIELD, 'org_live']], + ['global counter delete', buildGlobalCounterDeleteSql(client), [OBJECT, FIELD, GLOBAL_TENANT]], + ]; + for (const [label, sql, params] of statements) { + // A failure here names the statement AND its text — the parse error alone + // does not say which builder produced it. + await expect( + conn.query(sql, params), + `${label} must run on MySQL — statement: ${sql}`, + ).resolves.toBeDefined(); + } + }); + + it('a multi-autonumber object stamps with one derived table per guard', async () => { + await seedFixture(); + await conn.query(`ALTER TABLE \`${OBJECT}\` ADD COLUMN \`ticket_no\` VARCHAR(64)`); + // Two guards in ONE statement: a repeated derived-table alias would be + // ER_NONUNIQ_TABLE, and the un-wrapped form ER_UPDATE_TABLE_USED. + await expect( + conn.query(buildStampSql(OBJECT, [FIELD, 'ticket_no'], 'mysql2'), ['org_live']), + ).resolves.toBeDefined(); + }); + + it('repairs the split end to end, and reports the already-minted duplicates', async () => { + await seedFixture(); + const warnings: string[] = []; + const result = await backfillSeedTenancy(seam, { + warn: (m: string) => warnings.push(m), + info: () => {}, + } as never); + + expect(result.status).toBe('applied'); + expect(result.organizationId).toBe('org_live'); + expect(result.splits).toEqual([ + { object: OBJECT, field: FIELD, globalLastValue: 38, organizationLastValue: 4 }, + ]); + // Reported, never renumbered — the two values minted on both sides. + expect(result.collisions.map((c) => c.value).sort()).toEqual(['CASE-00001', 'CASE-00002']); + + // The movable row moved; the two colliding rows kept their NULL. + const [rows] = await conn.query( + `SELECT \`id\`, \`organization_id\` FROM \`${OBJECT}\` ORDER BY \`id\``, + ); + const byId = Object.fromEntries( + (rows as Array<{ id: string; organization_id: string | null }>).map((r) => [ + r.id, + r.organization_id, + ]), + ); + expect(byId.s3).toBe('org_live'); + expect(byId.s1).toBeNull(); + expect(byId.s2).toBeNull(); + + // The counters were merged at max(last_value) and the `__global__` row retired. + const [counters] = await conn.query( + `SELECT \`tenant_id\`, \`last_value\` FROM \`${SEQUENCES_TABLE}\` ORDER BY \`tenant_id\``, + ); + expect(counters).toEqual([{ tenant_id: 'org_live', last_value: 38 }]); + }); + + it('is idempotent — a second run finds no split', async () => { + const second = await backfillSeedTenancy(seam); + expect(second.status).toBe('no-split'); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts index 755d526a54..67d46a1798 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.test.ts @@ -24,6 +24,8 @@ import { describe, it, expect } from 'vitest'; import { normalizeRows, + resolveSeedTenancySeam, + buildOrganizationProbeSql, buildSplitProbeSql, buildCollisionProbeSql, buildStampSql, @@ -76,6 +78,7 @@ describe('#8686 SQL builders', () => { // repair would decline seconds before the first duplicate is minted. expect(sql).toContain('LEFT JOIN'); expect(sql).not.toMatch(/\bFROM\s+"_objectstack_sequences"\s+g\s+JOIN\b/); + expect(sql).toContain('"_objectstack_sequences" g LEFT JOIN'); expect(sql).toContain(SEQUENCES_TABLE); }); @@ -86,13 +89,18 @@ describe('#8686 SQL builders', () => { // Without this the whole UPDATE is refused by the partitioned unique index on // any install that already minted duplicates, rolling back even the rows that // had no conflict. - expect(sql).toContain('"case_number" NOT IN (SELECT "case_number" FROM "crm_case"'); + expect(sql).toContain('"case_number" NOT IN (SELECT'); + expect(sql).toContain('FROM "crm_case"'); }); it('the stamp guards EVERY split field of a multi-autonumber object', () => { const sql = buildStampSql('crm_case', ['case_number', 'ticket_no']); expect(sql).toContain('"case_number" NOT IN'); expect(sql).toContain('"ticket_no" NOT IN'); + // One derived table per guard: MySQL rejects a repeated derived-table alias + // in one statement, and the guards are all in one statement. + expect(sql).toContain('AS "taken_0"'); + expect(sql).toContain('AS "taken_1"'); }); it('the collision probe asks for values held on BOTH sides of the split', () => { @@ -105,9 +113,11 @@ describe('#8686 SQL builders', () => { it('counter statements bind every value and name no literal tenant', () => { // The tenant id reaching these is an organization id read from the database. // It is bound, never interpolated. - expect(buildCounterMergeSql()).toContain('SET last_value = ?'); - expect(buildCounterMergeSql()).toContain('WHERE object = ? AND field = ? AND tenant_id = ?'); - expect(buildGlobalCounterDeleteSql()).toContain('WHERE object = ? AND field = ? AND tenant_id = ?'); + expect(buildCounterMergeSql()).toContain('SET "last_value" = ?'); + expect(buildCounterMergeSql()).toContain('WHERE "object" = ? AND "field" = ? AND "tenant_id" = ?'); + expect(buildGlobalCounterDeleteSql()).toContain( + 'WHERE "object" = ? AND "field" = ? AND "tenant_id" = ?', + ); expect(buildCounterMergeSql()).not.toContain(GLOBAL_TENANT); expect(buildGlobalCounterDeleteSql()).not.toContain(GLOBAL_TENANT); }); @@ -148,3 +158,98 @@ describe('#8686 identifier gate', () => { expect(() => buildCollisionProbeSql('_odd_but_legal', 'f1')).not.toThrow(); }); }); + +describe('#9381 dialect-aware statement text', () => { + // MySQL does not run with `ANSI_QUOTES` — measured on a live MySQL 8.0.46, + // whose `sql_mode` is + // ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE, + // ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION, and nothing in + // `driver-sql` sets one. `"x"` is therefore a STRING LITERAL there, and every + // statement this module builds was an `ER_PARSE_ERROR` before this fix. + // + // The live counterpart of this suite is + // `seed-tenancy-backfill.live-mysql.test.ts`, which RUNS the statements on a + // real server. This one pins the text so the seam is covered on every + // machine, with or without a MySQL. + const BACKTICK = String.fromCharCode(96); + const bt = (name: string) => `${BACKTICK}${name}${BACKTICK}`; + + for (const client of ['mysql', 'mysql2']) { + describe(`client=${client}`, () => { + it('quotes every table and column with backticks, never with double quotes', () => { + const statements = [ + buildSequencesPresenceSql(client), + buildSplitProbeSql(client), + buildOrganizationProbeSql(client), + buildCollisionProbeSql('crm_case', 'case_number', client), + buildStampSql('crm_case', ['case_number'], client), + buildCounterMergeSql(client), + buildGlobalCounterDeleteSql(client), + ]; + for (const sql of statements) { + expect(sql).not.toContain('"'); + expect(sql).toContain(BACKTICK); + } + expect(buildSequencesPresenceSql(client)).toContain(bt(SEQUENCES_TABLE)); + expect(buildStampSql('crm_case', ['case_number'], client)).toContain( + `UPDATE ${bt('crm_case')} SET ${bt(ORGANIZATION_FIELD)} = ?`, + ); + }); + + it('quotes `last_value` — a RESERVED word on MySQL 8.0 — wherever it is unqualified', () => { + // `LAST_VALUE()` is a window function there, so a bare `last_value` is a + // parse error even when the table name is spelled correctly. Measured. + const merge = buildCounterMergeSql(client); + expect(merge).toContain(`SET ${bt('last_value')} = ?`); + expect(merge).not.toMatch(/(? { + // ER_UPDATE_TABLE_USED (1093): "You can't specify target table 'crm_case' + // for update in FROM clause". Not a quoting problem — the statement stays + // refused after the identifiers are spelled the MySQL way. + const sql = buildStampSql('crm_case', ['case_number'], client); + expect(sql).toContain(`AS ${bt('taken_0')})`); + expect(sql).not.toMatch( + new RegExp(`NOT IN \\(SELECT ${BACKTICK}case_number${BACKTICK} FROM`), + ); + }); + }); + } + + for (const client of ['pg', 'better-sqlite3', 'sqlite3', undefined]) { + it(`keeps the ANSI spelling for client=${String(client)}`, () => { + const statements = [ + buildSequencesPresenceSql(client), + buildSplitProbeSql(client), + buildOrganizationProbeSql(client), + buildCollisionProbeSql('crm_case', 'case_number', client), + buildStampSql('crm_case', ['case_number'], client), + buildCounterMergeSql(client), + buildGlobalCounterDeleteSql(client), + ]; + for (const sql of statements) { + expect(sql).not.toContain(BACKTICK); + expect(sql).toContain('"'); + } + }); + } + + it('the seam carries the dialect, so a caller cannot drop it', () => { + // The structural half of the fix: `backfillSeedTenancy` takes the pair, and + // the resolver is what produces the pair. A driver that reports no client + // still resolves — ANSI is the right default for the two dialects that want + // it, and for a MySQL running with ANSI_QUOTES. + const driver = { + execute: async () => [], + config: { client: 'mysql2' }, + }; + const seam = resolveSeedTenancySeam({ driver }); + expect(seam?.client).toBe('mysql2'); + expect(typeof seam?.exec).toBe('function'); + + const clientless = resolveSeedTenancySeam({ driver: { execute: async () => [] } }); + expect(clientless?.client).toBeUndefined(); + expect(resolveSeedTenancySeam({})).toBeUndefined(); + }); +}); diff --git a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts index 11fdb18e8a..bda9ceaaff 100644 --- a/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts +++ b/packages/metadata-protocol/src/migrations/seed-tenancy-backfill.ts @@ -67,6 +67,27 @@ * (Same paradigm as `sys_setting`'s identity-index migration, which hands * back the duplicate list rather than applying a keep-one rule.) * + * ## Dialects (#9381) + * + * Every statement here is compiled for the dialect the seam is connected to. + * That is not decoration: MySQL does not run with `ANSI_QUOTES`, so the ANSI + * `"identifier"` this module used to emit unconditionally is a STRING LITERAL + * there and every one of the five statements failed to parse — measured on a + * live MySQL 8.0.46, where all seven statements returned `ER_PARSE_ERROR` + * before the fix and all seven run after it. Two MySQL-specific traps beyond the + * quote character, both measured on the same server: + * + * - `last_value` is RESERVED on MySQL 8.0 (`LAST_VALUE()`), so the COLUMN + * names are quoted too, not just the tables — an unqualified `last_value` + * is a parse error even when the table is spelled correctly; + * - `UPDATE t … (SELECT … FROM t)` is refused outright with + * `ER_UPDATE_TABLE_USED` (1093), so the stamp's exclusion sub-SELECTs go + * through a derived table (see {@link buildStampSql}). + * + * The failure was invisible because a migration must never fail a boot: every + * call site catches and warns, so the symptom on MySQL was a skipped repair in + * the log, not an error — declared (the module claims MySQL) ≠ enforced. + * * ## Why `max(last_value)` and not `max(data)` * * The ruling says `max(last_value)`, and the two differ in the direction that @@ -165,7 +186,39 @@ export interface SeedTenancyBackfillResult { } /** - * Resolve a row-returning raw-SQL seam. + * The raw-SQL seam PLUS the dialect it speaks — resolved together, from one + * driver, on purpose (#9381). + * + * Every statement in this module interpolates identifiers, and the three + * supported dialects do not spell an identifier the same way. An `exec` handed + * around without the dialect beside it is an invitation to compile ANSI SQL for + * a server that does not parse it — which is exactly the defect #9381 records: + * `"x"` is an identifier on SQLite and PostgreSQL, and a STRING LITERAL on + * MySQL, whose `sql_mode` does not include `ANSI_QUOTES` (measured on MySQL + * 8.0.46: `ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,` + * `ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION` — no `ANSI_QUOTES`, and + * nothing in `driver-sql` sets one; its only session `SET` is `time_zone`). + * + * So the two travel as ONE value: a caller cannot obtain the exec without also + * obtaining the client name, and {@link backfillSeedTenancy} takes the pair + * rather than a bare function. That is the structural half of the fix — the + * quoting helper alone would still let a future caller lose the dialect. + */ +export interface SeedTenancySeam { + /** The row-returning raw-SQL seam. */ + exec: SeedTenancyExec; + /** + * The knex client name of the driver behind {@link exec} — `'mysql2'`, + * `'mysql'`, `'pg'`, `'better-sqlite3'`, `'sqlite3'`. `undefined` on a host + * that exposes no config, where the ANSI spelling is the only sane default + * (it is what SQLite and PostgreSQL want, and what a MySQL running with + * `ANSI_QUOTES` would want too). + */ + client?: string; +} + +/** + * Resolve a row-returning raw-SQL seam, together with the dialect it speaks. * * `execute` is probed BEFORE `raw` — the opposite order to * `resolveIndexExecForTable` — because `execute(sql, params)` carries bound @@ -176,8 +229,13 @@ export interface SeedTenancyBackfillResult { * `_objectstack_sequences`, which is driver-private and not a registered object, * so there is nothing to ask about. The engine's own default driver is the one * that owns it. + * + * The client name is read from the SAME driver object the exec came from + * (`driver.config.client`, the field `SqlDriver.isMysql` itself reads, and the + * same source `os migrate duplicates` uses for its own probes), so the dialect + * can never describe a different connection than the one the statements run on. */ -export function resolveSeedTenancyExec(engine: unknown): SeedTenancyExec | undefined { +export function resolveSeedTenancySeam(engine: unknown): SeedTenancySeam | undefined { const engineAny = engine as any; const attempt = (fn: () => unknown): any => { try { @@ -201,10 +259,48 @@ export function resolveSeedTenancyExec(engine: unknown): SeedTenancyExec | undef } } if (!canRun(driver)) return undefined; + const client = resolveClientName(driver); if (typeof driver.execute === 'function') { - return (sql: string, params?: unknown[]) => driver.execute(sql, params ?? []); + return { exec: (sql: string, params?: unknown[]) => driver.execute(sql, params ?? []), client }; } - return (sql: string) => driver.raw(sql); + return { exec: (sql: string) => driver.raw(sql), client }; +} + +/** + * The knex client name of a driver, best-effort. + * + * `SqlDriver.config` is `protected` in TypeScript and an ordinary property at + * runtime; this module holds the engine as `unknown` and reads it the same way + * `os migrate duplicates` does. The knex instance is the fallback for a driver + * that keeps its config elsewhere. Anything unreadable is `undefined`, which + * means "quote the ANSI way" — today's behaviour, unchanged. + */ +function resolveClientName(driver: any): string | undefined { + const read = (fn: () => unknown): string | undefined => { + try { + const v = fn(); + return typeof v === 'string' && v.length > 0 ? v : undefined; + } catch { + return undefined; + } + }; + return ( + read(() => driver?.config?.client) ?? + read(() => driver?.knex?.client?.config?.client) ?? + read(() => driver?.knex?.context?.client?.config?.client) + ); +} + +/** + * The exec half alone, for callers that resolve the dialect themselves. + * + * `os migrate duplicates` is the one: it reads `stack.driver.config.client` for + * its OWN probes and only borrows this resolver's driver-walk. Anything that + * compiles statements from THIS module must take {@link resolveSeedTenancySeam} + * instead, so the dialect cannot be dropped on the way. + */ +export function resolveSeedTenancyExec(engine: unknown): SeedTenancyExec | undefined { + return resolveSeedTenancySeam(engine)?.exec; } /** @@ -245,14 +341,28 @@ function isSafeIdentifier(name: unknown): name is string { return typeof name === 'string' && /^[A-Za-z_][A-Za-z0-9_]*$/.test(name); } -/** Quote an identifier for the target dialect. */ -function quoteIdent(name: string): string { - return `"${name}"`; +/** + * Quote an identifier for the dialect actually connected (#9381). + * + * MySQL does not run with `ANSI_QUOTES`, so `"x"` there is a STRING LITERAL and + * not an identifier — measured on a live MySQL 8.0.46, where every statement + * this module builds failed with `ER_PARSE_ERROR` before this fix. One quoting + * style for every dialect cannot run on all three. + * + * The same shape, deliberately, as `quoteIdent` in the CLI's + * `migrate/duplicates.ts` (#8928), which took this route first for its own + * probes. Two copies is two copies; unifying them is a separate decision (see + * the PR discussion on #9381) and NOT a drive-by of this fix. + */ +function quoteIdent(name: string, client?: string): string { + const c = String(client ?? '').toLowerCase(); + if (c === 'mysql' || c === 'mysql2') return `\`${name.replace(/`/g, '``')}\``; + return `"${name.replace(/"/g, '""')}"`; } /** Probe statement: does the counter table exist at all? */ -export function buildSequencesPresenceSql(): string { - return `SELECT tenant_id FROM ${quoteIdent(SEQUENCES_TABLE)} WHERE 1 = 0`; +export function buildSequencesPresenceSql(client?: string): string { + return `SELECT ${quoteIdent('tenant_id', client)} FROM ${quoteIdent(SEQUENCES_TABLE, client)} WHERE 1 = 0`; } /** @@ -275,14 +385,20 @@ export function buildSequencesPresenceSql(): string { * "close the split before it can do damage", which is the ruled contract * (Option 1) rather than merely its clean-up. */ -export function buildSplitProbeSql(): string { - const t = quoteIdent(SEQUENCES_TABLE); +export function buildSplitProbeSql(client?: string): string { + const t = quoteIdent(SEQUENCES_TABLE, client); + const q = (name: string) => quoteIdent(name, client); + // `last_value` is a RESERVED word on MySQL 8.0 (the `LAST_VALUE()` window + // function), so even the bare column name is an `ER_PARSE_ERROR` there — + // measured. Every identifier is quoted, not just the table. return ( - `SELECT g.object AS object, g.field AS field, ` + - `g.last_value AS global_last_value, o.last_value AS organization_last_value ` + + `SELECT g.${q('object')} AS ${q('object')}, g.${q('field')} AS ${q('field')}, ` + + `g.${q('last_value')} AS ${q('global_last_value')}, ` + + `o.${q('last_value')} AS ${q('organization_last_value')} ` + `FROM ${t} g LEFT JOIN ${t} o ` + - `ON g.object = o.object AND g.field = o.field AND o.tenant_id <> ? ` + - `WHERE g.tenant_id = ?` + `ON g.${q('object')} = o.${q('object')} AND g.${q('field')} = o.${q('field')} ` + + `AND o.${q('tenant_id')} <> ? ` + + `WHERE g.${q('tenant_id')} = ?` ); } @@ -302,8 +418,8 @@ export function buildSplitProbeSql(): string { const PLATFORM_NAMESPACE = /^(sys_|cloud_|ai_)/; /** The organizations the install has, capped — the single-tenant guard reads this. */ -export function buildOrganizationProbeSql(): string { - return `SELECT id FROM ${quoteIdent(ORGANIZATION_TABLE)}`; +export function buildOrganizationProbeSql(client?: string): string { + return `SELECT ${quoteIdent('id', client)} FROM ${quoteIdent(ORGANIZATION_TABLE, client)}`; } /** @@ -314,15 +430,16 @@ export function buildOrganizationProbeSql(): string { * handed the same statement the migration ran, so the report does not depend on * trusting this module's own summary. */ -export function buildCollisionProbeSql(object: string, field: string): string { +export function buildCollisionProbeSql(object: string, field: string, client?: string): string { if (!isSafeIdentifier(object) || !isSafeIdentifier(field)) { throw new Error(`unsafe identifier in collision probe: ${object}.${field}`); } - const t = quoteIdent(object); - const f = quoteIdent(field); - const org = quoteIdent(ORGANIZATION_FIELD); + const t = quoteIdent(object, client); + const f = quoteIdent(field, client); + const org = quoteIdent(ORGANIZATION_FIELD, client); return ( - `SELECT ${f} AS value, COUNT(*) AS rows_holding FROM ${t} ` + + `SELECT ${f} AS ${quoteIdent('value', client)}, ` + + `COUNT(*) AS ${quoteIdent('rows_holding', client)} FROM ${t} ` + `WHERE ${f} IN (SELECT ${f} FROM ${t} WHERE ${org} IS NULL) ` + `AND ${f} IN (SELECT ${f} FROM ${t} WHERE ${org} IS NOT NULL) ` + `GROUP BY ${f} ORDER BY ${f}` @@ -354,41 +471,71 @@ export function buildCollisionProbeSql(object: string, field: string): string { * * `fields` is every split field of this object: a row is unmovable if it * collides on ANY of them. + * + * ## Why each guard sub-SELECT is wrapped in a derived table (#9381) + * + * MySQL refuses `UPDATE t … WHERE c NOT IN (SELECT … FROM t)` outright: + * `ER_UPDATE_TABLE_USED` (1093) — "You can't specify target table 't' for update + * in FROM clause". Measured on a live MySQL 8.0.46, and it is NOT a quoting + * problem: the statement stays refused after the identifiers are spelled the + * MySQL way. Selecting the same rows through a derived table makes MySQL + * materialize them first, which is exactly what the restriction asks for, and + * the form is plain ANSI — SQLite and PostgreSQL run it unchanged (both + * measured). The alias is per-field so a multi-autonumber object does not + * declare the same derived table twice in one statement. */ -export function buildStampSql(object: string, fields: string[]): string { +export function buildStampSql(object: string, fields: string[], client?: string): string { if (!isSafeIdentifier(object)) { throw new Error(`unsafe identifier in stamp: ${object}`); } - const t = quoteIdent(object); - const org = quoteIdent(ORGANIZATION_FIELD); - const guards = fields.map((field) => { + const t = quoteIdent(object, client); + const org = quoteIdent(ORGANIZATION_FIELD, client); + const guards = fields.map((field, i) => { if (!isSafeIdentifier(field)) { throw new Error(`unsafe identifier in stamp: ${object}.${field}`); } - const f = quoteIdent(field); - return ` AND ${f} NOT IN (SELECT ${f} FROM ${t} WHERE ${org} IS NOT NULL AND ${f} IS NOT NULL)`; + const f = quoteIdent(field, client); + // The derived table's own column and alias are this module's literals, not + // data — but they are quoted like everything else so one rule covers the + // whole statement. + const taken = quoteIdent(`taken_${i}`, client); + const v = quoteIdent('v', client); + return ( + ` AND ${f} NOT IN (SELECT ${taken}.${v} FROM ` + + `(SELECT ${f} AS ${v} FROM ${t} WHERE ${org} IS NOT NULL AND ${f} IS NOT NULL) AS ${taken})` + ); }); return `UPDATE ${t} SET ${org} = ? WHERE ${org} IS NULL${guards.join('')}`; } /** Raise the organization-scoped counter to the merged high-water mark. */ -export function buildCounterMergeSql(): string { +export function buildCounterMergeSql(client?: string): string { + const q = (name: string) => quoteIdent(name, client); + // `last_value` unqualified is an `ER_PARSE_ERROR` on MySQL 8.0 — it is the + // reserved `LAST_VALUE()` window function there — so the columns are quoted + // and not only the table (measured). return ( - `UPDATE ${quoteIdent(SEQUENCES_TABLE)} SET last_value = ?, updated_at = CURRENT_TIMESTAMP ` + - `WHERE object = ? AND field = ? AND tenant_id = ?` + `UPDATE ${q(SEQUENCES_TABLE)} SET ${q('last_value')} = ?, ` + + `${q('updated_at')} = CURRENT_TIMESTAMP ` + + `WHERE ${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} = ?` ); } /** Retire the `__global__` counter once its value has been merged. */ -export function buildGlobalCounterDeleteSql(): string { - return `DELETE FROM ${quoteIdent(SEQUENCES_TABLE)} WHERE object = ? AND field = ? AND tenant_id = ?`; +export function buildGlobalCounterDeleteSql(client?: string): string { + const q = (name: string) => quoteIdent(name, client); + return ( + `DELETE FROM ${q(SEQUENCES_TABLE)} ` + + `WHERE ${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} = ?` + ); } /** The organization-scoped counter rows for one split object/field. */ -function buildOrgCounterProbeSql(): string { +function buildOrgCounterProbeSql(client?: string): string { + const q = (name: string) => quoteIdent(name, client); return ( - `SELECT tenant_id, last_value FROM ${quoteIdent(SEQUENCES_TABLE)} ` + - `WHERE object = ? AND field = ? AND tenant_id <> ?` + `SELECT ${q('tenant_id')}, ${q('last_value')} FROM ${q(SEQUENCES_TABLE)} ` + + `WHERE ${q('object')} = ? AND ${q('field')} = ? AND ${q('tenant_id')} <> ?` ); } @@ -407,18 +554,23 @@ function toNumber(value: unknown): number { * * Idempotent: once the rows carry the organization and the `__global__` counter * is gone, the split probe finds nothing and a re-run is `no-split`. + * + * Takes the SEAM — exec plus dialect — rather than a bare exec (#9381). Every + * statement below is compiled for `seam.client`, so the pair has to arrive + * together or the statements would be spelled for a dialect nobody checked. */ export async function backfillSeedTenancy( - exec: SeedTenancyExec | undefined, + seam: SeedTenancySeam | undefined, logger?: SeedTenancyLogger, ): Promise { const empty = { splits: [], collisions: [], objectsStamped: 0 }; - if (!exec) return { status: 'no-driver', ...empty }; + if (!seam?.exec) return { status: 'no-driver', ...empty }; + const { exec, client } = seam; // 1. Is there a counter table at all? Absent on a memory engine, and on any // install that has never allocated an autonumber. try { - await exec(buildSequencesPresenceSql()); + await exec(buildSequencesPresenceSql(client)); } catch { return { status: 'absent', ...empty }; } @@ -429,7 +581,7 @@ export async function backfillSeedTenancy( // loudly, because reaching it means a real defect is present. let splits: SeedTenancySplit[]; try { - const rows = normalizeRows(await exec(buildSplitProbeSql(), [GLOBAL_TENANT, GLOBAL_TENANT])); + const rows = normalizeRows(await exec(buildSplitProbeSql(client), [GLOBAL_TENANT, GLOBAL_TENANT])); splits = rows .filter((r) => isSafeIdentifier(r.object) && isSafeIdentifier(r.field)) // Platform seeds stay global — the loader's own rule, see PLATFORM_NAMESPACE. @@ -473,7 +625,7 @@ export async function backfillSeedTenancy( // 4. Exactly one organization, or there is nothing derivable to adopt. let organizationIds: string[] = []; try { - organizationIds = normalizeRows(await exec(buildOrganizationProbeSql())) + organizationIds = normalizeRows(await exec(buildOrganizationProbeSql(client))) .map((r) => (r.id == null ? '' : String(r.id))) .filter((id) => id.length > 0); } catch { @@ -501,7 +653,7 @@ export async function backfillSeedTenancy( const collisions: SeedTenancyCollision[] = []; for (const split of splits) { try { - const rows = normalizeRows(await exec(buildCollisionProbeSql(split.object, split.field))); + const rows = normalizeRows(await exec(buildCollisionProbeSql(split.object, split.field, client))); for (const r of rows) { if (r.value == null) continue; collisions.push({ @@ -515,7 +667,7 @@ export async function backfillSeedTenancy( logger?.warn?.( `[metadata-protocol] could not list already-minted duplicates for ${split.object}.${split.field} ` + `(#8686) — the backfill continues; verify manually with: ` + - `${buildCollisionProbeSql(split.object, split.field)}`, + `${buildCollisionProbeSql(split.object, split.field, client)}`, { error: (e as Error).message }, ); } @@ -534,7 +686,7 @@ export async function backfillSeedTenancy( const stampFailures: string[] = []; for (const [object, fields] of fieldsByObject) { try { - await exec(buildStampSql(object, fields), [organizationId]); + await exec(buildStampSql(object, fields, client), [organizationId]); objectsStamped += 1; } catch (e) { stampFailures.push(object); @@ -555,7 +707,7 @@ export async function backfillSeedTenancy( if (stampFailures.includes(split.object)) continue; try { const orgRows = normalizeRows( - await exec(buildOrgCounterProbeSql(), [split.object, split.field, GLOBAL_TENANT]), + await exec(buildOrgCounterProbeSql(client), [split.object, split.field, GLOBAL_TENANT]), ); for (const row of orgRows) { const tenantId = row.tenant_id == null ? '' : String(row.tenant_id); @@ -563,7 +715,7 @@ export async function backfillSeedTenancy( // The ruling's merge rule: the greater of the two COUNTERS, never the // data max — a counter is allowed to sit ahead of its rows. const merged = Math.max(split.globalLastValue, toNumber(row.last_value)); - await exec(buildCounterMergeSql(), [merged, split.object, split.field, tenantId]); + await exec(buildCounterMergeSql(client), [merged, split.object, split.field, tenantId]); } // Retire the `__global__` counter last. // @@ -579,7 +731,7 @@ export async function backfillSeedTenancy( // row, scans `scanMaxNumericTail` SCOPED TO THE RESOLVED TENANT — which, the // stamp above having just run, now includes the adopted seed rows — and // starts at that max + 1. One tested code path, no duplicated hashing. - await exec(buildGlobalCounterDeleteSql(), [split.object, split.field, GLOBAL_TENANT]); + await exec(buildGlobalCounterDeleteSql(client), [split.object, split.field, GLOBAL_TENANT]); } catch (e) { logger?.warn?.( `[metadata-protocol] seed tenancy backfill could not merge the counter for ` + diff --git a/packages/metadata-protocol/src/plugin.ts b/packages/metadata-protocol/src/plugin.ts index 516b061553..da3e9d9a0d 100644 --- a/packages/metadata-protocol/src/plugin.ts +++ b/packages/metadata-protocol/src/plugin.ts @@ -41,7 +41,7 @@ import { } from './migrations/sys-setting-identity-index.js'; import { backfillSeedTenancy, - resolveSeedTenancyExec, + resolveSeedTenancySeam, } from './migrations/seed-tenancy-backfill.js'; import { ObjectStackProtocolImplementation } from './protocol.js'; import type { MetadataAuthoringChannel } from './protocol.js'; @@ -256,7 +256,7 @@ export function assembleMetadataProtocol( // organization exists yet — and is handled at the first-admin // handoff instead (see runtime's app-plugin). try { - await backfillSeedTenancy(resolveSeedTenancyExec(ql), ctx.logger); + await backfillSeedTenancy(resolveSeedTenancySeam(ql), ctx.logger); } catch (e: unknown) { ctx.logger.warn( '[metadata-protocol] seed/API tenancy backfill skipped (#8686)', diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index 7df8df2139..21cda81ea4 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -1282,10 +1282,10 @@ export class AppPlugin implements Plugin { (opCtx?.operation === 'create' || opCtx?.operation === 'insert'); if (!isOrgCreate) return; try { - const { backfillSeedTenancy, resolveSeedTenancyExec } = await import( + const { backfillSeedTenancy, resolveSeedTenancySeam } = await import( '@objectstack/metadata-protocol' ); - await backfillSeedTenancy(resolveSeedTenancyExec(ql), ctx.logger); + await backfillSeedTenancy(resolveSeedTenancySeam(ql), ctx.logger); } catch (e: any) { // Best-effort, exactly like the ownership handoff beside it: an // organization was just created and that must stand whatever diff --git a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts index 17012b8dca..702edaeef5 100644 --- a/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts +++ b/packages/runtime/src/seed-tenancy-autonumber-split.integration.test.ts @@ -50,7 +50,7 @@ import { SqlDriver } from '@objectstack/driver-sql'; import { SeedLoaderService, backfillSeedTenancy, - resolveSeedTenancyExec, + resolveSeedTenancySeam, GLOBAL_TENANT, } from '@objectstack/metadata-protocol'; @@ -196,7 +196,7 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { // The fix: the moment an organization exists, the untenanted seed rows are // adopted into it and the `__global__` counter is retired. - const result = await backfillSeedTenancy(resolveSeedTenancyExec(engine), createLogger() as any); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); expect(result.status).toBe('applied'); expect(result.organizationId).toBe(ORG_ID); expect(result.collisions).toEqual([]); @@ -225,7 +225,7 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { for (let i = 0; i < 4; i++) await apiCreate(engine, `api ${i + 1}`); expect(await readDuplicates(driver)).toHaveLength(4); - const result = await backfillSeedTenancy(resolveSeedTenancyExec(engine), createLogger() as any); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); expect(result.status).toBe('applied'); // Reported — every already-minted duplicate is named, with its holder count. @@ -260,8 +260,8 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { await seedFreshInstall(engine); await createOrganization(engine); - await backfillSeedTenancy(resolveSeedTenancyExec(engine), createLogger() as any); - const second = await backfillSeedTenancy(resolveSeedTenancyExec(engine), createLogger() as any); + await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); + const second = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); // Nothing left to detect: the `__global__` counter is gone, so the probe // short-circuits before any guard or write is even considered. @@ -279,7 +279,7 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { // source, not re-derived. `isolated` is a walled (multi-organization) shape. vi.stubEnv('OS_TENANCY_POSTURE', 'isolated'); const logger = createLogger(); - const result = await backfillSeedTenancy(resolveSeedTenancyExec(engine), logger as any); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), logger as any); expect(result.status).toBe('skipped-multi-tenant'); // Skipping is not silence: the ruling requires the condition AND the remedy. @@ -307,7 +307,7 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { ); const logger = createLogger(); - const result = await backfillSeedTenancy(resolveSeedTenancyExec(engine), logger as any); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), logger as any); // The posture says single-tenant but the DATA says otherwise. Two organizations // means the owner of an untenanted row is not derivable, whatever the posture @@ -374,7 +374,7 @@ describe('#8686 seed/API tenancy split — autonumber scope', () => { await createOrganization(engine); expect(await readSequences(driver)).toEqual([{ tenant: GLOBAL_TENANT, lastValue: 3 }]); - const result = await backfillSeedTenancy(resolveSeedTenancyExec(engine), createLogger() as any); + const result = await backfillSeedTenancy(resolveSeedTenancySeam(engine), createLogger() as any); // Seen as no split at all — the platform namespace is filtered before any // guard runs, so nothing is adopted and nothing is warned about. diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 464d23a012..b8e61b6000 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1245,6 +1245,9 @@ importers: '@types/node': specifier: ^26.1.2 version: 26.1.2 + mysql2: + specifier: ^3.23.1 + version: 3.23.1(@types/node@26.1.2) tsup: specifier: ^8.5.1 version: 8.5.1(jiti@2.7.0)(postcss@8.5.26)(tsx@4.23.12)(typescript@6.0.3)(yaml@2.9.0) From d79b35377ceecc0862203eb1785c82c0c8e72229 Mon Sep 17 00:00:00 2001 From: os-steve Date: Tue, 18 Aug 2026 00:58:20 +0000 Subject: [PATCH 2/2] test(ci): run the metadata-protocol migration statements against the live MySQL (#9381) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01NTKPDRoynY8i3HmdSFUxFj --- .changeset/seed-tenancy-mysql-dialect.md | 30 ++++++++++++++++++++++++ .github/workflows/ci.yml | 27 +++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 .changeset/seed-tenancy-mysql-dialect.md diff --git a/.changeset/seed-tenancy-mysql-dialect.md b/.changeset/seed-tenancy-mysql-dialect.md new file mode 100644 index 0000000000..8922cbcb12 --- /dev/null +++ b/.changeset/seed-tenancy-mysql-dialect.md @@ -0,0 +1,30 @@ +--- +"@objectstack/metadata-protocol": minor +"@objectstack/runtime": patch +--- + +fix(metadata-protocol): compile the seed-tenancy backfill's statements for the connected dialect, so they run on MySQL (#9381) + +`seed-tenancy-backfill.ts` quoted every identifier the ANSI way (`"x"`) on every +dialect. MySQL does not run with `ANSI_QUOTES` — measured on a live MySQL 8.0.46, +whose `sql_mode` is +`ONLY_FULL_GROUP_BY,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION`, +and nothing in `driver-sql` sets one — so `"x"` is a string literal there and all +seven statements failed with `ER_PARSE_ERROR`. The repair for #8686 therefore +never ran on MySQL, silently: a migration must not fail a boot, so every call site +turns the failure into a warning and the symptom was a skipped repair in the log +rather than an error. + +The statements are now compiled for the driver actually connected, and the seam +carries the dialect with it (`resolveSeedTenancySeam` returns `{ exec, client }`; +`backfillSeedTenancy` takes that pair) so a caller cannot lose it. Two further +MySQL-only defects in the same statements, both measured on the same server, are +fixed with it: `last_value` is a reserved word on MySQL 8.0 and is now quoted +wherever it is unqualified, and the stamp's exclusion sub-SELECTs go through a +derived table because MySQL refuses `UPDATE t … (SELECT … FROM t)` with +`ER_UPDATE_TABLE_USED`. SQLite and PostgreSQL keep the exact ANSI spelling they +had (both re-verified live). + +`resolveSeedTenancyExec` stays exported and unchanged for callers that resolve the +dialect themselves; `backfillSeedTenancy` now takes the seam object instead of a +bare exec. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 28ddd00992..5ae0f484e7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -734,6 +734,33 @@ jobs: --filter @objectstack/service-analytics \ test + # ── The migration statements, on the live MySQL (#9381) ─────────────── + # + # `metadata-protocol` builds raw SQL by hand for the three dialects the + # platform supports, and MySQL is the only one that can fail: it does not + # run with `ANSI_QUOTES`, so the ANSI `"identifier"` the seed-tenancy + # backfill used to emit unconditionally was a STRING LITERAL there and + # every statement was an `ER_PARSE_ERROR`. Nothing surfaced it, because a + # migration must never fail a boot — every call site turns the failure + # into a warning, so the symptom was a skipped repair in the log. + # + # This rides this job rather than getting its own because this is where a + # live MySQL already exists. Only the live file runs: the rest of the + # package's suite has no server axis and runs in Test Core. + - name: Build metadata-protocol and its dependencies + run: pnpm exec turbo run build --filter=@objectstack/metadata-protocol... --concurrency=4 + + - name: Run the metadata-protocol migration statements against live MySQL + env: + OS_TEST_MYSQL_URL: mysql://root:root@127.0.0.1:3306/conformance + # Same vacuous-pass guard as the driver-sql leg: this runner + # provisioned the server, so a missing URL is a defect in the runner + # and must be a red rather than a skip. + OS_EXPECT_LIVE_DIALECT_MATRIX: '1' + run: | + pnpm --filter @objectstack/metadata-protocol exec vitest run \ + src/migrations/seed-tenancy-backfill.live-mysql.test.ts + dogfood: # Sharded 3-way: the suite is ~60 independent test files, each booting its # own in-process app; a single 4-vCPU runner needed ~7½ minutes for the