You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit da40a5f
Browse filesBrowse the repository at this point in the historyBrowse files
fix(driver-sql): connect reads PRAGMA auto_vacuum first and sets it only when the file is not already INCREMENTAL (#21744)
Fixes#21734
Clause-②: no
`SqlDriver.connect()` now reads `PRAGMA auto_vacuum` first, and runs
`PRAGMA auto_vacuum = INCREMENTAL` only when the file does not already
answer `2`. This applies to every connect alike, with no read-only-boot
special case, per triage's direction. On a file an earlier boot already
configured, a connect and a whole `os migrate duplicates` run now leave
the file's md5 unchanged. A fresh database still ends up
`auto_vacuum=2`.
QA source: the 17.7 pre-release run #21721
(`cli.migrate-duplicates-inventory`, outside the item's clauses).
## What changed
- `packages/drivers/driver-sql/src/sql-driver.ts`:
`applyAutoVacuumIncremental` reads first and sets only on a difference.
`autoVacuumOf` parses the reply in the shape of the existing
`journalModeOf`. A reply it cannot read counts as "not INCREMENTAL", so
an unreadable answer costs the old write but never the mode. The
docblock now says what the method does, replacing the false "no-op"
sentence. The `connect()` comment names the one rule, which both pragmas
follow.
-
`packages/drivers/driver-sql/src/sql-driver-21734-auto-vacuum-read-first.test.ts`
(new) holds pins 1 and 3, plus a control and a legacy-file pin. Details
below.
-
`packages/drivers/driver-sql/src/sql-driver-sqlite-journal-mode.test.ts`:
the native-load-failure case pinned the first statement as the setter.
It is now the read, `PRAGMA auto_vacuum`, which fails, so the setter is
never attempted.
- `packages/cli/src/commands/migrate/duplicates.integration.test.ts`
holds pin 2 in a new describe. The file's `readState` comment said a
plain read-write open rewrites the header counters. Measured, that is
false: a plain open is byte-neutral. That fixture moves because its
first connect converts it from a rollback journal to WAL. The comment
now says so.
- `.changeset/21734-driver-sql-auto-vacuum-read-first.md`: `patch` for
`@objectstack/driver-sql`, `Clause-②: no`.
File surface: the change lands where the claim expected it, in
`applyAutoVacuumIncremental`. The measurement found no other producer
(H3 below).
## Pins
1. **`connect()` on an `auto_vacuum=2` file leaves the md5 unchanged.**
The fixture is a driver connected to a fresh file, with 50 rows written
afterwards: `auto_vacuum=2`, `journal_mode=wal`, as asserted. The test
takes the md5, runs `connect()` and `disconnect()` on a new driver, and
compares the md5 again.
- Control, same fixture: an explicit setter followed by disconnect DOES
move the bytes, and only within 24 to 27 and 92 to 95, while
`auto_vacuum` stays 2. This proves the md5 instrument can see the stamp
that pin 1 forbids.
2. **`os migrate duplicates` on such a file leaves the md5 unchanged.**
The test runs the real command in-process:
`MigrateDuplicates.run(['--database-url', 'file:...'], { root: CLI_ROOT
})`, with cwd moved to the fixture project. That covers the oclif parse,
the read-only `bootSchemaStack` boot, the probes, the JSON payload and
the shutdown. I chose in-process over a spawn because one spawn of the
source entry pays a tsx compile of the whole CLI, for an answer that
lies entirely inside `run()`. The in-process `MigrateResume.run` calls
in `resume.recorded-by.integration.test.ts` are the precedent. The run
must really look, so the test also asserts the payload reports the
seeded cross-partition duplicate (`CASE-00001`).
3. **A fresh database still ends up `auto_vacuum=2`.** Covered for a
fresh file and for `:memory:`.
There is also a fourth pin, for unchanged behaviour (H2). A legacy NONE
file that already holds tables still receives the setter. It stays NONE,
and connect issues no `VACUUM`.
## Reverse verification
Run from the committed fix (`061d2fb7fd`), through
`scripts/ablation-replace.mjs` in WRAP mode, with a `trap` restore
armed.
- **Mutation:** the anchor `if (current !==
SqlDriver.SQLITE_AUTO_VACUUM_INCREMENTAL) {` became `... ||
'ablation-21734') {`. The setter was unconditional again.
- On disk: anchor x1 to x0, replacement x0 to x1, blob `0bcf7465c257` to
`b4cb6068b13b`.
- Then `pnpm --filter @objectstack/driver-sql build`, and
`ablation-dist-preflight` found the marker present in `dist/index.js`
and `dist/index.mjs`. The CLI pin consumes driver-sql's `dist/`, so the
rebuild was required.
- **Readings with the setter unconditional:**
- pin 1 RED: md5 `5b70462c…` against `736c98da…`;
- pin 2 RED: md5 `aa43f8da…` against `d51aba83…`;
- pin 3 GREEN (fresh file and `:memory:`);
- control GREEN, legacy pin GREEN, the existing journal-mode suite GREEN
(14), the existing #8928 integration case GREEN.
- **Restore:** `git checkout HEAD -- ABS_PATH`.
- The blob after restore equals HEAD (`0bcf7465c257`), `git diff HEAD`
is empty, and the whole-tree `git status --porcelain` is empty.
- After the rebuild, `ablation-dist-preflight --absent` reports the
marker absent from all 6 built files and the tree clean.
- Pins re-run green: driver-sql 19/19, CLI file 2/2.
## Mechanism hypotheses, measured
- **H1, byte-neutral read: holds.** Measured with better-sqlite3 on
copies of an `auto_vacuum=2`, `journal_mode=wal` file holding 200 rows.
- These leave the md5 unchanged: open/close, `PRAGMA auto_vacuum`,
`PRAGMA journal_mode`, `PRAGMA journal_mode = WAL`, and `SELECT count(*)
FROM sqlite_master`.
- `PRAGMA auto_vacuum = INCREMENTAL` alone changes offsets 27 and 95,
inside 24 to 27 and 92 to 95.
- The new read-then-set-if-different leaves the md5 unchanged.
- **H2, fresh and legacy files: behaviour unchanged.**
- A fresh file reads 0 and gets the setter. Neither the read nor the
later `journal_mode = WAL` stops the mode from taking: it ends `{av: 2,
wal}`.
- A non-empty legacy `auto_vacuum=0` file is left at 0 by the setter
alone, read on the same connection and on a fresh one; it moves only
with setter plus `VACUUM`. Today's `connect()` runs no `VACUUM`, before
and after this change. Only `os db clean` does. The legacy file still
receives the setter, which the legacy pin asserts.
- A FULL file still moves to INCREMENTAL (1 to 2).
- **H3, other connect-time writes: none on an already-configured file.**
`connect()` issues these statements:
- `ensureDatabaseExists`: a `mkdir`, with no SQL on SQLite;
- `PRAGMA auto_vacuum`, then the setter only if it differs;
- `PRAGMA journal_mode`, then `PRAGMA journal_mode = WAL` only if it
differs (already this shape before the change);
- `SELECT count(*) FROM sqlite_master` (the WAL read-through probe).
On a configured file each is byte-neutral or not issued. The whole `os
migrate duplicates` run is byte-identical there (pin 2). One edge sits
outside the card: a file no driver ever connected, still on a rollback
journal, gets converted to WAL by the first connect of any command. That
writes bytes 18, 19, 27 and 95. It is already the read-then-set shape,
so there is nothing to fix here. See Acceptance notes.
- **H4, dialects:**
- Postgres and MySQL: both pragma steps sit inside `connect()`'s `if
(this.isSqlite)` branch, so those paths are untouched.
- Turso remote: `connect()` returns before `super.connect()` and sets no
`auto_vacuum`, as its own docblock says.
- Turso local / replica: its knex client is spelled `better-sqlite3`, so
it inherits the rule through `super.connect()`.
- WASM SQLite (`SqliteWasmDriver`, `isSqlite` forced true): it inherits
the rule, and this measurably helps it. Its dialect marks the in-memory
image dirty on any PRAGMA assignment, and the bare read is not one.
Connect plus disconnect on an INCREMENTAL image file read `md5 CHANGED,
mtime CHANGED` with the unconditional setter (ablation leg) and `md5
UNCHANGED, mtime UNCHANGED` with this change.
## Docs re-read after the fix
- Driver docblock. Before: "Harmless / no-op on :memory: and on
already-incremental databases." That was untrue, so it is replaced by
the "Read first, set only when different" section, which lists the four
file shapes and what each one gets.
- `content/docs/deployment/cli.mdx` (`os migrate duplicates` row):
"Renumbers nothing and writes nothing at all". It is not edited: on a
database a serving boot configured it is now true, as pin 2 measures.
The never-connected rollback-journal edge is in Acceptance notes.
- `content/docs/data-modeling/drivers.mdx` ("The SQL driver connects
SQLite with `auto_vacuum=INCREMENTAL`") is still true and not edited.
## Verification (at `6ffd1485a8`, origin/main `316be321ef`; the merge of
origin/main was already up to date)
- `pnpm --filter @objectstack/driver-sql exec vitest run
--maxWorkers=2`: 218 files passed, 11 skipped; 3633 tests passed, 206
skipped.
- `pnpm --filter @objectstack/driver-sql typecheck`: exit 0.
`--listFiles` includes both edited test files.
- `pnpm --filter @objectstack/cli exec vitest run --project integration
--maxWorkers=2 src/commands/migrate/duplicates.integration.test.ts`:
2/2. The `unit` tier is untouched by this diff and is declared to CI.
- `pnpm --filter @objectstack/cli typecheck`, including
`check:test-typecheck`: OK (3 files, 28 errors, 6 pinned signatures,
unchanged).
- Downstream consumers of `connect()`:
- `driver-sqlite-wasm` 36 files, 675 tests;
- `driver-turso` 88 files, 2373 passed, 33 skipped.
- Gates: `node scripts/pm/dispatch-gates.mjs --commands` (no paths)
derived 68 commands. I also ran the 53-family artifact-roster block
printed outside that total and the four symbol-anchor sweeps
(`check:adr-symbol-anchors`, `check:scripts-symbol-anchors`,
`check:spec-docblock-symbol-anchors`, `check:adr-anchors`). That is 124
commands, all exit 0, with these exceptions:
- `check:dual-build-cjs-loads`, `check:i18n`, `check:i18n-coverage` and
`check:i18n-walk-parity` first answered exit 3 (PREREQUISITE NOT MET).
After `turbo run build`, all four exit 0. The dist-reading gates
`check:dts-closure`, `check:lean-entry-closure` and
`check:published-files` were re-run after that build: exit 0.
- The three PR-context scripts (`check-closing-target-claim.mjs`,
`check-partof-closing-keyword.mjs`, `check-single-claim-paths.mjs`)
answer exit 2 NOT WIRED without a pull request. Their `--self-test`
forms exit 0. `check-partof-closing-keyword.mjs` was also run with
`PR_BODY` set to this body.
- Reconciliation: `dispatch-gates --ran` reports "68 derived famil(ies)
accounted for — 68 run, 0 NOT-MEASURED (a DERIVED zero — all 68 recorded
an exit code and none of them is 3)".
- Lint, as a proven narrowing rather than a repo-wide run:
- Scope: the scan population is `eslint.config.mjs`'s `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']`, which covers all four edited
`.ts` files.
- Count: `eslint --no-inline-config --format json` on them linted 4
files, with 0 errors and 0 warnings.
- Why the narrowing is safe: the config never enables type-aware linting
(no `parserOptions.project`, as `eslint.config.mjs` states), so this
diff cannot move a verdict on an untouched file. Full `pnpm lint` is
CI's.
## Acceptance notes
- **A never-connected rollback-journal file is converted to WAL by its
first connect, a read-only `os migrate duplicates` included.**
- Measured through the command's real `run()`, in-process, on a fixture
written without connecting (`auto_vacuum=0`, `journal_mode=delete`). The
run moved offsets 18, 19, 27 and 95, and the file ended
`journal_mode=wal`. On the configured fixture the same run moved
nothing.
- This is the journal-mode step's one-time configuration, already
read-then-set. It only reaches a SQLite file that no current-version
driver has ever connected to. It is noted, not changed here: triage
ruled "one rule for every connect", and the card's pins cover the
already-configured file. The `cli.mdx` sentence "writes nothing at all"
is not literally true on such a file. This is raised in the report for
the seat to decide.
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ErfyP2Rx7XWHJA27QjyUi)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
A connect to a SQLite file that is already `auto_vacuum=INCREMENTAL` no longer writes to it. `SqlDriver.connect()` now reads `PRAGMA auto_vacuum` first and runs `PRAGMA auto_vacuum = INCREMENTAL` only when the file answers something else.
6
+
7
+
Clause-②: no
8
+
9
+
-**What changed on disk.** On a file that already answered INCREMENTAL, the setter changed no mode, but it still stamped two header counters: the file change counter (bytes 24–27) and the version-valid-for number (bytes 92–95). So a read-only command such as `os migrate duplicates`, which the CLI docs say writes nothing at all, changed the file's md5 on every run. Reading the pragma changes no bytes, so such a file now comes through a connect, and a whole `os migrate duplicates` run, byte-identical.
10
+
-**Unchanged.** A fresh file and `:memory:` still come out INCREMENTAL. A legacy NONE file that already holds tables still gets the setter, which leaves it NONE until a `VACUUM` (`os db clean`), exactly as before. The journal-mode step already read first and set only on a difference, and it is untouched. Postgres and MySQL issue no PRAGMA.
11
+
-**The WASM SQLite driver** inherits the rule. A connect and disconnect no longer rewrites an already-INCREMENTAL image file, because the setter was what marked the image dirty.
12
+
- ⛔ No config key, export, error code or accepted input changes.
0 commit comments