Skip to content

Commit f59035c

Browse files
os-zhuangclaude
andauthored
fix(driver-sql): report every member of a SQLite composite primary key, in key order (#11104)
`SqlDriver.introspectPrimaryKeys` filtered `PRAGMA table_info` rows on `row.pk === 1`. 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, ...). The filter 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`, so a consumer could not cross-check its way back to the dropped member. Repairing the list repairs the flag with it. The rows are now also ordered by the `pk` ordinal instead of being taken in `table_info` row order (which is COLUMN order). The two differ whenever a key is declared out of column sequence, and `primaryKeys` is consumed as an addressing / upsert-conflict-target key where order is load-bearing. `SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and override neither method, so they inherit the repair. The Postgres and MySQL arms did not carry this defect and are unchanged. Claude-Session: https://claude.ai/code/session_01RfyXxZ2WPjcjhuXpiQQc3y Co-authored-by: Claude <noreply@anthropic.com>
1 parent 6cac9cd commit f59035c

3 files changed

Lines changed: 226 additions & 4 deletions

File tree

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
SQLite introspection now reports every member of a composite primary key, in
6+
declared key order. `SqlDriver.introspectPrimaryKeys` filtered
7+
`PRAGMA table_info` rows on `row.pk === 1`, but SQLite does not report `pk` as
8+
a boolean — it is the column's **1-based position within the primary key**
9+
(`0` = not part of the key, `1` = first key column, `2` = second, and so on).
10+
The filter therefore kept only the first member of a composite key and silently
11+
dropped the rest.
12+
13+
Measured on in-memory SQLite, table declared `primary key (order_id, line_no)`:
14+
15+
| signal | reported | reports instead |
16+
| --- | --- | --- |
17+
| `table.primaryKeys` | `['order_id']` | `['order_id', 'line_no']` |
18+
| `column.isPrimary` for `line_no` | `false` | `true` |
19+
20+
Both signals were wrong together and for the same reason: `introspectSchema`
21+
derives `col.isPrimary` from `primaryKeys`, so a consumer could not recover the
22+
dropped member by cross-checking the two. Fixing the list repairs the flag with
23+
it.
24+
25+
The rows are now also ordered by the `pk` ordinal rather than taken in
26+
`table_info` row order (which is *column* order). The two differ whenever a key
27+
is declared out of column sequence — a table with columns
28+
`(carrier_code, shipment_id, leg_seq)` and `primary key (shipment_id,
29+
carrier_code)` now reports `['shipment_id', 'carrier_code']` — and
30+
`primaryKeys` is consumed as an addressing / upsert-conflict-target key, where
31+
the order is load-bearing.
32+
33+
Consumers affected: the federated-object codegen and the persisted
34+
`external_catalog` (ADR-0015) recorded a partial addressing/upsert key, and
35+
schema-drift comparison against a declared composite key read as drift on the
36+
dropped member. `SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and
37+
override neither method, so they inherit the repair. The Postgres and MySQL
38+
arms did not have this defect and are unchanged.
Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,170 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Pin: SQLite introspection must report EVERY member of a composite primary
5+
* key, in DECLARED KEY ORDER.
6+
*
7+
* `PRAGMA table_info` does not report `pk` as a boolean. It reports the
8+
* column's **1-based position within the primary key** — `0` for "not part of
9+
* the key", `1` for the first key column, `2` for the second, and so on.
10+
* `introspectPrimaryKeys` previously filtered on `pk === 1`, which kept only
11+
* the first member of a composite key and silently dropped the rest.
12+
*
13+
* Both output signals were wrong together and for the same reason:
14+
* `introspectSchema` derives `col.isPrimary` FROM `primaryKeys`
15+
* (`if (primaryKeys.includes(col.name)) col.isPrimary = true`), so a consumer
16+
* could not recover the missing member by cross-checking the two. Both are
17+
* asserted here.
18+
*
19+
* `SqliteWasmDriver` and `TursoDriver` extend `SqlDriver` and override neither
20+
* `introspectPrimaryKeys` nor `introspectSchema`, so they inherit this arm.
21+
* Only the better-sqlite3 binding is executed here.
22+
*/
23+
24+
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
25+
import { SqlDriver } from '../src/index.js';
26+
27+
/**
28+
* The consumer-visible consequence, modelled locally: comparing a DECLARED
29+
* composite key against what introspection recovered. This is what
30+
* schema-drift comparison and the federated `external_catalog` addressing key
31+
* do with `primaryKeys` — an under-reported member reads as drift.
32+
*
33+
* Order-sensitive on purpose: `pk` is an ordinal, so a key reported in column
34+
* order rather than declared-key order is a different key for addressing and
35+
* upsert-conflict-target purposes.
36+
*/
37+
function compareDeclaredKey(declared: string[], introspected: string[]): string[] {
38+
const findings: string[] = [];
39+
for (const col of declared) {
40+
if (!introspected.includes(col)) findings.push(`missing_key_member:${col}`);
41+
}
42+
for (const col of introspected) {
43+
if (!declared.includes(col)) findings.push(`unexpected_key_member:${col}`);
44+
}
45+
if (findings.length === 0 && declared.join(',') !== introspected.join(',')) {
46+
findings.push(`key_order_mismatch:${declared.join(',')}!=${introspected.join(',')}`);
47+
}
48+
return findings;
49+
}
50+
51+
describe('SqlDriver composite primary-key introspection (SQLite)', () => {
52+
let driver: SqlDriver;
53+
let knexInstance: any;
54+
55+
beforeEach(async () => {
56+
driver = new SqlDriver({
57+
client: 'better-sqlite3',
58+
connection: { filename: ':memory:' },
59+
useNullAsDefault: true,
60+
});
61+
knexInstance = (driver as any).knex;
62+
});
63+
64+
afterEach(async () => {
65+
await knexInstance.destroy();
66+
});
67+
68+
it('pins the SQLite fact this repair rests on: `pk` is a 1-based ordinal, not a boolean', async () => {
69+
await knexInstance.schema.createTable('order_lines', (t: any) => {
70+
t.string('order_id').notNullable();
71+
t.integer('line_no').notNullable();
72+
t.string('sku');
73+
t.primary(['order_id', 'line_no']);
74+
});
75+
76+
const rows: any[] = await knexInstance.raw('PRAGMA table_info(order_lines)');
77+
const pkByName = Object.fromEntries(rows.map((r) => [r.name, r.pk]));
78+
79+
// If SQLite ever reported `pk` as a boolean, the fix below would be wrong.
80+
expect(pkByName).toEqual({ order_id: 1, line_no: 2, sku: 0 });
81+
});
82+
83+
it('reports every member of a composite key, and derives isPrimary for all of them', async () => {
84+
await knexInstance.schema.createTable('order_lines', (t: any) => {
85+
t.string('order_id').notNullable();
86+
t.integer('line_no').notNullable();
87+
t.string('sku');
88+
t.primary(['order_id', 'line_no']);
89+
});
90+
91+
const schema = await driver.introspectSchema();
92+
const table = schema.tables['order_lines'];
93+
94+
// Signal 1: the table-level list. `line_no` was dropped before the fix.
95+
expect(table.primaryKeys).toEqual(['order_id', 'line_no']);
96+
97+
// Signal 2: the per-column flag, derived FROM signal 1 — repaired with it.
98+
const isPrimaryByName = Object.fromEntries(table.columns.map((c) => [c.name, c.isPrimary === true]));
99+
expect(isPrimaryByName).toEqual({ order_id: true, line_no: true, sku: false });
100+
});
101+
102+
it('orders primaryKeys by pk ordinal, not by column position', async () => {
103+
// Declared key order (shipment_id, carrier_code) deliberately differs from
104+
// column order (carrier_code, shipment_id, leg_seq): iterating table_info
105+
// rows in row order would yield the columns in the wrong key order.
106+
await knexInstance.schema.createTable('shipment_legs', (t: any) => {
107+
t.string('carrier_code').notNullable();
108+
t.string('shipment_id').notNullable();
109+
t.integer('leg_seq');
110+
t.primary(['shipment_id', 'carrier_code']);
111+
});
112+
113+
const rows: any[] = await knexInstance.raw('PRAGMA table_info(shipment_legs)');
114+
// Row order is column order; the ordinals run against it.
115+
expect(rows.map((r) => [r.name, r.pk])).toEqual([
116+
['carrier_code', 2],
117+
['shipment_id', 1],
118+
['leg_seq', 0],
119+
]);
120+
121+
const schema = await driver.introspectSchema();
122+
expect(schema.tables['shipment_legs'].primaryKeys).toEqual(['shipment_id', 'carrier_code']);
123+
});
124+
125+
it('reads as NO drift when a declared composite key is compared against introspection', async () => {
126+
await knexInstance.schema.createTable('order_lines', (t: any) => {
127+
t.string('order_id').notNullable();
128+
t.integer('line_no').notNullable();
129+
t.string('sku');
130+
t.primary(['order_id', 'line_no']);
131+
});
132+
133+
const schema = await driver.introspectSchema();
134+
const introspected = schema.tables['order_lines'].primaryKeys;
135+
136+
expect(compareDeclaredKey(['order_id', 'line_no'], introspected)).toEqual([]);
137+
138+
// Negative control: the comparison above is a real detector, not a
139+
// vacuously-empty one. A genuinely different declared key still drifts.
140+
expect(compareDeclaredKey(['order_id', 'warehouse_id'], introspected)).toEqual([
141+
'missing_key_member:warehouse_id',
142+
'unexpected_key_member:line_no',
143+
]);
144+
expect(compareDeclaredKey(['line_no', 'order_id'], introspected)).toEqual([
145+
'key_order_mismatch:line_no,order_id!=order_id,line_no',
146+
]);
147+
});
148+
149+
it('still reports a single-column key exactly, and an unkeyed table as empty', async () => {
150+
await knexInstance.schema.createTable('widgets', (t: any) => {
151+
t.string('id').primary();
152+
t.string('name');
153+
});
154+
155+
// No primary key at all: every `pk` is 0. Guards the repair against
156+
// becoming `pk >= 0`, which would report every column as a key member.
157+
await knexInstance.schema.createTable('audit_lines', (t: any) => {
158+
t.string('actor');
159+
t.string('action');
160+
});
161+
162+
const schema = await driver.introspectSchema();
163+
164+
expect(schema.tables['widgets'].primaryKeys).toEqual(['id']);
165+
expect(schema.tables['audit_lines'].primaryKeys).toEqual([]);
166+
167+
const auditPrimary = schema.tables['audit_lines'].columns.map((c) => c.isPrimary === true);
168+
expect(auditPrimary).toEqual([false, false]);
169+
});
170+
});

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

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12690,10 +12690,24 @@ export class SqlDriver implements IDataDriver {
1269012690

1269112691
const result = await this.knex.raw(`PRAGMA table_info(${safeTableName})`);
1269212692

12693-
for (const row of result) {
12694-
if (row.pk === 1) {
12695-
primaryKeys.push(row.name);
12696-
}
12693+
// `PRAGMA table_info` does not report `pk` as a boolean. It reports the
12694+
// column's 1-based position WITHIN the primary key — `0` for "not part
12695+
// of the key", `1` for the first key column, `2` for the second, and so
12696+
// on. Filtering on `pk === 1` therefore kept only the first member of a
12697+
// composite key and silently dropped the rest, and because
12698+
// `introspectSchema` derives `col.isPrimary` FROM this list, both output
12699+
// signals were wrong together.
12700+
//
12701+
// Ordering by the ordinal (rather than taking `table_info`'s row order,
12702+
// which is COLUMN order) makes `primaryKeys` the declared key order. The
12703+
// two differ whenever a key is declared out of column sequence, and this
12704+
// list is used as an addressing / upsert-conflict-target key, where the
12705+
// order is load-bearing.
12706+
const keyedRows = (result as { name: string; pk: number }[]).filter((row) => row.pk > 0);
12707+
keyedRows.sort((a, b) => a.pk - b.pk);
12708+
12709+
for (const row of keyedRows) {
12710+
primaryKeys.push(row.name);
1269712711
}
1269812712
}
1269912713
} catch {

0 commit comments

Comments
 (0)