Skip to content

Commit 7adcd07

Browse files
os-warrenclaude
andauthored
fix(driver-sql): a PRIMARY KEY is not a unique constraint on the SQLite arm (#11654) (#11827)
`introspectUniqueConstraints` reached its answer from a different catalog per dialect, and the catalogs disagreed about primary keys. Postgres and MySQL filter on `CONSTRAINT_TYPE = 'UNIQUE'`, which never matches one. The SQLite arm iterated `PRAGMA index_list` keyed only on `unique === 1`, never on `origin` — and SQLite materialises a non-INTEGER primary key as a unique auto-index, so the key column was reported. SQLite also disagreed with itself: an `INTEGER PRIMARY KEY` is a rowid alias for which no auto-index exists, so `index_list` is empty and the key was never flagged. The same logical schema produced a different `isUnique` from the declared type of its key alone. The arm now skips `origin: 'pk'` rows, which closes both gaps at once (`WITHOUT ROWID` keys included). This continues the convention #11202 landed: `isUnique` means a declared single-column UNIQUE constraint. Nothing is lost — key membership is reported losslessly by `primaryKeys` and `IntrospectedColumn.primaryKey`, both unmoved. The test is on the INDEX's origin, not on whether the COLUMN is in the key: those are different changes and only one is correct. A key column that separately carries its own unique index really does have a declared single-column unique constraint and stays flagged — pinned, because it fails under the wrong sibling implementation. Measured before the fix on embedded better-sqlite3: `t_text` (varchar key) reported `['email', 'id']`, `t_int` (INTEGER key) reported `[]`. Four pins go red against the unfixed arm and green after it; the rest of the package suite is unmoved (1972 -> 1976 passed, 0 failed). Claude-Session: https://claude.ai/code/session_01Rxnd8cyFnoU8V5y21PaTsy Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8b13cc8 commit 7adcd07

3 files changed

Lines changed: 293 additions & 1 deletion

File tree

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@objectstack/driver-sql": patch
3+
---
4+
5+
`introspectUniqueConstraints` no longer reports a PRIMARY KEY column as unique on SQLite, so all three dialects now answer the same question (#11654). The SQLite arm read `PRAGMA index_list` keyed only on `unique === 1`, and SQLite materialises a non-INTEGER primary key as a unique auto-index — so a `varchar` key was reported while the Postgres and MySQL arms, which filter on `CONSTRAINT_TYPE = 'UNIQUE'`, never see a primary key at all. It also disagreed with itself: an `INTEGER PRIMARY KEY` is a rowid alias with no auto-index, so the same logical schema produced a different `isUnique` flag depending only on the declared type of its key. The arm now skips `origin: 'pk'` index rows, which closes both gaps at once (`WITHOUT ROWID` keys included).
6+
7+
This continues #11202's convention: `isUnique` means a *declared single-column UNIQUE constraint*. Nothing is lost — primary-key membership is still reported losslessly through `IntrospectedTable.primaryKeys` and `IntrospectedColumn.primaryKey`. The filter is on the index's `origin`, not on whether the column is in the key, so a key column that separately carries its own unique index stays flagged.
8+
9+
Consumer-visible effect: `introspectedSchemaToObjects` in `@objectstack/objectql` turns this flag into a drafted field's `unique: true`, so a federated-object draft (ADR-0015) taken from a SQLite table no longer gains a redundant `unique: true` on its key column that the same table drafted through Postgres or MySQL never had. Drivers extending `SqlDriver` (`driver-turso`, `driver-sqlite-wasm`) inherit the change.
Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#11654] A PRIMARY KEY is not a UNIQUE constraint to
5+
* `introspectUniqueConstraints` — on any dialect, and for any key type.
6+
*
7+
* This is the residual cell of the family #11202 opened. That change unified
8+
* the three arms on *single-column* uniqueness; it deliberately left the
9+
* primary-key question alone, because the flag it produced was not false, only
10+
* inconsistent — and its fixture carries no primary key at all, precisely so it
11+
* measured the composite-vs-single question and nothing else.
12+
*
13+
* ## The convention this inherits, and its reason
14+
*
15+
* `isUnique` means **a declared single-column UNIQUE constraint**. Primary-key
16+
* membership is already reported losslessly through a face of its own —
17+
* `IntrospectedTable.primaryKeys` and `IntrospectedColumn.primaryKey` — so
18+
* excluding keys from `isUnique` loses no information and leaves the two flags
19+
* non-overlapping. That is #11202's convention applied one cell over, not a new
20+
* decision, which is why this file pins the `primaryKeys` face as well: it is
21+
* the half that makes the exclusion lossless rather than merely narrower.
22+
*
23+
* ## What was measured before the fix (2026-08-24, embedded better-sqlite3)
24+
*
25+
* The catalogs disagreed. Postgres and MySQL filter on
26+
* `CONSTRAINT_TYPE = 'UNIQUE'`, which excludes primary keys outright. SQLite
27+
* iterated `PRAGMA index_list` keyed only on `idx.unique === 1`, never on
28+
* `origin` — and SQLite materialises a non-INTEGER primary key as a unique
29+
* auto-index:
30+
*
31+
* ```text
32+
* create table t_text (id varchar(64) primary key, email varchar(64) unique)
33+
* PRAGMA index_list(t_text) ->
34+
* { name: 'sqlite_autoindex_t_text_2', unique: 1, origin: 'u' }
35+
* { name: 'sqlite_autoindex_t_text_1', unique: 1, origin: 'pk' }
36+
* introspectUniqueConstraints -> ['email', 'id'] <-- 'id' is the PRIMARY KEY
37+
*
38+
* create table t_int (id integer primary key, note varchar(64))
39+
* PRAGMA index_list(t_int) -> []
40+
* introspectUniqueConstraints -> []
41+
* ```
42+
*
43+
* So SQLite disagreed with the other two dialects AND with itself: an
44+
* `INTEGER PRIMARY KEY` is a rowid alias with no auto-index and was never
45+
* flagged, while a `varchar` key was — the same logical schema producing
46+
* different `isUnique` flags from the declared type of its key alone.
47+
*
48+
* ## The fix filters the INDEX by origin, not the COLUMN by key membership
49+
*
50+
* Those are different changes and only one is correct. A column that is the
51+
* primary key AND separately carries its own unique index really does have a
52+
* declared single-column unique constraint, and must stay flagged; dropping
53+
* every primary-key *column* would lose it. `t_pk_and_idx` below is that
54+
* distinction as a pin — it fails under the wrong sibling implementation and
55+
* passes under this one.
56+
*/
57+
58+
import { describe, it, expect, beforeAll, afterAll } from 'vitest';
59+
import { SqlDriver } from './sql-driver.js';
60+
import { DIALECT_CELLS, declareDialectCell, type DialectCell } from './live-dialect-matrix.testkit.js';
61+
62+
const MATRIX = 'primary key is not a unique constraint';
63+
64+
/** A varchar primary key plus a real single-column `UNIQUE (email)`. */
65+
const TABLE = 'os11654_pk';
66+
67+
/** `introspectUniqueConstraints` is `protected`; this is the narrowest reach. */
68+
class UniqueProbeDriver extends SqlDriver {
69+
uniqueConstraints(table: string) {
70+
return this.introspectUniqueConstraints(table);
71+
}
72+
}
73+
74+
// ── Half 1: every provisioned dialect answers the same ──────────────────────
75+
76+
function declarePrimaryKeyUniqueSuite(cell: DialectCell): void {
77+
describe(`introspectUniqueConstraints — a PRIMARY KEY is not unique — ${cell.label} (#11654)`, () => {
78+
let driver: UniqueProbeDriver;
79+
80+
beforeAll(async () => {
81+
driver = new UniqueProbeDriver(cell.config());
82+
await driver.execute(`drop table if exists ${TABLE}`).catch(() => {});
83+
// A NON-INTEGER key on purpose: this is the shape SQLite materialises as
84+
// a unique auto-index, and therefore the only shape on which the three
85+
// dialects ever disagreed.
86+
await driver.execute(
87+
`create table ${TABLE} (
88+
id varchar(64) not null primary key,
89+
email varchar(64) not null unique,
90+
note varchar(64)
91+
)`,
92+
);
93+
});
94+
95+
afterAll(async () => {
96+
await driver.execute(`drop table if exists ${TABLE}`).catch(() => {});
97+
await driver.disconnect().catch(() => {});
98+
});
99+
100+
it('the fixture is real: the key IS enforced and so is the UNIQUE column', async () => {
101+
// Non-vacuity, asserted against the server rather than the catalog. The
102+
// interesting assertion below is an ABSENCE, which goes green for free on
103+
// a table whose key never landed — so prove both constraints exist and
104+
// are enforced first. Note what this establishes: `id` really is unique
105+
// in the database. The flag's absence is a statement about what KIND of
106+
// constraint makes it so, not a claim that duplicates are allowed.
107+
await driver.execute(`insert into ${TABLE} (id, email) values ('k1', 'e1@example.com')`);
108+
109+
// The key repeated — REJECTED, so the PRIMARY KEY is enforced.
110+
await expect(
111+
driver.execute(`insert into ${TABLE} (id, email) values ('k1', 'e2@example.com')`),
112+
).rejects.toThrow();
113+
114+
// `email` repeated — REJECTED, so the single-column UNIQUE exists too.
115+
await expect(
116+
driver.execute(`insert into ${TABLE} (id, email) values ('k2', 'e1@example.com')`),
117+
).rejects.toThrow();
118+
});
119+
120+
it('reports the UNIQUE column and NOT the primary-key column', async () => {
121+
const columns = await driver.uniqueConstraints(TABLE);
122+
123+
expect(columns).toContain('email');
124+
expect(columns).not.toContain('id');
125+
expect(columns).not.toContain('note');
126+
// Exact, so a dialect that starts reporting something extra is caught
127+
// rather than absorbed by the `not.toContain`s above.
128+
expect(columns).toEqual(['email']);
129+
});
130+
131+
it('`introspectSchema` folds that into `isUnique` — the consumer-visible half', async () => {
132+
const schema = await driver.introspectSchema();
133+
const table = schema.tables[TABLE];
134+
expect(table, `${TABLE} missing from the introspected schema`).toBeDefined();
135+
136+
const byName = Object.fromEntries(table.columns.map((col) => [col.name, col]));
137+
expect(byName.email?.isUnique).toBe(true);
138+
// Falsy, not `false`: `isUnique` is only ever SET to `true`, so asserting
139+
// `false` would pin a shape the producer does not promise (#11202).
140+
expect(byName.id?.isUnique).toBeFalsy();
141+
expect(byName.note?.isUnique).toBeFalsy();
142+
});
143+
144+
it('nothing is lost: the `primaryKeys` face still reports the key', async () => {
145+
// This is what makes the exclusion lossless rather than a narrowing that
146+
// drops information. A consumer asking "is this column the key?" has an
147+
// answer that did not move, on both faces.
148+
const schema = await driver.introspectSchema();
149+
const table = schema.tables[TABLE];
150+
expect(table.primaryKeys).toEqual(['id']);
151+
152+
const byName = Object.fromEntries(table.columns.map((col) => [col.name, col]));
153+
expect(byName.id?.primaryKey).toBe(true);
154+
expect(byName.email?.primaryKey).toBeFalsy();
155+
});
156+
});
157+
}
158+
159+
for (const cell of DIALECT_CELLS) {
160+
declareDialectCell(cell, MATRIX, declarePrimaryKeyUniqueSuite);
161+
}
162+
163+
// ── Half 2: the SQLite key shapes no other dialect can produce ──────────────
164+
165+
describe('SQLite key materialisation — every key shape answers the same (#11654)', () => {
166+
let driver: UniqueProbeDriver;
167+
168+
beforeAll(async () => {
169+
driver = new UniqueProbeDriver({
170+
client: 'better-sqlite3',
171+
connection: { filename: ':memory:' },
172+
useNullAsDefault: true,
173+
});
174+
// The card's two tables, verbatim.
175+
await driver.execute(`create table t_text (id varchar(64) primary key, email varchar(64) unique)`);
176+
await driver.execute(`create table t_int (id integer primary key, note varchar(64))`);
177+
// A WITHOUT ROWID table still materialises its key as a `pk`-origin index.
178+
await driver.execute(
179+
`create table t_worid (id varchar(64) primary key, email varchar(64) unique) without rowid`,
180+
);
181+
// Composite key: two members, so the #11202 width filter already dropped
182+
// it. Pinned so the origin filter is not credited with it, and so it stays
183+
// dropped if the width filter is ever reworked.
184+
await driver.execute(
185+
`create table t_comp (a varchar(64), b varchar(64), email varchar(64) unique, primary key (a, b))`,
186+
);
187+
// The key column ALSO carrying its own unique index (`origin: 'c'`).
188+
await driver.execute(`create table t_pk_and_idx (id varchar(64) primary key, note varchar(64))`);
189+
await driver.execute(`create unique index t_pk_and_idx_id_u on t_pk_and_idx (id)`);
190+
});
191+
192+
afterAll(async () => {
193+
await driver.disconnect().catch(() => {});
194+
});
195+
196+
it('PRAGMA index_list really reports origin `pk` for a varchar key', async () => {
197+
// Pins the premise the filter rests on. If SQLite ever stopped tagging the
198+
// auto-index, the filter would be dead code and should be re-read.
199+
const rows: any = await driver.execute(`PRAGMA index_list(t_text)`);
200+
const byOrigin = Object.fromEntries((rows as any[]).map((r) => [r.origin, r]));
201+
expect(byOrigin.pk, 'no pk-origin auto-index — the premise moved').toBeDefined();
202+
expect(byOrigin.pk.unique).toBe(1);
203+
expect(byOrigin.u, 'no u-origin index for UNIQUE(email)').toBeDefined();
204+
});
205+
206+
it('an INTEGER key and a varchar key now agree — neither is flagged', async () => {
207+
// The self-inconsistency this card names: an INTEGER PRIMARY KEY is a rowid
208+
// alias with no auto-index and was never flagged, while a varchar key was.
209+
// The same logical schema must not answer differently by key type.
210+
expect(await driver.uniqueConstraints('t_text')).toEqual(['email']);
211+
expect(await driver.uniqueConstraints('t_int')).toEqual([]);
212+
});
213+
214+
it('a WITHOUT ROWID key is not flagged either', async () => {
215+
expect(await driver.uniqueConstraints('t_worid')).toEqual(['email']);
216+
});
217+
218+
it('a composite key contributes nothing, and its UNIQUE sibling survives', async () => {
219+
const columns = await driver.uniqueConstraints('t_comp');
220+
expect(columns).toEqual(['email']);
221+
expect(columns).not.toContain('a');
222+
expect(columns).not.toContain('b');
223+
});
224+
225+
it('a key column with its OWN unique index stays flagged', async () => {
226+
// The filter drops pk-ORIGIN INDEXES, not primary-key COLUMNS. `id` here
227+
// carries a separately declared single-column unique constraint, which is
228+
// exactly what `isUnique` means — dropping it would be a different, wrong
229+
// change wearing the same description.
230+
expect(await driver.uniqueConstraints('t_pk_and_idx')).toEqual(['id']);
231+
});
232+
});

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

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3698,6 +3698,16 @@ export interface IntrospectedColumn extends SpecIntrospectedColumn {
36983698
* option B and waits for demand; until it exists, an absent flag on a
36993699
* composite member means "not single-column unique", never "no constraint".
37003700
*
3701+
* A PRIMARY KEY is NOT a unique constraint to this flag (#11654), on any
3702+
* dialect and for any key type. `isUnique` means a *declared* single-column
3703+
* UNIQUE constraint; key membership has a lossless face of its own
3704+
* ({@link IntrospectedTable.primaryKeys} and `primaryKey` below), so
3705+
* excluding keys keeps the two flags non-overlapping and drops no fact. Note
3706+
* this is a statement about what KIND of constraint the flag reports, never
3707+
* a claim that a key column admits duplicates. A key column that separately
3708+
* carries its own single-column unique constraint is still flagged — the
3709+
* constraint is what is being reported, not the column.
3710+
*
37013711
* All three dialect arms of {@link SqlDriver.introspectUniqueConstraints}
37023712
* produce it through one predicate — see {@link singleColumnUniqueColumns}.
37033713
*/
@@ -14792,6 +14802,23 @@ export class SqlDriver implements IDataDriver {
1479214802
* All three now normalise their rows to {@link UniqueConstraintMember} and
1479314803
* decide through {@link singleColumnUniqueColumns} — one predicate, so a
1479414804
* fourth dialect cannot quietly acquire a fourth meaning.
14805+
*
14806+
* ## A PRIMARY KEY is not one of them (#11654)
14807+
*
14808+
* The residual cell of the same family, and the same convention applied one
14809+
* step over: a column appears here iff a *declared UNIQUE constraint* covers
14810+
* it alone. Postgres and MySQL got this for free — `CONSTRAINT_TYPE =
14811+
* 'UNIQUE'` never matches a primary key. SQLite did not: it keys on
14812+
* `PRAGMA index_list`, which reports the unique auto-index SQLite
14813+
* materialises for a non-INTEGER key, so a `varchar` key was flagged while
14814+
* an `INTEGER PRIMARY KEY` — a rowid alias with no auto-index at all — was
14815+
* not. Three dialects, four answers, from the declared type of a key.
14816+
*
14817+
* The SQLite arm now skips `origin: 'pk'` rows. Unlike the composite case
14818+
* above the excluded flag was not FALSE, only inconsistent — a key column
14819+
* really is unique — which is why the exclusion has to be paid for by the
14820+
* face that keeps the fact: `introspectPrimaryKeys` reports it through
14821+
* `primaryKeys` and `IntrospectedColumn.primaryKey`, both unmoved by this.
1479514822
*/
1479614823
protected async introspectUniqueConstraints(
1479714824
tableName: string,
@@ -14887,7 +14914,31 @@ export class SqlDriver implements IDataDriver {
1488714914
const indexes = await this.knex.raw(`PRAGMA index_list(${safeTableName})`);
1488814915

1488914916
for (const idx of indexes) {
14890-
if (idx.unique === 1) {
14917+
// `origin: 'pk'` is the auto-index SQLite materialises for a
14918+
// non-INTEGER PRIMARY KEY — not a declared UNIQUE constraint, and so
14919+
// not what this method reports (#11654). Skipping it is what makes
14920+
// SQLite agree with the other two arms, whose
14921+
// `CONSTRAINT_TYPE = 'UNIQUE'` filter never sees a primary key at
14922+
// all, AND what makes SQLite agree with ITSELF: an `INTEGER PRIMARY
14923+
// KEY` is a rowid alias for which SQLite creates no auto-index, so
14924+
// `index_list` is empty and the key was never flagged — the same
14925+
// logical schema answering differently by the declared type of its
14926+
// key alone. Nothing is lost by the exclusion: primary-key
14927+
// membership is reported losslessly by `introspectPrimaryKeys`,
14928+
// through `IntrospectedTable.primaryKeys` and
14929+
// `IntrospectedColumn.primaryKey`.
14930+
//
14931+
// The test is on the INDEX's origin, never on whether the COLUMN is
14932+
// in the key: a key column that separately carries its own unique
14933+
// index (`origin: 'c'`) really does have a declared single-column
14934+
// unique constraint and stays flagged.
14935+
//
14936+
// `origin` has been reported by this pragma since SQLite 3.8.9, so
14937+
// an absent value is not a case that arises here; were it ever
14938+
// absent this reads as the pre-#11654 behaviour rather than
14939+
// silently dropping real constraints, and the pin on
14940+
// `index_list`'s own origin values turns red.
14941+
if (idx.unique === 1 && idx.origin !== 'pk') {
1489114942
// `PRAGMA index_info` reports one row per index MEMBER, and a
1489214943
// member is not always a column: an expression term
1489314944
// (`CREATE UNIQUE INDEX … ON t (lower(a))`) arrives with

0 commit comments

Comments
 (0)