Skip to content

Extract the duplicated sqlite savepoint methods into one module #1013

Description

@sirtimid

Problem

createSavepoint, rollbackSavepoint, and releaseSavepoint are byte-for-byte identical in packages/kernel-store/src/sqlite/nodejs.ts and packages/kernel-store/src/sqlite/wasm.ts — roughly 83 lines each, comments included.

#1012 is the argument for extracting them: it wrote the same releaseSavepoint recovery block twice, the same nested abort-swallow twice, the same logging twice, and the same two tests twice. The hazard comment now appears four times in source and twice in tests. The next fix in this area will be written twice again — and a divergence between the two copies is silent data loss on one platform only.

Why the seam is clean

What genuinely differs already lives outside these three functions. beginIfNeeded / commitIfNeeded / rollbackIfNeeded differ substantially:

  • nodejs reads authoritative db.inTransaction from SQLite and uses .run()
  • wasm tracks _inTx itself and uses .step() / .reset()

That difference is exactly why #1012's third fix (_inTx = false before the abort) applies to one driver only. Injecting those three as parameters collapses the driver difference to a predicate and leaves the savepoint logic shared.

Sketch

A new packages/kernel-store/src/sqlite/savepoints.ts exporting makeSavepointMethods({ db, beginIfNeeded, commitIfNeeded, rollbackIfNeeded }), with rollbackSavepoint and releaseSavepoint both delegating to one endSavepoint(template, name, finish) — they differ only in the SQL template and in whether an emptied stack commits or aborts.

Each driver then drops ~83 lines for a single call plus a spread into its returned object. Net around −110 lines, hazard comment 4 copies → 1.

Worth folding in while touching this: _spStack is a lossy mirror of SQLite's real savepoint stack. ROLLBACK TO tN leaves tN alive in SQLite, but both drivers splice it away. Harmless today — the eventual RELEASE t0 sweeps the orphans — but the type claims a faithful stack and doesn't have one. An extracted module is the place to either fix that or say so.

Caveats

Surfaced by review agents on #1012.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions