diff --git a/.changeset/sqlite-composite-primary-key-introspection.md b/.changeset/sqlite-composite-primary-key-introspection.md new file mode 100644 index 0000000000..0e7c3e0c39 --- /dev/null +++ b/.changeset/sqlite-composite-primary-key-introspection.md @@ -0,0 +1,38 @@ +--- +"@objectstack/driver-sql": patch +--- + +SQLite introspection now reports every member of a composite primary key, in +declared key order. `SqlDriver.introspectPrimaryKeys` filtered +`PRAGMA table_info` rows on `row.pk === 1`, but SQLite does not report `pk` as +a boolean — it is the column's **1-based position within the primary key** +(`0` = not part of the key, `1` = first key column, `2` = second, and so on). +The filter therefore kept only the first member of a composite key and silently +dropped the rest. + +Measured on in-memory SQLite, table declared `primary key (order_id, line_no)`: + +| signal | reported | reports instead | +| --- | --- | --- | +| `table.primaryKeys` | `['order_id']` | `['order_id', 'line_no']` | +| `column.isPrimary` for `line_no` | `false` | `true` | + +Both signals were wrong together and for the same reason: `introspectSchema` +derives `col.isPrimary` from `primaryKeys`, so a consumer could not recover the +dropped member by cross-checking the two. Fixing the list repairs the flag with +it. + +The rows are now also ordered by the `pk` ordinal rather than taken in +`table_info` row order (which is *column* order). The two differ whenever a key +is declared out of column sequence — a table with columns +`(carrier_code, shipment_id, leg_seq)` and `primary key (shipment_id, +carrier_code)` now reports `['shipment_id', 'carrier_code']` — and +`primaryKeys` is consumed as an addressing / upsert-conflict-target key, where +the order is load-bearing. + +Consumers affected: the federated-object codegen and the persisted +`external_catalog` (ADR-0015) recorded a partial addressing/upsert key, and +schema-drift comparison against a declared composite key read as drift on the +dropped member. `SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and +override neither method, so they inherit the repair. The Postgres and MySQL +arms did not have this defect and are unchanged. diff --git a/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts b/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts new file mode 100644 index 0000000000..49a51882f8 --- /dev/null +++ b/packages/drivers/driver-sql/src/sql-driver-composite-primary-key-introspection.test.ts @@ -0,0 +1,170 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Pin: SQLite introspection must report EVERY member of a composite primary + * key, in DECLARED KEY ORDER. + * + * `PRAGMA table_info` does not report `pk` as a boolean. It reports the + * column's **1-based position within the primary key** — `0` for "not part of + * the key", `1` for the first key column, `2` for the second, and so on. + * `introspectPrimaryKeys` previously filtered on `pk === 1`, which kept only + * the first member of a composite key and silently dropped the rest. + * + * Both output signals were wrong together and for the same reason: + * `introspectSchema` derives `col.isPrimary` FROM `primaryKeys` + * (`if (primaryKeys.includes(col.name)) col.isPrimary = true`), so a consumer + * could not recover the missing member by cross-checking the two. Both are + * asserted here. + * + * `SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and override neither + * `introspectPrimaryKeys` nor `introspectSchema`, so they inherit this arm. + * Only the better-sqlite3 binding is executed here. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { SqlDriver } from '../src/index.js'; + +/** + * The consumer-visible consequence, modelled locally: comparing a DECLARED + * composite key against what introspection recovered. This is what + * schema-drift comparison and the federated `external_catalog` addressing key + * do with `primaryKeys` — an under-reported member reads as drift. + * + * Order-sensitive on purpose: `pk` is an ordinal, so a key reported in column + * order rather than declared-key order is a different key for addressing and + * upsert-conflict-target purposes. + */ +function compareDeclaredKey(declared: string[], introspected: string[]): string[] { + const findings: string[] = []; + for (const col of declared) { + if (!introspected.includes(col)) findings.push(`missing_key_member:${col}`); + } + for (const col of introspected) { + if (!declared.includes(col)) findings.push(`unexpected_key_member:${col}`); + } + if (findings.length === 0 && declared.join(',') !== introspected.join(',')) { + findings.push(`key_order_mismatch:${declared.join(',')}!=${introspected.join(',')}`); + } + return findings; +} + +describe('SqlDriver composite primary-key introspection (SQLite)', () => { + let driver: SqlDriver; + let knexInstance: any; + + beforeEach(async () => { + driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + knexInstance = (driver as any).knex; + }); + + afterEach(async () => { + await knexInstance.destroy(); + }); + + it('pins the SQLite fact this repair rests on: `pk` is a 1-based ordinal, not a boolean', async () => { + await knexInstance.schema.createTable('order_lines', (t: any) => { + t.string('order_id').notNullable(); + t.integer('line_no').notNullable(); + t.string('sku'); + t.primary(['order_id', 'line_no']); + }); + + const rows: any[] = await knexInstance.raw('PRAGMA table_info(order_lines)'); + const pkByName = Object.fromEntries(rows.map((r) => [r.name, r.pk])); + + // If SQLite ever reported `pk` as a boolean, the fix below would be wrong. + expect(pkByName).toEqual({ order_id: 1, line_no: 2, sku: 0 }); + }); + + it('reports every member of a composite key, and derives isPrimary for all of them', async () => { + await knexInstance.schema.createTable('order_lines', (t: any) => { + t.string('order_id').notNullable(); + t.integer('line_no').notNullable(); + t.string('sku'); + t.primary(['order_id', 'line_no']); + }); + + const schema = await driver.introspectSchema(); + const table = schema.tables['order_lines']; + + // Signal 1: the table-level list. `line_no` was dropped before the fix. + expect(table.primaryKeys).toEqual(['order_id', 'line_no']); + + // Signal 2: the per-column flag, derived FROM signal 1 — repaired with it. + const isPrimaryByName = Object.fromEntries(table.columns.map((c) => [c.name, c.isPrimary === true])); + expect(isPrimaryByName).toEqual({ order_id: true, line_no: true, sku: false }); + }); + + it('orders primaryKeys by pk ordinal, not by column position', async () => { + // Declared key order (shipment_id, carrier_code) deliberately differs from + // column order (carrier_code, shipment_id, leg_seq): iterating table_info + // rows in row order would yield the columns in the wrong key order. + await knexInstance.schema.createTable('shipment_legs', (t: any) => { + t.string('carrier_code').notNullable(); + t.string('shipment_id').notNullable(); + t.integer('leg_seq'); + t.primary(['shipment_id', 'carrier_code']); + }); + + const rows: any[] = await knexInstance.raw('PRAGMA table_info(shipment_legs)'); + // Row order is column order; the ordinals run against it. + expect(rows.map((r) => [r.name, r.pk])).toEqual([ + ['carrier_code', 2], + ['shipment_id', 1], + ['leg_seq', 0], + ]); + + const schema = await driver.introspectSchema(); + expect(schema.tables['shipment_legs'].primaryKeys).toEqual(['shipment_id', 'carrier_code']); + }); + + it('reads as NO drift when a declared composite key is compared against introspection', async () => { + await knexInstance.schema.createTable('order_lines', (t: any) => { + t.string('order_id').notNullable(); + t.integer('line_no').notNullable(); + t.string('sku'); + t.primary(['order_id', 'line_no']); + }); + + const schema = await driver.introspectSchema(); + const introspected = schema.tables['order_lines'].primaryKeys; + + expect(compareDeclaredKey(['order_id', 'line_no'], introspected)).toEqual([]); + + // Negative control: the comparison above is a real detector, not a + // vacuously-empty one. A genuinely different declared key still drifts. + expect(compareDeclaredKey(['order_id', 'warehouse_id'], introspected)).toEqual([ + 'missing_key_member:warehouse_id', + 'unexpected_key_member:line_no', + ]); + expect(compareDeclaredKey(['line_no', 'order_id'], introspected)).toEqual([ + 'key_order_mismatch:line_no,order_id!=order_id,line_no', + ]); + }); + + it('still reports a single-column key exactly, and an unkeyed table as empty', async () => { + await knexInstance.schema.createTable('widgets', (t: any) => { + t.string('id').primary(); + t.string('name'); + }); + + // No primary key at all: every `pk` is 0. Guards the repair against + // becoming `pk >= 0`, which would report every column as a key member. + await knexInstance.schema.createTable('audit_lines', (t: any) => { + t.string('actor'); + t.string('action'); + }); + + const schema = await driver.introspectSchema(); + + expect(schema.tables['widgets'].primaryKeys).toEqual(['id']); + expect(schema.tables['audit_lines'].primaryKeys).toEqual([]); + + const auditPrimary = schema.tables['audit_lines'].columns.map((c) => c.isPrimary === true); + expect(auditPrimary).toEqual([false, false]); + }); +}); diff --git a/packages/drivers/driver-sql/src/sql-driver.ts b/packages/drivers/driver-sql/src/sql-driver.ts index 035fea7ef0..074283e965 100644 --- a/packages/drivers/driver-sql/src/sql-driver.ts +++ b/packages/drivers/driver-sql/src/sql-driver.ts @@ -12690,10 +12690,24 @@ export class SqlDriver implements IDataDriver { const result = await this.knex.raw(`PRAGMA table_info(${safeTableName})`); - for (const row of result) { - if (row.pk === 1) { - primaryKeys.push(row.name); - } + // `PRAGMA table_info` does not report `pk` as a boolean. It reports the + // column's 1-based position WITHIN the primary key — `0` for "not part + // of the key", `1` for the first key column, `2` for the second, and so + // on. Filtering on `pk === 1` therefore kept only the first member of a + // composite key and silently dropped the rest, and because + // `introspectSchema` derives `col.isPrimary` FROM this list, both output + // signals were wrong together. + // + // Ordering by the ordinal (rather than taking `table_info`'s row order, + // which is COLUMN order) makes `primaryKeys` the declared key order. The + // two differ whenever a key is declared out of column sequence, and this + // list is used as an addressing / upsert-conflict-target key, where the + // order is load-bearing. + const keyedRows = (result as { name: string; pk: number }[]).filter((row) => row.pk > 0); + keyedRows.sort((a, b) => a.pk - b.pk); + + for (const row of keyedRows) { + primaryKeys.push(row.name); } } } catch {