Repository navigation
Commit 9bfbacb
fix(driver-sql): the local json backfill lets the codec decide, so JSON nested past SQLite's depth limit is no longer quoted into a string (#19972)
Fixes #19912
Clause-②: no
## What was wrong
`SqlDriver.backfillCanonicalJsonEncoding` (the local SQLite `Field.json`
storage backfill from #12380) ran one statement per json column on every
`syncSchema` / `initObjects` over an existing table:
```sql
update ?? set ?? = json_quote(??) where typeof(??) = 'text' and json_valid(??) = 0
```
SQLite's `json_valid()` answers 0 for JSON nested more than 1000 levels
deep, while the driver reads such a cell with `JSON.parse` without
trouble. So a deep array the current write door stored correctly was
quoted into a JSON string on the next schema sync and read back as a
string from then on. No error, no log line: a correct cell silently
became a wrong one.
## The fix
SQL now only pre-filters. The driver's codec decides.
- **One rule, shared by both faces.** New
`packages/drivers/driver-sql/src/unencoded-json-text.ts` exports
`recoverUnencodedJsonText(stored)`: when `JSON.parse` accepts the text
it returns `null` and the cell is left as stored; when `JSON.parse`
rejects it, the cell is a legacy plain string and the function returns
`JSON.stringify(stored)`, which is what the write path stores. It is
exported from `@objectstack/driver-sql`, and the Turso remote residue
backfill (`remote-codec-residue-backfill.ts`, from PR #19904) now
imports it. That remote change is an import plus one line in place of
its inline copy, and the remote behaviour does not change.
- **The local pass** (`convergeJsonColumn`) reads the `typeof(col) =
'text' and json_valid(col) = 0` candidates 500 at a time in `rowid`
order, applies the rule to each, and rewrites only the cells the rule
rejects. Each rewrite is a compare-and-set, `where rowid = ? and
typeof(col) = 'text' and col = ?` bound to the text the decision read,
so a value written concurrently is never overwritten. One transaction
covers each page's writes. The pass walks every page, so a column still
converges in a single sync, and memory stays bounded to one page.
- **Idempotence holds.** A converted cell is outside the pre-filter. A
withheld cell is re-read on every sync but never written. A re-run over
a converged table changes zero rows, and that is pinned with
`total_changes()`.
- **Logging.** The failure posture is unchanged: `warn`, and the rows
stay legacy. The warn now also reports how many rows the committed pages
converted. Withheld cells get one `info` line per column.
## Measured before acting (the dispatch's H1 to H5)
- **H1, confirmed** at `2c1011b`: the method and the statement are
exactly as quoted above, guarded by `if (!this.isSqlite || !fields ||
fields.length === 0) return;` and `if (!tableExisted) return;`.
- **H2, confirmed. It is a build-time constant, and every bundled build
has 1000.** `json_valid` of an N-level array, measured on each bundled
engine:
| engine | SQLite | N=1000 | N=1001 | N=2000 | N=2001 |
|---|---|---|---|---|---|
| better-sqlite3 13.0.3 | 3.53.4 | 1 | 0 | 0 | 0 |
| sql.js 1.14.1 | 3.49.1 | 1 | 0 | 0 | 0 |
| `@libsql/client` 0.17.4 (local) | 3.45.1 | 1 | 0 | 0 | 0 |
`JSON.parse` (Node v22.22.2) reads 1001, 1999, 2000 and 2001 levels. The
bundled `sqlite3.c` defines `JSON_MAX_DEPTH 1000` unless
`SQLITE_JSON_MAX_DEPTH` is set, and notes the limit "was 2000 prior to
version 3.42.0". A Turso server build, or any custom or older build, can
therefore differ. The fix does not depend on the value.
- **H3, confirmed.** No override of the method exists outside
`sql-driver.ts`. `SqliteWasmDriver` forces `isSqlite` and inherits
`syncSchema` / `initObjects` unchanged. In local mode,
`TursoDriver.syncSchema` goes to `super.syncSchema`, then `initObjects`,
then `super.initObjects`; `initObjects` goes to `super`;
`syncSchemasBatch` calls `super.syncSchema` once per schema. All three
doors reach the backfill, and the Turso local face runs on
better-sqlite3 (`toKnexConfig`). The Turso doors are pinned in a test.
The wasm face was measured with a scratch probe but is not pinned (see
Acceptance notes): through both `syncSchema` and `initObjects`, the
1001-level array read as an array after two backfills, and `bare` was
converted.
- **H4: the decision rule is shared, the paging machinery is not.** The
remote pass runs over libsql `execute`/`batch` (`RemoteBackfillClient`),
with a probe and a per-invocation budget. The local pass runs over knex.
Sharing the walker would take more than an import swap on the remote
side. The pre-filter SQL reads the same on both faces, but it only
narrows the scan and never decides.
- **H5, confirmed. The two are identical by construction.** The SQLite
json arm of `formatOutput` is `JSON.parse(text)` inside a try/catch. The
rule asks the same question, and a test pins the link through the
driver: the backfill rewrites a cell exactly when the driver's read of
that cell answers the stored text itself.
**Preservation, byte for byte.** Stored and then read on better-sqlite3,
`json_quote(col)` equals `JSON.stringify` of the text read back, for
every non-surrogate BMP code point plus astral characters and an
embedded NUL: 63,499 samples, 0 mismatches. Lone surrogates never reach
disk; better-sqlite3 stores them as U+FFFD. The compare-and-set only
writes when the stored bytes equal what was read, so every cell written
is written with the bytes the retired statement produced.
## Tests
New
`packages/drivers/driver-sql/src/sql-driver-json-backfill-depth-limit.test.ts`
(7 tests):
1. The card's reproduction: `bare`, a 1001-level array and `[1,2]`. Only
`bare` is quoted, and the array still reads as an array after a
**second** backfill.
2. Idempotence: `total_changes()` does not move over two re-runs,
including with a withheld cell present.
3. Preservation: 11 legacy bare texts (empty, control characters, an
embedded NUL, U+2028/2029, a leading U+FEFF, emoji and CJK) are each
converted to the exact bytes `select json_quote(?)` gives, and each
still reads as itself.
4. The rule is the read codec. The cases are 1000-, 1001- and 2001-level
arrays, a 1001-level object, parseable and unparseable texts, `1e400`
and an escaped lone surrogate. A cell is rewritten if and only if its
pre-backfill read answers its own text, and no read moves.
5. Compare-and-set: a second connection writes the deep array into a
candidate row between the page read and the write, and the write leaves
it alone.
6. Paging: 1,100 rows, with deep cells at rows 0, 499, 500, 999 and
1099. All 1,095 legacy rows converge in one pass, and the log payloads
report `rowsConverted: 1095` and `rowsWithheld: 5`.
7. A unit test of `recoverUnencodedJsonText`.
New
`packages/drivers/driver-turso/src/turso-local-json-backfill-depth-limit.test.ts`
(3 tests) runs the dev's reproduction on `new TursoDriver({ url:
':memory:' })`, once through each local door: `syncSchema`,
`initObjects` and `syncSchemasBatch`.
Suites run at `74ff944`:
- driver-sql: 181 files passed, 11 skipped; 2677 tests passed, 170
skipped. The skips are the live PG and MySQL cells; CI runs them in
`Temporal Conformance`.
- driver-turso: 60 files, 1334 tests passed. This includes the unchanged
`turso-remote-codec-residue-backfill.test.ts`, run against the rebuilt
driver-sql `dist`.
- driver-sqlite-wasm: 29 files, 518 tests passed.
- `tsc --noEmit` exit 0 for driver-sql and driver-turso. `--listFiles`
shows each new test file inside its package's program.
## Reverse verification
Both ablations went through `scripts/ablation-replace.mjs` (anchor hit 1
time, blob changed, and restore proven as blob == HEAD with an empty
`git diff HEAD`) from the committed fix at `74ff944`.
- **Ablation 1: the retired statement put back** in place of
`convergeJsonColumn`. Afterwards driver-sql was rebuilt.
`ablation-dist-preflight` found the marker `json_quote(??)` in
`dist/index.js` and `dist/index.mjs`; before the mutation it was absent
from both.
- driver-sql file: **4 red, 3 green**. Red: the reproduction, the
read-codec rule, paging, and compare-and-set. Compare-and-set is red
because the old code never reads candidates, so the injection never
fired (`expected +0 to be 1`); ablation 2 isolates it. Green:
idempotence, preservation, and the unit test, which is the expected
direction because the old statement is also idempotent and
byte-identical on bare text.
- Turso file: **3 of 3 red**, all with `expected false to be true` on
`Array.isArray`.
- Restore leg: rebuilt, the preflight with `--absent` found the marker
in none of the 6 built files, and the tests went back to 7/7 and 3/3.
- **Ablation 2: the compare-and-set predicate removed.** The update
became `where rowid = ?`. **Exactly one test went red**,
compare-and-set: `raced` was overwritten with `"bare"`. After the
restore, the file is 7/7 green again.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` at `74ff944` derived 62 families: the
dispatch's 56, plus `check:engine-double-contract`,
`check:objectql-double-limit`, `check:query-options-erasure`,
`check:type-check-coverage`, `check:type-check-debt` and
`check:where-matcher`. Reconciled with `--ran` (exit codes recorded):
**60 exit 0 and 2 NOT MEASURED** (both exit 3, a missing prerequisite).
- `check:dual-build-cjs-loads` needs every workspace package's `dist`,
and 78 published entry points have no `dist` here. As a narrowed probe:
the CJS and ESM entries of driver-sql and driver-turso both load, and
`recoverUnencodedJsonText` resolves from both driver-sql builds.
- `check:type-check-debt` is missing the built type entries of 23
dependencies. The two packages this diff touches type-check clean
(above).
- `check:plugin-teardown-shape --self-test` first exited 3 on the
shallow clone (its pinned fixture commit was unreachable). After
fetching that one commit it passed, and `check:lean-entry-closure`
passed after the `@objectstack/objectql` closure was built.
- `check:driver-conformance` gave the same line before (`2c1011b`) and
after (`74ff944`): `OK — 50 covered cell(s), 0 in the DEBT ledger, 0
exempt`.
- `check:durability-log-level` exit 0: 36 durability-critical seams, all
loud.
- Narrowed lint, not the repo-wide `pnpm lint`: `eslint
--no-inline-config --format json` over the 6 changed TS files gave 6
results, 0 errors and 0 warnings, at `74ff944`. Every one of the 6 falls
under the `packages/**/*.{ts,tsx,mts,cts}` blocks of
`eslint.config.mjs`, and the `.changeset` file is outside every lint
glob. The config enables no type-aware linting (no
`parserOptions.project`, no typed rules, as its own comment records), so
this diff cannot move the verdict for an untouched file.
## Acceptance notes
- **Behaviour on the wasm face, measured only.** sql.js reads text
through a decoder that drops a leading U+FEFF, and its bind path
truncates at an embedded NUL. Where the read differs from the stored
bytes, the compare-and-set does not match, so the cell is not written
and keeps reading as it did. The retired statement rewrote such cells,
because `json_quote()` works on the stored bytes. For a BOM-prefixed
JSON text, that would change this face's read from an array to a string.
That last point is reasoned from the two measured reads; it was not run
against the old code. The wasm face also used to run a zero-row `UPDATE`
on every sync for every json column. Its dirty flag is set by statement
kind, so the file-backed image was marked dirty each time. It now runs
an `UPDATE` only when a candidate is rejected. A committed wasm pin
would be a new file in `driver-sqlite-wasm`, which is outside this
card's declared file surface, so none was added.
- **Withheld cells cost a read on every sync.** A deep cell stays a
pre-filter candidate forever, so it is read and parsed on each sync,
with one `info` line per column. These cells are rare by shape.
- **The json pre-filter has the same embedded-NUL exception as the date
one.** `json_valid()` stops at a NUL, so a text that is valid JSON up to
a NUL is never selected. That is the safe direction, and the local doc
now says so. The remote module's header names only the date exception
("with one exception in the safe direction (the date bullet)"). This is
a gap in the docs, not a defect.
- **A cursor stall guard.** If a rowid comes back rounded (past 2^53)
and the cursor stops advancing, the pass stops, leaving the rest of the
column legacy, which reads the same.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01Bvd69VPa6puiNzzPUroDBx)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent c839986 commit 9bfbacb
7 files changed
Lines changed: 597 additions & 38 deletions
File tree
- .changeset
- packages/drivers
- driver-sql/src
- driver-turso/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
17 | 21 | | |
18 | 22 | | |
19 | 23 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
0 commit comments