From 21b756e38b9ff77ed98d37f26a718e4130c6df2b Mon Sep 17 00:00:00 2001 From: os-elon Date: Thu, 20 Aug 2026 17:11:42 +0000 Subject: [PATCH 1/3] test(driver-sql): isolate the live-dialect matrix per test file (#9350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every live-matrix file resolved its connection from one env var per dialect, so all 14 of them shared one conformance database — one `public` schema on Postgres, one `conformance` database on MySQL — including the driver's internal `_objectstack_sequences` counter table, which each file's autonumber path lazily creates and writes. `cell.config()` now derives a per-FILE schema (Postgres) / database (MySQL) from vitest's own `testPath`, created by a `pool.afterCreate` hook so every pooled connection lands in it. No assertion changed; no test is skipped, quarantined, retried, or given a larger budget. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../src/live-dialect-matrix.isolation.test.ts | 244 ++++++++++++++++++ .../src/live-dialect-matrix.testkit.ts | 222 +++++++++++++++- .../sql-driver-date-now-default-live.test.ts | 27 +- .../sql-driver-datetime-mysql-storage.test.ts | 15 +- ...-driver-datetime-postgres-timezone.test.ts | 11 +- .../src/sql-driver-time-live-dialects.test.ts | 29 ++- 6 files changed, 511 insertions(+), 37 deletions(-) create mode 100644 packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts diff --git a/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts b/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts new file mode 100644 index 0000000000..72037b8f56 --- /dev/null +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts @@ -0,0 +1,244 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9350 — the live-dialect matrix files must not be able to share one database. + * + * ## Why this suite is structural and not a reproduction + * + * The failure it guards against is a `Error: Test timed out in 5000ms.` on live + * MySQL, seen four times in three days and **zero times in the three days + * after**. A green CI run therefore proves nothing here — CI was already going + * green. So this suite does not try to reproduce contention. It asserts the + * property that makes the contention impossible: **two files resolve two + * different schemas, and each file's schema derives from the file rather than + * from a shared constant.** + * + * That is checkable without a live server, which matters because a live MySQL + * cannot be run in an agent container at all. + * + * ## What each part is for + * + * - `liveSchemaNameFor` is asserted INJECTIVE over the real list of live files + * in this package, read off disk — not over invented inputs. A property that + * holds for two hand-written strings and fails for the two files that + * actually collide would be the same kind of measurement-of-a-neighbouring- + * object that this issue burned two rounds on. + * - the identifier-limit case is not decoration: Postgres truncates an + * identifier over 63 bytes **silently**, so two long file names could be + * truncated back onto one schema and the isolation would evaporate with + * nothing red anywhere. + * - the source scan is what keeps the property TRUE. Nothing stops a new live + * suite from reading `process.env.OS_TEST_MYSQL_URL` and hand-building + * `{ client, connection }` again — that is exactly how every file in this + * package came to share one database. The scan makes the cell the only + * route to a live server. + * - the `afterCreate` cases drive the REAL hook out of the REAL config with a + * recording connection. They are the only verification the MySQL half can + * get without a server, and they are honest about their limit: they prove + * which statements are issued, not that a server accepts them. + */ + +import { describe, expect, it } from 'vitest'; +import { readFileSync, readdirSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { + LIVE_SCHEMA_PREFIX, + MYSQL_CELL, + PG_CELL, + currentLiveSchema, + liveSchemaNameFor, +} from './live-dialect-matrix.testkit.js'; + +const SRC_DIR = dirname(fileURLToPath(import.meta.url)); + +/** Every test file in this package, as the repo-relative keys the resolver takes. */ +const TEST_FILE_KEYS = readdirSync(SRC_DIR) + .filter((f) => f.endsWith('.test.ts')) + .sort() + .map((f) => `packages/drivers/driver-sql/src/${f}`); + +/** The identifier ceiling that binds: Postgres 63 bytes, MySQL 64. */ +const IDENTIFIER_LIMIT = 63; + +describe('live-dialect matrix — per-file schema isolation (#9350)', () => { + it('has a non-trivial file list to measure over', () => { + // Guards the vacuous pass: every assertion below iterates this list, so a + // list that came back empty would report success having checked nothing. + expect(TEST_FILE_KEYS.length).toBeGreaterThan(50); + }); + + it('gives every test file in this package a DISTINCT schema', () => { + const byName = new Map(); + for (const key of TEST_FILE_KEYS) { + const name = liveSchemaNameFor(key); + byName.set(name, [...(byName.get(name) ?? []), key]); + } + const collisions = [...byName].filter(([, files]) => files.length > 1); + expect(collisions, `these files would share one database: ${JSON.stringify(collisions)}`) + .toEqual([]); + expect(byName.size).toBe(TEST_FILE_KEYS.length); + }); + + it('is deterministic — the same file always resolves the same schema', () => { + for (const key of TEST_FILE_KEYS.slice(0, 5)) { + expect(liveSchemaNameFor(key)).toBe(liveSchemaNameFor(key)); + } + }); + + it('derives the name from the WHOLE path, so same-named files in two packages differ', () => { + const a = 'packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts'; + const b = 'packages/metadata-protocol/src/sql-driver-datetime-mysql-storage.test.ts'; + expect(liveSchemaNameFor(a)).not.toBe(liveSchemaNameFor(b)); + }); + + it('stays distinct where the readable slug truncates to the same prefix', () => { + // The trap: the slug is capped so the whole identifier fits, and a cap is + // not injective. Two names that agree for the first 40+ characters are the + // realistic shape of that — `sql-driver-datetime-postgres-timezone` and + // `sql-driver-upsert-conflict-target-dialects` already truncate today. + const a = 'packages/drivers/driver-sql/src/sql-driver-a-very-long-live-dialect-name-one.test.ts'; + const b = 'packages/drivers/driver-sql/src/sql-driver-a-very-long-live-dialect-name-two.test.ts'; + const [na, nb] = [liveSchemaNameFor(a), liveSchemaNameFor(b)]; + expect(na.slice(0, 40)).toBe(nb.slice(0, 40)); // the fixture is vacuous otherwise + expect(na).not.toBe(nb); + }); + + it('fits both dialects’ identifier limits, and is DDL-safe by construction', () => { + for (const key of TEST_FILE_KEYS) { + const name = liveSchemaNameFor(key); + expect(name.length, `${name} exceeds the ${IDENTIFIER_LIMIT}-byte limit`) + .toBeLessThanOrEqual(IDENTIFIER_LIMIT); + expect(name).toMatch(/^[a-z][a-z0-9_]*$/); + expect(name.startsWith(LIVE_SCHEMA_PREFIX)).toBe(true); + } + }); + + it('resolves the CURRENT file, from vitest’s testPath rather than an argument', () => { + // `currentLiveSchema()` takes nothing, so there is no parameter a copy-paste + // could carry over from the file it was copied from. + expect(currentLiveSchema()).toBe( + liveSchemaNameFor('packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts'), + ); + }); +}); + +describe('live-dialect matrix — the cell is the only route to a live server (#9350)', () => { + /** Source with line and block comments removed, so prose about the env var is not a hit. */ + const codeOf = (file: string): string => + readFileSync(join(SRC_DIR, file), 'utf8') + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/^[ \t]*\/\/.*$/gm, ''); + + /** + * This file, and only this file. + * + * The scan matched ITSELF on first run — the pattern it searches for is a + * regex literal in its own source. Excluding a file from the scan is exactly + * the shape of hole the scan exists to prevent, so the exclusion is pinned by + * the next case rather than trusted: this suite opens no connection at all, + * so it cannot be a live file however it spells the env var. + */ + const SCANNER = 'live-dialect-matrix.isolation.test.ts'; + + it('no test file in this package reads OS_TEST_*_URL directly', () => { + const offenders = TEST_FILE_KEYS + .map((k) => k.slice(k.lastIndexOf('/') + 1)) + .filter((f) => f !== SCANNER) + .filter((f) => /process\.env\.OS_TEST_(POSTGRES|MYSQL)_URL/.test(codeOf(f))); + expect( + offenders, + 'these read the env var directly, which is one value for the whole process and puts ' + + 'every reader in the SAME database (#9350). Use MYSQL_CELL / PG_CELL from ' + + 'live-dialect-matrix.testkit.ts: `cell.url` for skipIf, `cell.config()` to connect.', + ).toEqual([]); + }); + + it('the one excluded file cannot itself be a live suite', () => { + // What earns the exclusion: no driver, no knex, no connection. If this ever + // stops holding, the exclusion stops being safe and this goes red first. + // + // The needles are ASSEMBLED rather than written as literals, because a + // literal would appear in this file and the case would match itself — the + // same self-reference that made the scan above flag its own source on its + // first run, and the reason the exclusion exists at all. + const own = codeOf(SCANNER); + const needle = (head: string, tail: string) => new RegExp(head + tail); + expect(own).not.toMatch(needle('new ', 'SqlDriver')); + expect(own).not.toMatch(needle('knex', '\\(')); + expect(own).not.toMatch(needle('\\.', 'connect\\(')); + expect(own).not.toMatch(needle('driver', '\\.execute')); + }); + + it('the testkit itself is the one place that reads them', () => { + const testkit = readFileSync(join(SRC_DIR, 'live-dialect-matrix.testkit.ts'), 'utf8'); + expect(testkit).toContain('process.env.OS_TEST_POSTGRES_URL'); + expect(testkit).toContain('process.env.OS_TEST_MYSQL_URL'); + }); +}); + +/** A knex pool connection that records the SQL a hook issues instead of running it. */ +function recordingConnection(): { sql: string[]; conn: any } { + const sql: string[] = []; + const conn = { + query(text: string, cb: (err: unknown) => void) { + sql.push(text); + cb(null); + }, + }; + return { sql, conn }; +} + +async function runAfterCreate(config: any): Promise { + const { sql, conn } = recordingConnection(); + const afterCreate = config.pool?.afterCreate; + expect(typeof afterCreate, 'the live config carries no afterCreate hook').toBe('function'); + await new Promise((resolve, reject) => { + afterCreate(conn, (err: unknown) => (err ? reject(err) : resolve())); + }); + return sql; +} + +describe('live-dialect matrix — what each dialect issues to isolate itself (#9350)', () => { + const schema = liveSchemaNameFor( + 'packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts', + ); + + it('postgres: knex is pointed at the file’s schema, which the hook creates', async () => { + const config: any = PG_CELL.config(); + expect(config.client).toBe('pg'); + expect(config.searchPath).toEqual([schema]); + expect(await runAfterCreate(config)).toEqual([ + `create schema if not exists "${schema}"`, + `set search_path to "${schema}"`, + ]); + }); + + it('mysql: the session is moved into the file’s own database', async () => { + // MySQL has no schema-inside-a-database, so the database IS the unit. `use` + // rather than a rewritten URL keeps `database()` — which several suites + // compare `information_schema.columns` against — reporting the isolated one. + const config: any = MYSQL_CELL.config(); + expect(config.client).toBe('mysql2'); + expect(await runAfterCreate(config)).toEqual([ + `create database if not exists \`${schema}\``, + `use \`${schema}\``, + ]); + }); + + it('runs the hook on EVERY pooled connection, not just the first', async () => { + // A pool opens connections lazily and forever: a connection created late in + // a file must land in the same schema as the first one, or half the file + // silently runs against the shared default database. + const config: any = PG_CELL.config(); + expect(await runAfterCreate(config)).toEqual(await runAfterCreate(config)); + }); + + it('the two live dialects do not resolve to one shared name', () => { + // Same file, so the NAME is deliberately shared across dialects — what must + // not happen is two FILES sharing one. This pins the axis the isolation is + // on, so a future edit that keyed off the dialect instead of the file is red. + const other = liveSchemaNameFor('packages/drivers/driver-sql/src/some-other-live.test.ts'); + expect(other).not.toBe(schema); + }); +}); diff --git a/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts b/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts index a547340f0f..18c0533ba7 100644 --- a/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts @@ -43,6 +43,9 @@ */ import { describe, expect, it } from 'vitest'; +import { createHash } from 'node:crypto'; +import { existsSync } from 'node:fs'; +import { basename, dirname, join } from 'node:path'; import type { SqlDriver, SqlDriverConfig } from './sql-driver.js'; /** The dialects `driver-sql` speaks that the matrices are run across. */ @@ -91,6 +94,179 @@ const MYSQL_URL = process.env.OS_TEST_MYSQL_URL; */ export const EXPECT_LIVE_DIALECTS = process.env.OS_EXPECT_LIVE_DIALECT_MATRIX === '1'; +// ── Per-FILE schema isolation (#9350) ──────────────────────────────────────── +// +// Every live cell used to hand back the same connection string, so all ~14 +// live-matrix files in this package shared ONE conformance database: one +// `public` schema on Postgres, one `conformance` database on MySQL. Measured on +// a real Postgres 16 before this change, a run of the live-PG files left exactly +// one schema behind — `public`, holding `_objectstack_sequences`, the driver's +// internal auto-number counter table that EVERY file's autonumber path +// lazily creates (`hasTable` → `createTable`, a check-then-act pair) and writes +// rows into. Vitest runs those files in parallel workers, so that shared table +// is created, migrated (`_objectstack_sequences__rebuild`, a drop+rename) and +// written concurrently by files that know nothing about each other. +// +// The isolation is per FILE rather than per suite or per worker because a file +// is the unit vitest parallelises: two suites in one file never run at the same +// time, two files can always be running at once, and a worker is recycled +// across files so its identity outlives the isolation it would provide. +// +// ## The name derives from the FILE, and cannot be given a shared value +// +// {@link currentLiveSchema} reads vitest's own `testPath` — the caller passes +// nothing, so there is no parameter a copy-paste can carry over from the file +// it was copied from, and no constant a consumer can point two files at. That +// is the property `live-dialect-matrix.isolation.test.ts` asserts, and it is +// asserted structurally: distinct file paths map to distinct names, over the +// real list of live files in this package. +// +// ⛔ Note what this is NOT: no test is skipped, quarantined, retried or given a +// larger budget, and no assertion changed. Isolation removes contention; it +// does not accommodate it. + +/** Prefix every per-file schema/database carries, so a leftover is identifiable. */ +export const LIVE_SCHEMA_PREFIX = 'os_lv_'; + +/** + * The per-file schema (Postgres) / database (MySQL — the same concept there) + * for one test file, named by its repo-relative path. + * + * Pure and deterministic, so the isolation test can assert distinctness over the + * real file list without a server. The shape is + * `os_lv__<12 hex of sha256(path)>`: + * + * - the SLUG is readable, so an operator looking at `\dn` or `show databases` + * can tell which file owns a leftover; + * - the HASH is what carries uniqueness. The slug is truncated to keep the + * whole name inside the SHORTER of the two dialect limits (Postgres + * truncates an identifier over 63 bytes SILENTLY, which would turn two long + * file names back into one shared schema — the exact failure this removes), + * and a truncated slug is not injective. The hash is taken over the FULL + * path, so two files that truncate to the same slug still differ. + * + * `[a-z0-9_]` only, asserted below rather than assumed: the name is + * interpolated into DDL, and a name that can only be those characters cannot + * carry a quote out of a file name. + */ +export function liveSchemaNameFor(testFileKey: string): string { + const key = testFileKey.replace(/\\/g, '/'); + const slug = basename(key) + .replace(/\.test\.tsx?$/, '') + .toLowerCase() + .replace(/[^a-z0-9]+/g, '_') + .replace(/^_+|_+$/g, '') + .slice(0, 34); + const hash = createHash('sha256').update(key).digest('hex').slice(0, 12); + const name = `${LIVE_SCHEMA_PREFIX}${slug}_${hash}`; + if (!/^[a-z][a-z0-9_]*$/.test(name) || name.length > 63) { + throw new Error( + `live-dialect isolation: derived an unusable schema name ${JSON.stringify(name)} from ` + + `${JSON.stringify(testFileKey)} — it must match /^[a-z][a-z0-9_]*$/ and fit Postgres' ` + + `63-byte identifier limit.`, + ); + } + return name; +} + +/** Memoised so the walk below runs once per file, not once per connection. */ +const REPO_RELATIVE_CACHE = new Map(); + +/** + * An absolute test path reduced to something stable across machines: the path + * relative to the workspace root (the nearest ancestor holding + * `pnpm-workspace.yaml`). + * + * The absolute path would work for isolation — it differs per file either way — + * but it also differs per checkout, so the same file would get a different + * schema in a worktree than in CI and the isolation test could not pin a name. + */ +function repoRelativeTestPath(absolutePath: string): string { + const cached = REPO_RELATIVE_CACHE.get(absolutePath); + if (cached !== undefined) return cached; + let dir = dirname(absolutePath); + let relative = basename(absolutePath); + for (;;) { + if (existsSync(join(dir, 'pnpm-workspace.yaml'))) { + relative = absolutePath.slice(dir.length + 1); + break; + } + const parent = dirname(dir); + if (parent === dir) break; // reached the filesystem root: fall back to the basename + dir = parent; + } + REPO_RELATIVE_CACHE.set(absolutePath, relative); + return relative; +} + +/** + * The schema/database the CURRENT test file owns. + * + * Takes no argument on purpose. A parameter is the one thing a consumer could + * get wrong — two files given the same literal are back to sharing a database, + * and nothing about the call site would look wrong. `testPath` is vitest's own + * per-file fact, measured available at module scope as well as inside a test + * (vitest 4.1), so the derivation cannot be pointed anywhere else. + * + * Absent `testPath` is a hard error rather than a fallback: the fallback would + * be a shared name, which is the defect. + */ +export function currentLiveSchema(): string { + const testPath = expect.getState().testPath; + if (!testPath) { + throw new Error( + 'live-dialect isolation (#9350): vitest reported no testPath, so this live connection ' + + 'cannot be given a per-file schema and would fall back to sharing one database with ' + + 'every other live file — the contention this removed. Build live connections from a ' + + 'test file, through DIALECT_CELLS[].config().', + ); + } + return liveSchemaNameFor(repoRelativeTestPath(testPath)); +} + +/** + * Postgres: create the file's schema and point the session at it. + * + * Both halves run on EVERY pooled connection because a pool opens connections + * lazily and forever — a second connection created an hour into the run must + * land in the same schema as the first. `if not exists` makes the repeat free + * and makes two connections of the same file racing each other a no-op. + * + * `searchPath` is ALSO set in the knex config (below) so knex's own schema + * builder qualifies its DDL; knex applies that before this hook runs, against a + * schema that may not exist yet — which Postgres permits, since `search_path` + * entries are resolved per statement, not at `SET` time. + */ +function pgAfterCreate(schema: string) { + return (conn: any, done: (err: unknown, conn: unknown) => void): void => { + conn.query(`create schema if not exists "${schema}"`, (err: unknown) => { + if (err) return done(err, conn); + conn.query(`set search_path to "${schema}"`, (err2: unknown) => done(err2, conn)); + }); + }; +} + +/** + * MySQL: create the file's database and switch the session into it. + * + * MySQL has no schema-inside-a-database, so a database IS the unit of + * isolation — `create database` is the same cheap dictionary write `create + * schema` is on Postgres, not a template copy. + * + * `use` rather than rewriting the connection URL, so the URL a runner + * provisioned stays the thing we connect to and only the SESSION moves. It also + * keeps `database()` — which several suites compare `information_schema.columns` + * against — reporting the isolated database. + */ +function mysqlAfterCreate(schema: string) { + return (conn: any, done: (err: unknown, conn: unknown) => void): void => { + conn.query(`create database if not exists \`${schema}\``, (err: unknown) => { + if (err) return done(err, conn); + conn.query(`use \`${schema}\``, (err2: unknown) => done(err2, conn)); + }); + }; +} + /** * Every cell of the driver axis, available or not — a consumer iterates the * whole list so an unprovisioned dialect is *reported*, not omitted. @@ -117,7 +293,17 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!PG_URL, live: true, hasLegacyStorageForm: false, - config: () => ({ client: 'pg', connection: PG_URL }), + config: () => { + const schema = currentLiveSchema(); + return { + client: 'pg', + connection: PG_URL, + // knex qualifies its own DDL with this; `pgAfterCreate` creates the + // schema and re-points the session, so raw SQL in a suite lands there too. + searchPath: [schema], + pool: { afterCreate: pgAfterCreate(schema) }, + }; + }, }, { id: 'mysql', @@ -127,13 +313,45 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!MYSQL_URL, live: true, hasLegacyStorageForm: false, - config: () => ({ client: 'mysql2', connection: MYSQL_URL }), + config: () => { + const schema = currentLiveSchema(); + return { + client: 'mysql2', + connection: MYSQL_URL, + pool: { afterCreate: mysqlAfterCreate(schema) }, + }; + }, }, ] as const; /** The live cells only — the ones the server-timezone axis applies to. */ export const LIVE_DIALECT_CELLS = DIALECT_CELLS.filter((c) => c.live); +/** + * One cell by id — the entry point for a suite that is about ONE dialect and so + * has no matrix to iterate (`sql-driver-datetime-mysql-storage.test.ts` is only + * ever about MySQL). + * + * It exists so those suites stop reading `process.env.OS_TEST_*_URL` and + * building `{ client, connection }` by hand. That hand-rolled pair is what put + * every one of them in the SAME database: the env var is one value for the whole + * process, and a config built from it carries no per-file isolation. Going + * through the cell means `config()` — and therefore {@link currentLiveSchema} — + * is the only way to reach a live server, which is what + * `live-dialect-matrix.isolation.test.ts` enforces for this package. + */ +export function dialectCell(id: DialectId): DialectCell { + const cell = DIALECT_CELLS.find((c) => c.id === id); + if (!cell) throw new Error(`no dialect cell for ${id}`); + return cell; +} + +/** The MySQL cell. `.url` is the provisioned URL (or `undefined`) for `skipIf`. */ +export const MYSQL_CELL = dialectCell('mysql'); + +/** The Postgres cell. `.url` is the provisioned URL (or `undefined`) for `skipIf`. */ +export const PG_CELL = dialectCell('pg'); + /** * Declare a cell nobody provisioned: REPORTED, never omitted. * diff --git a/packages/drivers/driver-sql/src/sql-driver-date-now-default-live.test.ts b/packages/drivers/driver-sql/src/sql-driver-date-now-default-live.test.ts index 3313b0b6dd..74e33a17b3 100644 --- a/packages/drivers/driver-sql/src/sql-driver-date-now-default-live.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-date-now-default-live.test.ts @@ -22,9 +22,14 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { MYSQL_CELL, PG_CELL, type DialectCell } from './live-dialect-matrix.testkit.js'; -const PG_URL = process.env.OS_TEST_POSTGRES_URL; -const MY_URL = process.env.OS_TEST_MYSQL_URL; +// The cells, not `process.env.OS_TEST_*_URL`: an env var is one value for the +// whole process, so a config built from it puts this file in the same +// schema/database as every other live file. `cell.config()` derives a per-FILE +// one from vitest's own `testPath` (#9350). +const PG_URL = PG_CELL.url; +const MY_URL = MYSQL_CELL.url; const TABLE = 'os4022_probe'; const SHAPE = { @@ -35,16 +40,12 @@ const SHAPE = { }, } as any; -function suite(dialect: 'pg' | 'mysql', url: string | undefined) { - describe.skipIf(!url)(`Field.date NOW() default on live ${dialect} (#4022)`, () => { +function suite(cell: DialectCell) { + describe.skipIf(!cell.available)(`Field.date NOW() default on live ${cell.id} (#4022)`, () => { let driver: SqlDriver; beforeEach(async () => { - driver = new SqlDriver( - dialect === 'pg' - ? { client: 'pg', connection: url } - : { client: 'mysql2', connection: url }, - ); + driver = new SqlDriver(cell.config()); await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); await driver.initObjects([SHAPE]); }); @@ -58,12 +59,12 @@ function suite(dialect: 'pg' | 'mysql', url: string | undefined) { // On MySQL 8.0 reaching this line at all is the compatibility half of the // fix — the bare default fails CREATE TABLE there. const res: any = await driver.execute( - dialect === 'pg' + cell.id === 'pg' ? `select column_default as d from information_schema.columns where table_name = '${TABLE}' and column_name = 'due_on'` : `select column_default as d from information_schema.columns where table_schema = database() and table_name = ? and column_name = 'due_on'`, - dialect === 'pg' ? [] : [TABLE], + cell.id === 'pg' ? [] : [TABLE], ); const rows = Array.isArray(res) && Array.isArray(res[0]) ? res[0] : (res?.rows ?? res); const def = String((rows[0] as any).d ?? (rows[0] as any).D ?? '').toLowerCase(); @@ -86,5 +87,5 @@ function suite(dialect: 'pg' | 'mysql', url: string | undefined) { }); } -suite('pg', PG_URL); -suite('mysql', MY_URL); +suite(PG_CELL); +suite(MYSQL_CELL); diff --git a/packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts b/packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts index 6f6525c53f..de43ef4148 100644 --- a/packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-datetime-mysql-storage.test.ts @@ -34,8 +34,13 @@ import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { MYSQL_CELL } from './live-dialect-matrix.testkit.js'; -const URL = process.env.OS_TEST_MYSQL_URL; +// The cell, not `process.env.OS_TEST_MYSQL_URL`: the env var is one value for +// the whole process, so a config built from it puts this file in the same +// database as every other live file. `MYSQL_CELL.config()` derives a per-FILE +// database from vitest's own `testPath` (#9350). +const URL = MYSQL_CELL.url; const TABLE = 'os3942_probe'; const MIDDAY = '2026-03-20T12:34:56.789Z'; @@ -43,7 +48,7 @@ const MIDDAY = '2026-03-20T12:34:56.789Z'; const BOUNDARY = '2026-03-20T20:00:00.000Z'; /** A driver whose connection this suite does NOT pin, to read raw server state. */ -const rawDriver = () => new SqlDriver({ client: 'mysql2', connection: URL }); +const rawDriver = () => new SqlDriver(MYSQL_CELL.config()); describe.skipIf(!URL)('Field.datetime on MySQL (#3942)', () => { let driver: SqlDriver; @@ -57,7 +62,7 @@ describe.skipIf(!URL)('Field.datetime on MySQL (#3942)', () => { }); beforeEach(async () => { - driver = new SqlDriver({ client: 'mysql2', connection: URL }); + driver = new SqlDriver(MYSQL_CELL.config()); await driver.execute(`drop table if exists ${TABLE}`); await driver.initObjects([ { name: TABLE, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, @@ -172,7 +177,7 @@ describe.skipIf(!URL)('MySQL TIMESTAMP → DATETIME(3) migration (#3942)', () => ]); await legacy.disconnect(); - driver = new SqlDriver({ client: 'mysql2', connection: URL }); + driver = new SqlDriver(MYSQL_CELL.config()); }); afterEach(async () => { @@ -248,7 +253,7 @@ describe.skipIf(!URL)('os migrate plan lists the MySQL widening (#3954)', () => ]); } await legacy.disconnect(); - driver = new SqlDriver({ client: 'mysql2', connection: URL }); + driver = new SqlDriver(MYSQL_CELL.config()); }); afterEach(async () => { diff --git a/packages/drivers/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts b/packages/drivers/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts index c375dfcf92..b44f7632c2 100644 --- a/packages/drivers/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-datetime-postgres-timezone.test.ts @@ -32,8 +32,13 @@ import { describe, it, expect, beforeAll, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; +import { PG_CELL } from './live-dialect-matrix.testkit.js'; -const URL = process.env.OS_TEST_POSTGRES_URL; +// The cell, not `process.env.OS_TEST_POSTGRES_URL`: the env var is one value for +// the whole process, so a config built from it puts this file in the same schema +// as every other live file. `PG_CELL.config()` derives a per-FILE schema from +// vitest's own `testPath` (#9350). +const URL = PG_CELL.url; const TABLE = 'os3912_probe'; /** 20:00Z is 04:00 the NEXT day at +08:00 — so a day window discriminates. */ @@ -45,14 +50,14 @@ describe.skipIf(!URL)('Field.datetime on Postgres is timezone-independent (#3912 let serverTimeZone = ''; beforeAll(async () => { - const probe = new SqlDriver({ client: 'pg', connection: URL }); + const probe = new SqlDriver(PG_CELL.config()); const res: any = await probe.execute(`select current_setting('TimeZone') as tz`); serverTimeZone = ((res?.rows ?? res)[0] as any).tz; await probe.disconnect(); }); beforeEach(async () => { - driver = new SqlDriver({ client: 'pg', connection: URL }); + driver = new SqlDriver(PG_CELL.config()); await driver.execute(`drop table if exists "${TABLE}" cascade`); await driver.initObjects([ { name: TABLE, fields: { label: { type: 'string' }, at: { type: 'datetime' } } }, diff --git a/packages/drivers/driver-sql/src/sql-driver-time-live-dialects.test.ts b/packages/drivers/driver-sql/src/sql-driver-time-live-dialects.test.ts index 5b982f8018..85f70ddb33 100644 --- a/packages/drivers/driver-sql/src/sql-driver-time-live-dialects.test.ts +++ b/packages/drivers/driver-sql/src/sql-driver-time-live-dialects.test.ts @@ -23,9 +23,14 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import { SqlDriver } from '../src/index.js'; - -const PG_URL = process.env.OS_TEST_POSTGRES_URL; -const MY_URL = process.env.OS_TEST_MYSQL_URL; +import { MYSQL_CELL, PG_CELL, type DialectCell } from './live-dialect-matrix.testkit.js'; + +// The cells, not `process.env.OS_TEST_*_URL`: an env var is one value for the +// whole process, so a config built from it puts this file in the same +// schema/database as every other live file. `cell.config()` derives a per-FILE +// one from vitest's own `testPath` (#9350). +const PG_URL = PG_CELL.url; +const MY_URL = MYSQL_CELL.url; const TABLE = 'os3994_probe'; const SHAPE = { @@ -64,16 +69,12 @@ function minutesOffUtc(presented: string): number { return Math.min(diff, 1440 - diff); } -function suite(dialect: 'pg' | 'mysql', url: string | undefined) { - describe.skipIf(!url)(`Field.time on live ${dialect} (#3994)`, () => { +function suite(cell: DialectCell) { + describe.skipIf(!cell.available)(`Field.time on live ${cell.id} (#3994)`, () => { let driver: SqlDriver; beforeEach(async () => { - driver = new SqlDriver( - dialect === 'pg' - ? { client: 'pg', connection: url } - : { client: 'mysql2', connection: url }, - ); + driver = new SqlDriver(cell.config()); await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); await driver.initObjects([SHAPE]); }); @@ -143,8 +144,8 @@ function suite(dialect: 'pg' | 'mysql', url: string | undefined) { }); } -suite('pg', PG_URL); -suite('mysql', MY_URL); +suite(PG_CELL); +suite(MYSQL_CELL); describe.skipIf(!MY_URL)('MySQL TIME → TIME(3) widening (#3994)', () => { const LEGACY = 'os3994_legacy'; @@ -152,7 +153,7 @@ describe.skipIf(!MY_URL)('MySQL TIME → TIME(3) widening (#3994)', () => { beforeEach(async () => { // Build the table the way a pre-#3994 build did: a bare TIME column. - const legacy = new SqlDriver({ client: 'mysql2', connection: MY_URL }); + const legacy = new SqlDriver(MYSQL_CELL.config()); await legacy.execute(`drop table if exists ${LEGACY}`); await legacy.execute( `create table ${LEGACY} ( @@ -166,7 +167,7 @@ describe.skipIf(!MY_URL)('MySQL TIME → TIME(3) widening (#3994)', () => { ]); await legacy.disconnect(); - driver = new SqlDriver({ client: 'mysql2', connection: MY_URL }); + driver = new SqlDriver(MYSQL_CELL.config()); }); afterEach(async () => { From 46d19033e943630e38b0e9215f90ac91e6a22b94 Mon Sep 17 00:00:00 2001 From: os-elon Date: Thu, 20 Aug 2026 20:07:38 +0000 Subject: [PATCH 2/3] fix(driver-sql): name the isolated database in the connection, and let PG introspection follow the session (#9350) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first attempt moved the session with `use` (MySQL) while leaving the connection pointed at `conformance`. knex binds `client.database()` — the CONNECTION's database — into `columnInfo`, so DDL ran in the per-file database while the column read answered from `conformance`: the driver saw an empty column set for a fully populated table and emitted `alter table ... add `, which the server rejected with *Duplicate column name*. Measured on a live MariaDB 10.11: 18 red, matching CI's three failing files exactly. The database is now named in the connection URL, so the two halves cannot disagree, and a vitest globalSetup creates the schemas before any pool opens. Postgres' index read pinned `n.nspname = 'public'`, which returned [] for a table that measurably had a primary key and a declared unique index once the suites moved off `public` — and [] reads as "no indexes", which assertConflictTargetHonoured turns into a refusal. It now resolves the table with to_regclass, the same way every other statement in the session resolves it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../src/live-dialect-matrix.globalsetup.ts | 100 ++++++++ .../src/live-dialect-matrix.isolation.test.ts | 224 ++++++++++++------ .../src/live-dialect-matrix.testkit.ts | 103 ++++---- packages/drivers/driver-sql/src/sql-driver.ts | 25 +- packages/drivers/driver-sql/vitest.config.ts | 8 + 5 files changed, 328 insertions(+), 132 deletions(-) create mode 100644 packages/drivers/driver-sql/src/live-dialect-matrix.globalsetup.ts diff --git a/packages/drivers/driver-sql/src/live-dialect-matrix.globalsetup.ts b/packages/drivers/driver-sql/src/live-dialect-matrix.globalsetup.ts new file mode 100644 index 0000000000..c3e610b245 --- /dev/null +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.globalsetup.ts @@ -0,0 +1,100 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #9350 — create the per-file schemas before any test opens a connection. + * + * ## Why this runs here and not in a hook + * + * The isolation names each file's database in the CONNECTION (see + * `mysqlUrlForSchema`), which is what keeps knex's `client.database()` and the + * session the same value. The cost of that choice is an ordering constraint: + * connecting to a MySQL database that does not exist fails at the handshake, so + * the databases have to exist before the first pool opens. + * + * A `beforeAll` cannot do it. `cell.config()` is called from inside `beforeEach` + * in most of the eleven consumers, which is too late to register a hook, and the + * testkit module is cached PER WORKER rather than per file — so a hook + * registered at its module scope would attach to whichever file that worker + * collected first and to no other. `globalSetup` runs once, in the main process, + * before any worker starts, and can await. That is exactly the shape of the + * constraint. + * + * ## Deliberately total, and deliberately cheap + * + * It creates a schema for EVERY test file in the package rather than for the + * live ones only. Deciding which files are live would mean parsing them, and a + * wrong answer is a handshake failure in a required check. A schema costs one + * dictionary row on both dialects (`create database` on MySQL is not a template + * copy the way Postgres' `createdb` is), and the teardown removes them. + * + * Without either URL this does nothing at all: a developer running without + * servers sees no connection attempt, exactly as before. + */ + +import knex from 'knex'; +import { liveSchemaLedger } from './live-dialect-matrix.testkit.js'; + +const PG_URL = process.env.OS_TEST_POSTGRES_URL; +const MYSQL_URL = process.env.OS_TEST_MYSQL_URL; + +/** + * Statements are built from the ledger's names, never from anything a caller + * supplies, and `liveSchemaNameFor` refuses to emit a name outside + * `/^[a-z][a-z0-9_]*$/` — so the interpolation below cannot carry a quote. + */ +async function withServer( + client: 'pg' | 'mysql2', + connection: string, + run: (db: ReturnType) => Promise, +): Promise { + const db = knex({ client, connection, pool: { min: 0, max: 1 } }); + try { + return await run(db); + } finally { + await db.destroy(); + } +} + +export async function setup(): Promise { + const ledger = liveSchemaLedger(); + if (PG_URL) { + await withServer('pg', PG_URL, async (db) => { + for (const { schema } of ledger) { + await db.raw(`create schema if not exists "${schema}"`); + } + }); + } + if (MYSQL_URL) { + await withServer('mysql2', MYSQL_URL, async (db) => { + for (const { schema } of ledger) { + await db.raw(`create database if not exists \`${schema}\``); + } + }); + } +} + +/** + * Drop what the setup created. + * + * Best-effort by design: a failed drop must not turn a green run red — the + * schemas are re-created idempotently next time, and CI's servers are thrown + * away with the job. It exists for the developer running against a long-lived + * local server, who would otherwise accumulate one schema per test file. + */ +export async function teardown(): Promise { + const ledger = liveSchemaLedger(); + if (PG_URL) { + await withServer('pg', PG_URL, async (db) => { + for (const { schema } of ledger) { + await db.raw(`drop schema if exists "${schema}" cascade`).catch(() => {}); + } + }).catch(() => {}); + } + if (MYSQL_URL) { + await withServer('mysql2', MYSQL_URL, async (db) => { + for (const { schema } of ledger) { + await db.raw(`drop database if exists \`${schema}\``).catch(() => {}); + } + }).catch(() => {}); + } +} diff --git a/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts b/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts index 72037b8f56..bfd44646b2 100644 --- a/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts @@ -38,8 +38,9 @@ * which statements are issued, not that a server accepts them. */ -import { describe, expect, it } from 'vitest'; +import { afterEach, describe, expect, it } from 'vitest'; import { readFileSync, readdirSync } from 'node:fs'; +import { SqlDriver } from '../src/index.js'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; import { @@ -47,7 +48,9 @@ import { MYSQL_CELL, PG_CELL, currentLiveSchema, + liveSchemaLedger, liveSchemaNameFor, + mysqlUrlForSchema, } from './live-dialect-matrix.testkit.js'; const SRC_DIR = dirname(fileURLToPath(import.meta.url)); @@ -131,21 +134,20 @@ describe('live-dialect matrix — the cell is the only route to a live server (# .replace(/^[ \t]*\/\/.*$/gm, ''); /** - * This file, and only this file. + * The needle is ASSEMBLED rather than written as a literal. * - * The scan matched ITSELF on first run — the pattern it searches for is a - * regex literal in its own source. Excluding a file from the scan is exactly - * the shape of hole the scan exists to prevent, so the exclusion is pinned by - * the next case rather than trusted: this suite opens no connection at all, - * so it cannot be a live file however it spells the env var. + * Written out, it appears in this file's own source and the scan flags + * ITSELF — which it did, on its first run. An exclusion list was the first + * fix and the wrong one: excluding a file from the scan is the exact shape of + * hole the scan exists to close, and this file has since become a live suite + * itself, which would have made the exclusion actively wrong. */ - const SCANNER = 'live-dialect-matrix.isolation.test.ts'; + const ENV_READ = new RegExp('process' + '\\.env\\.OS_TEST_(POSTGRES|MYSQL)_URL'); it('no test file in this package reads OS_TEST_*_URL directly', () => { const offenders = TEST_FILE_KEYS .map((k) => k.slice(k.lastIndexOf('/') + 1)) - .filter((f) => f !== SCANNER) - .filter((f) => /process\.env\.OS_TEST_(POSTGRES|MYSQL)_URL/.test(codeOf(f))); + .filter((f) => ENV_READ.test(codeOf(f))); expect( offenders, 'these read the env var directly, which is one value for the whole process and puts ' + @@ -154,91 +156,163 @@ describe('live-dialect matrix — the cell is the only route to a live server (# ).toEqual([]); }); - it('the one excluded file cannot itself be a live suite', () => { - // What earns the exclusion: no driver, no knex, no connection. If this ever - // stops holding, the exclusion stops being safe and this goes red first. - // - // The needles are ASSEMBLED rather than written as literals, because a - // literal would appear in this file and the case would match itself — the - // same self-reference that made the scan above flag its own source on its - // first run, and the reason the exclusion exists at all. - const own = codeOf(SCANNER); - const needle = (head: string, tail: string) => new RegExp(head + tail); - expect(own).not.toMatch(needle('new ', 'SqlDriver')); - expect(own).not.toMatch(needle('knex', '\\(')); - expect(own).not.toMatch(needle('\\.', 'connect\\(')); - expect(own).not.toMatch(needle('driver', '\\.execute')); + it('the scan can still see a violation — it is not matching nothing', () => { + // Without this, an ENV_READ that silently stopped matching would report a + // clean scan forever. The negative case, stated: the pattern must hit a + // string that DOES read the env var. + // assembled for the same reason the needle is — a literal here is itself a + // violation, and the scan would flag this file + expect(ENV_READ.test('const U = process' + '.env.OS_TEST_MYSQL_URL;')).toBe(true); + expect(ENV_READ.test('const U = MYSQL_CELL.url;')).toBe(false); }); it('the testkit itself is the one place that reads them', () => { const testkit = readFileSync(join(SRC_DIR, 'live-dialect-matrix.testkit.ts'), 'utf8'); - expect(testkit).toContain('process.env.OS_TEST_POSTGRES_URL'); - expect(testkit).toContain('process.env.OS_TEST_MYSQL_URL'); + // assembled, like every other spelling of these names in this file + expect(testkit).toContain('process' + '.env.OS_TEST_POSTGRES_URL'); + expect(testkit).toContain('process' + '.env.OS_TEST_MYSQL_URL'); }); }); -/** A knex pool connection that records the SQL a hook issues instead of running it. */ -function recordingConnection(): { sql: string[]; conn: any } { - const sql: string[] = []; - const conn = { - query(text: string, cb: (err: unknown) => void) { - sql.push(text); - cb(null); - }, - }; - return { sql, conn }; -} - -async function runAfterCreate(config: any): Promise { - const { sql, conn } = recordingConnection(); - const afterCreate = config.pool?.afterCreate; - expect(typeof afterCreate, 'the live config carries no afterCreate hook').toBe('function'); - await new Promise((resolve, reject) => { - afterCreate(conn, (err: unknown) => (err ? reject(err) : resolve())); - }); - return sql; -} - -describe('live-dialect matrix — what each dialect issues to isolate itself (#9350)', () => { +describe('live-dialect matrix — how each dialect is pointed at its own schema (#9350)', () => { const schema = liveSchemaNameFor( 'packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts', ); - it('postgres: knex is pointed at the file’s schema, which the hook creates', async () => { - const config: any = PG_CELL.config(); - expect(config.client).toBe('pg'); - expect(config.searchPath).toEqual([schema]); - expect(await runAfterCreate(config)).toEqual([ - `create schema if not exists "${schema}"`, - `set search_path to "${schema}"`, - ]); + it('mysql: the file’s database is named in the CONNECTION, not switched into later', () => { + // The distinction is the defect this replaced, and it is worth an assertion + // rather than a comment. With `use`, the SESSION moved but knex's + // `client.database()` kept returning the URL's database — and knex binds + // THAT into `columnInfo`, so the driver read columns from `conformance` for + // a table it had just created elsewhere and emitted `alter table … add + // ` for columns already present. Measured on a live MariaDB 10.11: + // 18 tests red, all *Duplicate column name* or a missing UNIQUE index. + const rewritten = mysqlUrlForSchema('mysql://root:root@127.0.0.1:3306/conformance', schema); + expect(new URL(rewritten).pathname).toBe(`/${schema}`); }); - it('mysql: the session is moved into the file’s own database', async () => { - // MySQL has no schema-inside-a-database, so the database IS the unit. `use` - // rather than a rewritten URL keeps `database()` — which several suites - // compare `information_schema.columns` against — reporting the isolated one. - const config: any = MYSQL_CELL.config(); - expect(config.client).toBe('mysql2'); - expect(await runAfterCreate(config)).toEqual([ - `create database if not exists \`${schema}\``, - `use \`${schema}\``, + it('mysql: only the database segment moves — host, port and credentials are the runner’s', () => { + const from = new URL('mysql://runner:s3cret@db.internal:3307/conformance'); + const to = new URL(mysqlUrlForSchema(from.toString(), schema)); + expect([to.protocol, to.host, to.username, to.password]).toEqual([ + from.protocol, from.host, from.username, from.password, ]); + expect(to.pathname).not.toBe(from.pathname); }); - it('runs the hook on EVERY pooled connection, not just the first', async () => { - // A pool opens connections lazily and forever: a connection created late in - // a file must land in the same schema as the first one, or half the file - // silently runs against the shared default database. + it.skipIf(!MYSQL_CELL.available)( + 'mysql: the provisioned cell connects to this file’s database, with no pool hook', + () => { + const config: any = MYSQL_CELL.config(); + expect(config.client).toBe('mysql2'); + expect(typeof config.connection, 'a URL string, so the driver’s connect handling applies') + .toBe('string'); + expect(new URL(config.connection).pathname).toBe(`/${schema}`); + // nothing switches databases behind knex's back + expect(config.pool?.afterCreate).toBeUndefined(); + }, + ); + + it('postgres: knex is pointed at the file’s schema', () => { const config: any = PG_CELL.config(); - expect(await runAfterCreate(config)).toEqual(await runAfterCreate(config)); + expect(config.client).toBe('pg'); + expect(config.searchPath).toEqual([schema]); }); - it('the two live dialects do not resolve to one shared name', () => { - // Same file, so the NAME is deliberately shared across dialects — what must - // not happen is two FILES sharing one. This pins the axis the isolation is - // on, so a future edit that keyed off the dialect instead of the file is red. + it('two different files do not resolve to one shared name', () => { const other = liveSchemaNameFor('packages/drivers/driver-sql/src/some-other-live.test.ts'); expect(other).not.toBe(schema); }); }); + +describe('live-dialect matrix — the globalSetup creates what the cells connect to (#9350)', () => { + // The ordering constraint the connection-named database buys: MySQL refuses + // the handshake for a database that does not exist, so a file whose schema the + // globalSetup never created fails at connect. One derivation feeds both, and + // this asserts it really is one. + it('the ledger covers every test file in the package, with distinct schemas', () => { + const ledger = liveSchemaLedger(); + expect(ledger.length).toBe(TEST_FILE_KEYS.length); + expect(new Set(ledger.map((e) => e.schema)).size).toBe(ledger.length); + }); + + it('the ledger’s name for a file is the same one that file resolves for itself', () => { + const own = liveSchemaLedger().find((e) => e.file === 'live-dialect-matrix.isolation.test.ts'); + expect(own?.schema).toBe(currentLiveSchema()); + }); + + it('the cells connect to a schema the ledger knows', () => { + const known = new Set(liveSchemaLedger().map((e) => e.schema)); + expect(known.has((PG_CELL.config() as any).searchPath[0])).toBe(true); + expect(known.has(new URL(mysqlUrlForSchema('mysql://u:p@h:3306/c', currentLiveSchema())) + .pathname.slice(1))).toBe(true); + }); +}); + +describe('live-dialect matrix — the driver can SEE its own isolated schema (#9350)', () => { + // The other half of per-file isolation, and the half that fails silently. + // + // Moving the suites into their own schema is only safe if the driver's + // introspection follows them there. Postgres' index read pinned the schema + // literally (`n.nspname = 'public'`), so under an isolated search_path it + // returned `[]` for a table that measurably HAD a primary key and a declared + // unique index — and `[]` does not read as "I could not see", it reads as + // "there are no indexes", which `assertConflictTargetHonoured` turns into a + // refusal. A fail-open on an identity check, invisible to every existing test + // because the suites that exercise that path are MySQL-gated. + // + // Needs a live Postgres: this asserts what the SERVER reports, which is the + // only place the defect existed. + let driver: SqlDriver | undefined; + const TABLE = 'os9350_introspection_probe'; + + afterEach(async () => { + await driver?.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver?.disconnect().catch(() => {}); + driver = undefined; + }); + + it.skipIf(!PG_CELL.available)( + 'postgres: index introspection reports the indexes that exist in the file’s schema', + async () => { + driver = new SqlDriver(PG_CELL.config()); + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.initObjects([ + { name: TABLE, fields: { email: { type: 'string', unique: true } } }, + ] as any); + + // What the server says is really there — the control the assertion is + // measured against, so a green here cannot mean "nothing was created". + const live: any = await driver.execute( + `select i.relname as name + from pg_index ix + join pg_class i on i.oid = ix.indexrelid + where ix.indrelid = to_regclass(?)`, + [TABLE] as any, + ); + const actual = (live.rows ?? live).map((r: any) => r.name).sort(); + expect(actual.length, 'fixture is vacuous unless the table really has indexes') + .toBeGreaterThanOrEqual(2); + + const seen = await (driver as any).introspectIndexes(TABLE); + expect( + seen.map((i: any) => i.name).sort(), + 'the driver read a different set of indexes than the server holds — it is looking in ' + + 'another schema, and an empty read here becomes a silent upsert refusal', + ).toEqual(actual); + expect(seen.some((i: any) => i.primary)).toBe(true); + expect(seen.some((i: any) => i.unique && !i.primary)).toBe(true); + }, + ); + + it.skipIf(!PG_CELL.available)( + 'postgres: schema introspection lists a table created in the file’s schema', + async () => { + driver = new SqlDriver(PG_CELL.config()); + await driver.execute(`drop table if exists ${TABLE}`).catch(() => {}); + await driver.initObjects([{ name: TABLE, fields: { email: { type: 'string' } } }] as any); + const introspected = await driver.introspectSchema(); + expect(Object.keys(introspected.tables ?? {})).toContain(TABLE); + }, + ); +}); diff --git a/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts b/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts index 18c0533ba7..cd16016c97 100644 --- a/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.testkit.ts @@ -44,8 +44,9 @@ import { describe, expect, it } from 'vitest'; import { createHash } from 'node:crypto'; -import { existsSync } from 'node:fs'; +import { existsSync, readdirSync } from 'node:fs'; import { basename, dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; import type { SqlDriver, SqlDriverConfig } from './sql-driver.js'; /** The dialects `driver-sql` speaks that the matrices are run across. */ @@ -225,46 +226,48 @@ export function currentLiveSchema(): string { } /** - * Postgres: create the file's schema and point the session at it. + * The same MySQL URL, pointed at the file's own database. * - * Both halves run on EVERY pooled connection because a pool opens connections - * lazily and forever — a second connection created an hour into the run must - * land in the same schema as the first. `if not exists` makes the repeat free - * and makes two connections of the same file racing each other a no-op. + * The database is named in the CONNECTION, not switched afterwards with `use`, + * and that distinction is the whole defect this replaced. Measured on a live + * server: with `use` the SESSION moves, but knex's `client.database()` keeps + * returning the URL's database, and knex binds THAT into `columnInfo` + * (`mysql-querycompiler.js`: `table_schema = ?` from `this.client.database()`). + * So DDL executed in the per-file database while the column read answered from + * `conformance` — the driver saw an empty column set for a fully populated + * table and emitted `alter table ... add label`, which the server rejected with + * *Duplicate column name*. Naming the database in the URL keeps the two halves + * the same value by construction; there is no second place to disagree. * - * `searchPath` is ALSO set in the knex config (below) so knex's own schema - * builder qualifies its DDL; knex applies that before this hook runs, against a - * schema that may not exist yet — which Postgres permits, since `search_path` - * entries are resolved per statement, not at `SET` time. + * Consequence, and the reason {@link liveSchemaLedger} and the globalSetup + * exist: connecting to a database that does not exist fails at the handshake, + * so the databases must be created BEFORE any pool opens. */ -function pgAfterCreate(schema: string) { - return (conn: any, done: (err: unknown, conn: unknown) => void): void => { - conn.query(`create schema if not exists "${schema}"`, (err: unknown) => { - if (err) return done(err, conn); - conn.query(`set search_path to "${schema}"`, (err2: unknown) => done(err2, conn)); - }); - }; +export function mysqlUrlForSchema(url: string, schema: string): string { + const u = new URL(url); + u.pathname = `/${schema}`; + return u.toString(); } +/** Repo-relative prefix of this package's `src` — the key the ledger hashes. */ +const PACKAGE_SRC_PREFIX = 'packages/drivers/driver-sql/src/'; + /** - * MySQL: create the file's database and switch the session into it. + * Every test file in this package, paired with the schema it owns. * - * MySQL has no schema-inside-a-database, so a database IS the unit of - * isolation — `create database` is the same cheap dictionary write `create - * schema` is on Postgres, not a template copy. - * - * `use` rather than rewriting the connection URL, so the URL a runner - * provisioned stays the thing we connect to and only the SESSION moves. It also - * keeps `database()` — which several suites compare `information_schema.columns` - * against — reporting the isolated database. + * ONE derivation, read off disk, shared by the three things that must agree: + * the globalSetup that CREATES the schemas, the cells that CONNECT to them, and + * `live-dialect-matrix.isolation.test.ts` which asserts they are DISTINCT. + * Three hand-kept lists would be three chances for a new live file to be handed + * a connection to a database nobody created — and on MySQL that is a handshake + * failure, not a silent fallback. */ -function mysqlAfterCreate(schema: string) { - return (conn: any, done: (err: unknown, conn: unknown) => void): void => { - conn.query(`create database if not exists \`${schema}\``, (err: unknown) => { - if (err) return done(err, conn); - conn.query(`use \`${schema}\``, (err2: unknown) => done(err2, conn)); - }); - }; +export function liveSchemaLedger(): { file: string; schema: string }[] { + const dir = fileURLToPath(new URL('.', import.meta.url)); + return readdirSync(dir) + .filter((f) => f.endsWith('.test.ts')) + .sort() + .map((f) => ({ file: f, schema: liveSchemaNameFor(`${PACKAGE_SRC_PREFIX}${f}`) })); } /** @@ -293,17 +296,14 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!PG_URL, live: true, hasLegacyStorageForm: false, - config: () => { - const schema = currentLiveSchema(); - return { - client: 'pg', - connection: PG_URL, - // knex qualifies its own DDL with this; `pgAfterCreate` creates the - // schema and re-points the session, so raw SQL in a suite lands there too. - searchPath: [schema], - pool: { afterCreate: pgAfterCreate(schema) }, - }; - }, + config: () => ({ + client: 'pg', + connection: PG_URL, + // knex issues `set search_path` from this on every pooled connection and + // qualifies its own DDL with it, so a suite's raw SQL lands there too. The + // schema itself is created by the globalSetup, before any pool opens. + searchPath: [currentLiveSchema()], + }), }, { id: 'mysql', @@ -313,14 +313,13 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!MYSQL_URL, live: true, hasLegacyStorageForm: false, - config: () => { - const schema = currentLiveSchema(); - return { - client: 'mysql2', - connection: MYSQL_URL, - pool: { afterCreate: mysqlAfterCreate(schema) }, - }; - }, + config: () => ({ + client: 'mysql2', + // A URL STRING deliberately: the driver's own connect handling — the UTC + // session pin of #3942 and the connect bound of #3769 — keys off that + // shape, and a hand-built connection object would quietly opt out of both. + connection: mysqlUrlForSchema(MYSQL_URL!, currentLiveSchema()), + }), }, ] as const; diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 8b1dfcede5..51faa25379 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -9338,15 +9338,26 @@ export class SqlDriver implements IDataDriver { if (r.partial === 1 || r.partial === true) entry.partial = true; } } else if (this.isPostgres) { + // `to_regclass` resolves the name the way every other statement in the + // session does — first match along `search_path` — instead of pinning + // one schema literally. The literal was `n.nspname = 'public'`, which is + // the same answer for a default deployment (`current_schema()` IS + // `public` there) and NO answer at all for a session pointed anywhere + // else: measured on a live Postgres 16 under a non-public search_path, + // this returned `[]` for a table carrying a primary key AND a declared + // unique index. Empty here does not read as "I could not see"; it reads + // as "there are no indexes", which is what `assertConflictTargetHonoured` + // turns into a refusal — a silent fail-open on the identity check. + // Resolving by OID also removes the ambiguity a schema list would add: + // two schemas on the path can hold the same table name, and only one of + // them is the one a query would hit. const res: any = await this.knex.raw( `SELECT i.relname AS index_name, ix.indisunique AS is_unique, ix.indisprimary AS is_primary, (ix.indpred IS NOT NULL) AS is_partial, pg_get_indexdef(ix.indexrelid) AS indexdef - FROM pg_class t - JOIN pg_namespace n ON n.oid = t.relnamespace - JOIN pg_index ix ON t.oid = ix.indrelid + FROM pg_index ix JOIN pg_class i ON i.oid = ix.indexrelid - WHERE t.relname = ? AND n.nspname = 'public' + WHERE ix.indrelid = to_regclass(?) ORDER BY i.relname`, [tableName], ); @@ -9663,10 +9674,14 @@ export class SqlDriver implements IDataDriver { let tableNames: string[] = []; if (this.isPostgres) { + // Every schema the session can actually reach unqualified, rather than + // the literal `'public'` — same answer for a default deployment, where + // `current_schemas(false)` is exactly `{public}`, and the right answer for + // a session pointed at another schema, where the literal listed nothing. const result = await this.knex.raw(` SELECT table_name FROM information_schema.tables - WHERE table_schema = 'public' + WHERE table_schema = ANY (current_schemas(false)) AND table_type = 'BASE TABLE' `); tableNames = result.rows.map((row: any) => row.table_name); diff --git a/packages/drivers/driver-sql/vitest.config.ts b/packages/drivers/driver-sql/vitest.config.ts index 8c2b05f5a1..3d5689d336 100644 --- a/packages/drivers/driver-sql/vitest.config.ts +++ b/packages/drivers/driver-sql/vitest.config.ts @@ -7,6 +7,14 @@ export default defineConfig({ test: { globals: true, environment: 'node', + // #9350: the live-dialect files each own a schema (Postgres) / database + // (MySQL), named in the connection so knex's `client.database()` and the + // session cannot disagree. That naming has an ordering constraint — MySQL + // refuses the handshake for a database that does not exist — and a + // `beforeAll` cannot satisfy it: `cell.config()` is reached from inside + // `beforeEach`, and the testkit module is cached per WORKER, not per file. + // This runs once, before any worker, and is a no-op without a live URL. + globalSetup: ['./src/live-dialect-matrix.globalsetup.ts'], }, resolve: { alias: [ From 80589de9a77947f48a18940836eba5884d61f0f2 Mon Sep 17 00:00:00 2001 From: os-elon Date: Thu, 20 Aug 2026 20:10:33 +0000 Subject: [PATCH 3/3] chore(changeset): the PG introspection fix is user-visible (#9350) Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../driver-sql-pg-introspection-search-path.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/driver-sql-pg-introspection-search-path.md diff --git a/.changeset/driver-sql-pg-introspection-search-path.md b/.changeset/driver-sql-pg-introspection-search-path.md new file mode 100644 index 0000000000..89030fa8ab --- /dev/null +++ b/.changeset/driver-sql-pg-introspection-search-path.md @@ -0,0 +1,14 @@ +--- +"@objectstack/driver-sql": patch +--- + +**Bug fix:** on Postgres, index and schema introspection now resolve tables the way the session does, instead of assuming the `public` schema (#9350). + +`introspectIndexes` pinned `n.nspname = 'public'` and `introspectSchema` pinned `table_schema = 'public'`. For a driver whose connection carries a `searchPath` pointing anywhere else, both returned **empty** — not an error, an empty result. Measured on a live Postgres 16: for a table carrying a primary key *and* a declared unique index, `introspectIndexes` returned `[]` and `introspectSchema` listed no tables at all. + +Empty does not read as "I could not see" downstream; it reads as "there are no indexes". `assertConflictTargetHonoured` turns that into a refusal, so an `upsert` against a perfectly well-indexed table would be rejected with *no PRIMARY KEY or UNIQUE index backs them* — and index-drift detection would propose creating indexes that already exist. + +- `introspectIndexes` now resolves the table with `to_regclass(?)` and reads `pg_index` by OID. That is the same resolution every other statement in the session performs — first match along `search_path` — and it removes an ambiguity a schema list would introduce, since two schemas on the path can hold the same table name and only one of them is the one a query reaches. +- `introspectSchema` now lists `table_schema = ANY (current_schemas(false))`. + +**No change for a default deployment.** With the default `search_path`, `current_schemas(false)` is exactly `{public}` and `to_regclass` resolves into `public`, so both queries return what they returned before. The behaviour only differs where the old queries returned nothing.