Skip to content

Commit 09f9361

Browse files
huangyiireneclaude
andauthored
fix(driver-sql): report a multi-value field left on a stale varchar/text column (#11720)
* fix(driver-sql): report a multi-value field left on a stale varchar/text column A field that gains `multiple: true` materialises as a `json` column on a fresh database, but `initObjects` is additive-only: on an existing database nothing is missing, so the old varchar/text column is kept and every array written to it is stored as the stringified literal and read back as a string. Measured on live Postgres 16.13 and MySQL 8.0.46 on the pre-fix tree, `detectManagedDrift()` returned `[]` for exactly that shape. Detection only. The column is not migrated: an `ALTER TABLE ... TYPE json USING` over existing rows plus an index rebuild is a destructive migration over shipped data, and whether the platform should perform it is a separate open decision. The new `manual_column_type_change` op has no reconciler arm by design. Severity `error`, category `needs_confirm` — measured, not chosen for tone: the artifact-pinned boot gate refuses a boot for `category === 'destructive'` and nothing else, and every database this describes is already serving. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn * test(driver-sql): type the drift suite's find() queries instead of casting `{ filters: [] } as any` was not merely untyped — `filters` is not a DriverQuery key at all (`where` is), so the cast was hiding a wrong shape while adding two sites to the query-options-erasure test-surface ratchet. An empty query is the typed spelling of "all rows". Part of #11535 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VK8rFDtg8eREaxBGX99Csn --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 945ffbe commit 09f9361

3 files changed

Lines changed: 582 additions & 0 deletions

File tree

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
---
2+
'@objectstack/driver-sql': minor
3+
---
4+
5+
Report a multi-value field left on a stale `varchar`/`text` column, instead of
6+
letting it silently corrupt every array written to it
7+
8+
A field that gains `multiple: true` materialises as a `json` column on a fresh
9+
database, but `initObjects` is additive-only: on a database created while the
10+
field was single-value, nothing is missing, so nothing is added and the old
11+
`varchar`/`text` column is kept forever. The write path stringifies the array
12+
for a json field on every non-SQLite dialect; the read path relies on the
13+
driver's column-type-based decoding, which a stale textual column defeats. The
14+
array goes in as the literal `["id1","id2"]` and comes back as a **string**
15+
so a hook copying the value into a child record's single-lookup column writes
16+
that whole string as one id. User-filed production report, repaired by hand on a
17+
live database.
18+
19+
Until now the schema-drift detector said **nothing** about it. Measured on the
20+
pre-fix tree against live Postgres 16.13 and MySQL 8.0.46: after the metadata
21+
change and a reboot, `detectManagedDrift()` returned `[]` and the boot logged
22+
zero `[schema-drift]` lines, while the very next write stored
23+
`["user_A","user_B"]` into a `character varying(255)` column and read it back
24+
with `typeof === 'string'`. The action vocabulary had no "the base type is
25+
wrong" entry at all — only `relax`/`tighten_not_null`, `widen`/`narrow_varchar`,
26+
`drop_column`, `drop_column_default` and the index ops.
27+
28+
The divergence is now **detected and reported**, naming the table, the column,
29+
the declared type, the physical type and the exact statement an operator runs by
30+
hand — dialect-correct, and executed against both live servers by the suite
31+
rather than merely printed. ObjectStack does **not** change the column: an
32+
`ALTER TABLE … TYPE json USING …` over existing rows with an index drop and
33+
rebuild is a destructive migration over shipped data, and whether the platform
34+
should perform it is a separate, open decision. The new `manual_column_type_change`
35+
op deliberately has no reconciler arm; `applyMigrationEntries` reports it as
36+
skipped, which is the intended contract while that decision is open.
37+
38+
Reported at severity `error` and category **`needs_confirm`**, and the category
39+
is load-bearing rather than cosmetic. Every database this finding describes is
40+
already serving — that is the premise of the report — and the artifact-pinned
41+
boot gate refuses a boot for `category === 'destructive'` and nothing else
42+
(`severity` it never reads). Measured both ways: a `destructive` entry returns
43+
`ok=false` from that gate, this entry returns `ok=true`. Spelling it
44+
`destructive` would have turned every affected deployment into a crash-loop on
45+
its next restart — the report of the corruption becoming the outage.
46+
47+
SQLite is deliberately excluded, and the exclusion is a measurement rather than a
48+
scoping convenience: the same stale column reads back as a real `['x','y']`
49+
array there, because SQLite's read path `JSON.parse`s regardless of what the
50+
column calls itself. There is no corruption to report, and reporting it anyway
51+
would put a permanent `error` finding on every long-lived SQLite development
52+
database. A stale `integer`/`timestamp` column is excluded for the mirror-image
53+
reason — the server already refuses that write loudly, so there is no silence to
54+
break.
55+
56+
Also fixed, same defect class: a multi-value field that *also* declared
57+
`maxLength` used to produce `narrow_varchar` at severity `error`, category
58+
**destructive** on both enforcing dialects — a finding that refuses the
59+
artifact-pinned boot and invites `os migrate apply --allow-destructive` to
60+
rewrite the column to `varchar(50)`, the exact opposite of the repair it needs.
61+
`createColumn` returns at its `multiple` branch before `maxLength` is ever read,
62+
so the emitter never asks for that width; the differ no longer does either. The
63+
single-value width branch is untouched and pinned as untouched.

0 commit comments

Comments
 (0)