From 0cca415e852c1877af420c84e5c1d3677d13a606 Mon Sep 17 00:00:00 2001 From: kulesy Date: Thu, 28 May 2026 09:37:57 +1000 Subject: [PATCH 1/2] fix: migrate pre-2.0 .db files on open so ON CONFLICT(key) works MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit sqlite-level <2.0.0 created `kv (key TEXT, value TEXT)` with no UNIQUE constraint on `key` and inserted with a plain `INSERT INTO kv …`. Since 2.0 the inserts use `INSERT … ON CONFLICT(key) DO UPDATE`, which SQLite rejects unless `key` is UNIQUE — so opening a pre-2.0 file with 2.0+ threw `SqliteError: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint` on the first put/batch/clear. The CREATE TABLE IF NOT EXISTS in _open() is a no-op against an existing table and cannot retrofit the missing constraint. _open() now detects a missing single-column UNIQUE index on `key`, deduplicates any rows that share a key (last write wins — pre-2.0 _get returned the first matching row but on disk the last INSERT is the most recently written value, matching 2.0+ upsert semantics), and creates the UNIQUE INDEX. Idempotent: files created by 2.0+ already carry the column-level UNIQUE autoindex, so the detection query short-circuits and nothing changes. Tests construct a v1-format file with raw better-sqlite3 (including a duplicate-key case to exercise the dedupe path), open it through SqliteLevel, and assert put/batch/clear all succeed afterward; an idempotency test re-opens after migration; a freshly-created v2 test confirms no extra index is added on the happy path. With the fix removed, 3 of the 4 new tests fail with the exact production error. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/migrate-v1-format-on-open.md | 9 ++ src/index.test.ts | 144 ++++++++++++++++++++++++ src/index.ts | 35 ++++++ 3 files changed, 188 insertions(+) create mode 100644 .changeset/migrate-v1-format-on-open.md diff --git a/.changeset/migrate-v1-format-on-open.md b/.changeset/migrate-v1-format-on-open.md new file mode 100644 index 0000000..25a0dfe --- /dev/null +++ b/.changeset/migrate-v1-format-on-open.md @@ -0,0 +1,9 @@ +--- +"sqlite-level": patch +--- + +Fix `SqliteError: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE constraint` when opening a database originally created by sqlite-level <2.0.0. + +Pre-2.0 created the `kv` table as `(key TEXT, value TEXT)` with no UNIQUE constraint and inserted with a plain `INSERT INTO kv …`. Since 2.0 the inserts use `INSERT … ON CONFLICT(key) DO UPDATE`, which SQLite rejects unless `key` is UNIQUE — so opening a pre-2.0 file with 2.0+ threw on the first put/batch/clear, taking down whoever was awaiting the rejection. + +`_open()` now detects a missing UNIQUE index on `key`, deduplicates any rows that share a key (last write wins — pre-2.0 `_get` returned the first matching row, but on disk the last INSERT is the most recently written value, matching 2.0+ upsert semantics), and adds the UNIQUE index. The check is idempotent: files created by 2.0+ already have the column-level UNIQUE autoindex, so the migration short-circuits at the detection query. diff --git a/src/index.test.ts b/src/index.test.ts index e3fbdc8..d66a001 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -702,4 +702,148 @@ describe('sqlite-level', () => { expect(await level.get(keyWithBackslash)).toEqual(valueWithBackslash) }) }) + + describe('legacy v1-format migration (no UNIQUE on key)', () => { + // sqlite-level <2.0.0 created `kv (key TEXT, value TEXT)` with no UNIQUE + // constraint on `key`. Opening such a file with 2.0+ used to throw + // `SqliteError: ON CONFLICT clause does not match any PRIMARY KEY or + // UNIQUE constraint` on the first put/batch/clear. These tests cover the + // _open() backfill that adds the constraint (deduplicating rows first). + let tempDir: string + let dbPath: string + + beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'sqlite-level-v1-')) + dbPath = path.join(tempDir, 'legacy.db') + }) + + afterEach(() => { + fs.rmSync(tempDir, { recursive: true, force: true }) + }) + + // Materialise a database file the way sqlite-level <2.0.0 would have: kv + // table with no UNIQUE constraint, populated via plain INSERT (which is + // how pre-2.0 wrote, and which allows duplicate-key rows). + const seedV1Database = async ( + rows: Array<{ key: string; value: string }> + ) => { + const Database = (await import('better-sqlite3')).default + const raw = new Database(dbPath) + raw.exec('CREATE TABLE kv (key TEXT, value TEXT)') + const insert = raw.prepare('INSERT INTO kv (key, value) VALUES (?, ?)') + for (const { key, value } of rows) insert.run(key, value) + raw.close() + } + + it('put/batch/clear succeed against a v1-format file (no duplicates)', async () => { + await seedV1Database([ + { key: 'a', value: '1' }, + { key: 'b', value: '2' }, + ]) + + const level = new SqliteLevel({ filename: dbPath }) + await level.open() + + // Reads still work + expect(await level.get('a')).toEqual('1') + expect(await level.get('b')).toEqual('2') + + // The pre-fix failure mode: put fails because ON CONFLICT(key) has no + // matching UNIQUE constraint. After the migration this succeeds. + await level.put('a', '1-updated') + expect(await level.get('a')).toEqual('1-updated') + + await level.batch([ + { type: 'put', key: 'b', value: '2-updated' }, + { type: 'put', key: 'c', value: '3' }, + ]) + expect(await level.get('b')).toEqual('2-updated') + expect(await level.get('c')).toEqual('3') + + await level.clear() + await expect(level.get('a')).rejects.toThrow(keyNotFoundError('a')) + + await level.close() + }) + + it('deduplicates rows for keys that appear multiple times (last-write-wins)', async () => { + // Pre-2.0 INSERTs appended duplicate rows when the same key was put + // twice. _get used `stmt.get()` which returned the FIRST matching + // row, but on disk the LAST INSERT is the most recently written value. + // We pick MAX(rowid) to preserve "last write wins" — matches 2.0+ + // upsert semantics customers will expect after the bump. + await seedV1Database([ + { key: 'k1', value: 'first' }, + { key: 'k2', value: 'only' }, + { key: 'k1', value: 'middle' }, + { key: 'k1', value: 'latest' }, + ]) + + const level = new SqliteLevel({ filename: dbPath }) + await level.open() + + expect(await level.get('k1')).toEqual('latest') + expect(await level.get('k2')).toEqual('only') + + // Subsequent writes must still hit the UNIQUE path cleanly + await level.put('k1', 'rewritten') + expect(await level.get('k1')).toEqual('rewritten') + + // And the table should contain exactly one row per key + const entries: [string, string][] = [] + for await (const entry of level.iterator()) { + entries.push(entry as [string, string]) + } + expect(entries).toEqual([ + ['k1', 'rewritten'], + ['k2', 'only'], + ]) + + await level.close() + }) + + it('is idempotent: re-opening after migration is a no-op', async () => { + await seedV1Database([{ key: 'a', value: '1' }]) + + const first = new SqliteLevel({ filename: dbPath }) + await first.open() + await first.put('a', '1-updated') + await first.close() + + // Second open should detect the UNIQUE index from the first migration + // and skip the dedupe path entirely. + const second = new SqliteLevel({ filename: dbPath }) + await second.open() + expect(await second.get('a')).toEqual('1-updated') + await second.put('a', '1-twice') + expect(await second.get('a')).toEqual('1-twice') + await second.close() + }) + + it('does not touch freshly-created v2 files (migration short-circuits)', async () => { + // Open a fresh file via 2.0+ (no seeding), close, reopen. The migration + // detector must see the autoindex from the column-level UNIQUE and not + // attempt any DELETE/CREATE INDEX on subsequent opens. + const fresh = new SqliteLevel({ filename: dbPath }) + await fresh.open() + await fresh.put('x', 'y') + await fresh.close() + + // Confirm no explicit kv_key_unique index was created (only the + // sqlite_autoindex_* from the column-level UNIQUE). + const Database = (await import('better-sqlite3')).default + const raw = new Database(dbPath, { readonly: true }) + const indexes = raw + .prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'kv'") + .all() as Array<{ name: string }> + raw.close() + expect(indexes.some((i) => i.name === 'kv_key_unique')).toBe(false) + expect(indexes.some((i) => i.name.startsWith('sqlite_autoindex_kv'))).toBe(true) + + const reopened = new SqliteLevel({ filename: dbPath }) + await reopened.open() + expect(await reopened.get('x')).toEqual('y') + await reopened.close() + }) + }) }) diff --git a/src/index.ts b/src/index.ts index ed5944a..81ac9ca 100644 --- a/src/index.ts +++ b/src/index.ts @@ -226,6 +226,41 @@ export class SqliteLevel extends AbstractL async _open(options: AbstractOpenOptions, callback: (error?: Error) => void) { this.db.exec('CREATE TABLE IF NOT EXISTS kv (key TEXT UNIQUE, value TEXT)') + + // Backfill UNIQUE on `key` for databases originally created by sqlite-level + // <2.0.0. Pre-2.0 declared the table as `kv (key TEXT, value TEXT)` (no + // UNIQUE) and inserted with a plain `INSERT INTO kv …`. The 2.0 statements + // use `INSERT … ON CONFLICT(key) DO UPDATE`, which SQLite rejects unless + // `key` has a UNIQUE constraint or matching UNIQUE index — opening a + // pre-2.0 file without this backfill throws `SqliteError: ON CONFLICT + // clause does not match any PRIMARY KEY or UNIQUE constraint` on the + // first put/batch/clear. The `CREATE TABLE IF NOT EXISTS` above is a + // no-op against an existing table so cannot fix the missing constraint + // on its own. + // + // Detect by looking for a UNIQUE index whose single column is `key` + // (covers both the column-level `UNIQUE` autoindex written by ≥2.0 and + // any explicit UNIQUE INDEX). If absent, deduplicate (last write wins — + // pre-2.0 INSERTs appended duplicate rows but only the first row was + // ever returned by `_get`, so collapsing to MAX(rowid) preserves the + // most recently written value) and create the index. Idempotent: on a + // freshly-created v2 file this short-circuits at the SELECT. + const hasUniqueOnKey = this.db + .prepare( + `SELECT 1 FROM pragma_index_list('kv') il + WHERE il."unique" = 1 + AND (SELECT COUNT(*) FROM pragma_index_info(il.name)) = 1 + AND (SELECT name FROM pragma_index_info(il.name)) = 'key' + LIMIT 1` + ) + .get() + if (!hasUniqueOnKey) { + this.db.exec( + `DELETE FROM kv WHERE rowid NOT IN (SELECT MAX(rowid) FROM kv GROUP BY key); + CREATE UNIQUE INDEX kv_key_unique ON kv (key);` + ) + } + this.nextTick(callback) } From ead28e26c91a6835a3cf23de529f75e618f3220f Mon Sep 17 00:00:00 2001 From: kulesy Date: Fri, 29 May 2026 12:17:51 +1000 Subject: [PATCH 2/2] =?UTF-8?q?fix(=5Fopen):=20harden=20v1=20migration=20?= =?UTF-8?q?=E2=80=94=20readOnly=20gate,=20transaction,=20partial-index=20f?= =?UTF-8?q?ilter?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three follow-ups from PR review: 1. Skip the migration entirely when `readOnly: true`. _get works against the v1 schema unchanged, and _put/_batch/_clear are gated upstream, so the migration's writes are unnecessary and unwanted here (and would fail open on read-only storage). 2. Run DELETE + CREATE UNIQUE INDEX inside a BEGIN IMMEDIATE transaction via better-sqlite3's `.transaction().immediate()`, and use CREATE UNIQUE INDEX IF NOT EXISTS, so concurrent opens of the same legacy file serialise: the first migrates, subsequent ones wait on the write lock and the IF NOT EXISTS makes their attempt a no-op instead of crashing _open with "index kv_key_unique already exists". 3. Exclude partial unique indexes from the detector (`il.partial = 0`). A partial UNIQUE on `key` does not satisfy ON CONFLICT(key), so we shouldn't treat it as sufficient. New test asserts a readOnly open of a legacy duplicate-having file performs no DDL and no DELETE on disk, _get returns the pre-2.0 first-row value, and put rejects with LEVEL_READ_ONLY upstream. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/migrate-v1-format-on-open.md | 2 ++ src/index.test.ts | 47 +++++++++++++++++++++++++ src/index.ts | 41 +++++++++++++++------ 3 files changed, 79 insertions(+), 11 deletions(-) diff --git a/.changeset/migrate-v1-format-on-open.md b/.changeset/migrate-v1-format-on-open.md index 25a0dfe..3c6632c 100644 --- a/.changeset/migrate-v1-format-on-open.md +++ b/.changeset/migrate-v1-format-on-open.md @@ -7,3 +7,5 @@ Fix `SqliteError: ON CONFLICT clause does not match any PRIMARY KEY or UNIQUE co Pre-2.0 created the `kv` table as `(key TEXT, value TEXT)` with no UNIQUE constraint and inserted with a plain `INSERT INTO kv …`. Since 2.0 the inserts use `INSERT … ON CONFLICT(key) DO UPDATE`, which SQLite rejects unless `key` is UNIQUE — so opening a pre-2.0 file with 2.0+ threw on the first put/batch/clear, taking down whoever was awaiting the rejection. `_open()` now detects a missing UNIQUE index on `key`, deduplicates any rows that share a key (last write wins — pre-2.0 `_get` returned the first matching row, but on disk the last INSERT is the most recently written value, matching 2.0+ upsert semantics), and adds the UNIQUE index. The check is idempotent: files created by 2.0+ already have the column-level UNIQUE autoindex, so the migration short-circuits at the detection query. + +The migration is skipped entirely when the database is opened with `readOnly: true`, runs inside a `BEGIN IMMEDIATE` transaction with `CREATE UNIQUE INDEX IF NOT EXISTS` so concurrent opens of the same legacy file serialise cleanly, and ignores partial unique indexes (which don't satisfy `ON CONFLICT(key)`). diff --git a/src/index.test.ts b/src/index.test.ts index d66a001..5480cf0 100644 --- a/src/index.test.ts +++ b/src/index.test.ts @@ -820,6 +820,53 @@ describe('sqlite-level', () => { await second.close() }) + it('does not migrate when opened with readOnly: true', async () => { + // A consumer opening a legacy file in read-only mode should not have + // its file mutated. _get works against the v1 schema unchanged, and + // _put/_batch/_clear are gated by the readOnly check upstream — so the + // migration's writes are both unnecessary and unwanted here. + await seedV1Database([ + { key: 'a', value: '1' }, + { key: 'a', value: '2' }, + ]) + + const readOnlyLevel = new SqliteLevel({ + filename: dbPath, + readOnly: true, + }) + await readOnlyLevel.open() + + // Reads still work against the un-migrated v1 schema. Pre-2.0 _get + // returned the first matching row by rowid, which is preserved here + // because the migration didn't run. + expect(await readOnlyLevel.get('a')).toEqual('1') + + // Writes are rejected upstream, never reaching SQLite. + await expect(readOnlyLevel.put('a', '3')).rejects.toThrow( + new ModuleError('not authorized to write to branch', { + code: 'LEVEL_READ_ONLY', + }) + ) + + await readOnlyLevel.close() + + // Confirm the file on disk was not mutated: no kv_key_unique index, + // and the original duplicate rows are still present. + const Database = (await import('better-sqlite3')).default + const raw = new Database(dbPath, { readonly: true }) + const indexes = raw + .prepare( + "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'kv'" + ) + .all() as Array<{ name: string }> + const rowCount = ( + raw.prepare('SELECT COUNT(*) AS n FROM kv').get() as { n: number } + ).n + raw.close() + expect(indexes.some((i) => i.name === 'kv_key_unique')).toBe(false) + expect(rowCount).toEqual(2) + }) + it('does not touch freshly-created v2 files (migration short-circuits)', async () => { // Open a fresh file via 2.0+ (no seeding), close, reopen. The migration // detector must see the autoindex from the column-level UNIQUE and not diff --git a/src/index.ts b/src/index.ts index 81ac9ca..aed7f45 100644 --- a/src/index.ts +++ b/src/index.ts @@ -227,6 +227,16 @@ export class SqliteLevel extends AbstractL async _open(options: AbstractOpenOptions, callback: (error?: Error) => void) { this.db.exec('CREATE TABLE IF NOT EXISTS kv (key TEXT UNIQUE, value TEXT)') + // Read-only consumers don't need the UNIQUE constraint: `_get` works + // against the v1 schema unchanged, and `_put`/`_batch`/`_clear` are gated + // upstream by the `readOnly` check before they ever reach SQLite. Skipping + // the migration here keeps us from mutating a file the caller asked us + // not to touch (and avoids failing `_open` on read-only storage). + if (this.readOnly) { + this.nextTick(callback) + return + } + // Backfill UNIQUE on `key` for databases originally created by sqlite-level // <2.0.0. Pre-2.0 declared the table as `kv (key TEXT, value TEXT)` (no // UNIQUE) and inserted with a plain `INSERT INTO kv …`. The 2.0 statements @@ -238,27 +248,36 @@ export class SqliteLevel extends AbstractL // no-op against an existing table so cannot fix the missing constraint // on its own. // - // Detect by looking for a UNIQUE index whose single column is `key` - // (covers both the column-level `UNIQUE` autoindex written by ≥2.0 and - // any explicit UNIQUE INDEX). If absent, deduplicate (last write wins — - // pre-2.0 INSERTs appended duplicate rows but only the first row was - // ever returned by `_get`, so collapsing to MAX(rowid) preserves the - // most recently written value) and create the index. Idempotent: on a - // freshly-created v2 file this short-circuits at the SELECT. + // Detect by looking for a non-partial UNIQUE index whose single column is + // `key` (covers both the column-level `UNIQUE` autoindex written by ≥2.0 + // and any explicit UNIQUE INDEX; partial indexes don't satisfy + // ON CONFLICT(key) so we exclude them via `il.partial = 0`). If absent, + // deduplicate (last write wins — pre-2.0 INSERTs appended duplicate rows + // but only the first row was ever returned by `_get`, so collapsing to + // MAX(rowid) preserves the most recently written value) and create the + // index inside a `BEGIN IMMEDIATE` transaction so concurrent opens of the + // same legacy file serialise cleanly: the first one migrates, subsequent + // ones wait on the write lock, and `CREATE UNIQUE INDEX IF NOT EXISTS` + // makes the second attempt a no-op instead of a crash. Idempotent: on a + // freshly-created v2 file the detector short-circuits before any write. const hasUniqueOnKey = this.db .prepare( `SELECT 1 FROM pragma_index_list('kv') il WHERE il."unique" = 1 + AND il.partial = 0 AND (SELECT COUNT(*) FROM pragma_index_info(il.name)) = 1 AND (SELECT name FROM pragma_index_info(il.name)) = 'key' LIMIT 1` ) .get() if (!hasUniqueOnKey) { - this.db.exec( - `DELETE FROM kv WHERE rowid NOT IN (SELECT MAX(rowid) FROM kv GROUP BY key); - CREATE UNIQUE INDEX kv_key_unique ON kv (key);` - ) + const migrate = this.db.transaction(() => { + this.db.exec( + `DELETE FROM kv WHERE rowid NOT IN (SELECT MAX(rowid) FROM kv GROUP BY key); + CREATE UNIQUE INDEX IF NOT EXISTS kv_key_unique ON kv (key);` + ) + }) + migrate.immediate() } this.nextTick(callback)