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. 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 new file mode 100644 index 0000000000..bfd44646b2 --- /dev/null +++ b/packages/drivers/driver-sql/src/live-dialect-matrix.isolation.test.ts @@ -0,0 +1,318 @@ +// 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 { 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 { + LIVE_SCHEMA_PREFIX, + MYSQL_CELL, + PG_CELL, + currentLiveSchema, + liveSchemaLedger, + liveSchemaNameFor, + mysqlUrlForSchema, +} 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, ''); + + /** + * The needle is ASSEMBLED rather than written as a literal. + * + * 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 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) => ENV_READ.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 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'); + // 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'); + }); +}); + +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('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: 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.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(config.client).toBe('pg'); + expect(config.searchPath).toEqual([schema]); + }); + + 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 a547340f0f..cd16016c97 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,10 @@ */ import { describe, expect, it } from 'vitest'; +import { createHash } from 'node:crypto'; +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. */ @@ -91,6 +95,181 @@ 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)); +} + +/** + * The same MySQL URL, pointed at the file's own database. + * + * 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. + * + * 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. + */ +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/'; + +/** + * Every test file in this package, paired with the schema it owns. + * + * 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. + */ +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}`) })); +} + /** * 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 +296,14 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!PG_URL, live: true, hasLegacyStorageForm: false, - config: () => ({ client: 'pg', connection: PG_URL }), + 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', @@ -127,13 +313,44 @@ export const DIALECT_CELLS: readonly DialectCell[] = [ available: !!MYSQL_URL, live: true, hasLegacyStorageForm: false, - config: () => ({ client: 'mysql2', connection: MYSQL_URL }), + 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; /** 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 () => { 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: [