diff --git a/.changeset/schema-drift-single-value-json-column.md b/.changeset/schema-drift-single-value-json-column.md new file mode 100644 index 0000000000..4095cd124f --- /dev/null +++ b/.changeset/schema-drift-single-value-json-column.md @@ -0,0 +1,13 @@ +--- +"@objectstack/driver-sql": minor +--- + +Schema drift now reports a SINGLE-VALUE JSON-class column that a stale `varchar`/`text` column is holding — the population the detector could never see. + +The driver decides a field's column type with `JSON_COLUMN_TYPES.has(type) || !!field.multiple`: `createColumn` gives a json column to every JSON-class TYPE, and `isJsonField` — the read-side deserializer — asks the same question. The drift detector asked only `field.multiple === true`. So a single-value `file` / `image` / `location` / `address` / `record` / `vector` / `json` field (and the option families) sitting on a `varchar` or `text` column was written as JSON by the writer and did not exist to the differ. Because the additive sync never migrates a column's type, that column stayed wrong permanently and nothing reported it. Measured on the previous tree, one call per type: all fifteen JSON-class types the spec declares returned zero findings over a `character varying(2048)` column on `postgres` and `mysql`, while the same column under a `multiple: true` field returned one in the same run. + +The detector now reads the writer's own predicate, so the two halves can no longer disagree about which declarations get a json column. `SQLite is unchanged and still reports nothing`: its read path parses a textual column regardless of what the column calls itself, re-measured on an in-memory cell as a byte-identical round-trip between the stale column and the driver's own. + +**The remedy is offered to the array-valued half only.** `os migrate multi-value-columns` repairs a stale column by wrapping each stored value in a one-element JSON array, which is the right repair for a field whose value is a list and the wrong one for a field whose value is a scalar or an object. Findings for array-valued fields (`multiple: true`, and the inherently-multi option types) keep their message character for character, so that command keeps recovering the dialect from it and keeps working exactly as before. Findings for single-value JSON-class fields carry a message of their own that names neither the command nor its statement, explains why the automated route is withheld, and describes the by-hand conversion; the command refuses such an entry (`remedy_not_recognized`) instead of running array SQL over scalar rows. + +Also fixed by the same predicate: a single-value JSON-class field declaring a `maxLength` over a wider `varchar` column used to be reported as `narrow_varchar` at category `destructive` — inviting `os migrate apply --allow-destructive` to rewrite the column to a narrower varchar, the opposite of the repair it needs. It is now reported once, as the base-type divergence. diff --git a/packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts b/packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts index ce53c80f1f..b7229d5454 100644 --- a/packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts +++ b/packages/cli/src/commands/migrate/multi-value-columns.dialect-probe.test.ts @@ -170,6 +170,59 @@ describe('planning refuses anything it cannot read a dialect from (#11733)', () expect(planStaleColumnTargets(others, SQL)).toEqual({ targets: [], refusals: [] }); }); + it('a SINGLE-VALUE JSON-class column is refused, never planned — this command wraps values in an array (#15771)', () => { + // ⚠️ The cross-package half of #15771, pinned where the damage would land. + // + // The engine's base-type detector used to be keyed to `field.multiple` + // alone, so a single-value JSON-class field (`file`, `location`, `record`, + // …) on a stale `varchar` column produced no finding at all. Widening it + // put a NEW population in front of this command, and this command's repair + // is `json_build_array(col)` / `JSON_ARRAY(col)` — it WRAPS each stored + // value in a one-element array. Measured with this planner before the + // engine's message split: a single-value finding carrying the multi-value + // message was accepted as a target and planned with `json_build_array`, + // i.e. this command would have converted a scalar `"file_01HXYZ"` into + // `["\"file_01HXYZ\""]` on a customer's table. + // + // The engine therefore emits that population with a message carrying + // NEITHER the command's name NOR the statement, which lands it in the + // refusal branch above. That coupling is invisible from either side alone, + // so it is asserted from BOTH: the engine's side pins that the message + // omits the statement, and this pins what this command then does with it. + const single = diffManagedTable({ + table: TABLE, + fields: { [COLUMN]: { type: 'file' } as any }, + columns: STALE.postgres, + dialect: 'postgres', + }); + // Non-vacuity: the engine really does report this shape now. If it stops, + // the refusal below would pass over an empty array. + expect(single.map((d) => d.op.type)).toEqual(['manual_column_type_change']); + + const plan = planStaleColumnTargets(single, SQL); + expect(plan.targets).toEqual([]); + expect(plan.refusals).toHaveLength(1); + expect(plan.refusals[0]).toMatchObject({ table: TABLE, column: COLUMN, reason: 'remedy_not_recognized' }); + expect(plan.refusals[0].detail).toContain('os migrate plan'); + + // ⭐ And the CONTRAST in the same run, which is what makes the refusal a + // discrimination rather than a command that refuses everything: an + // inherently-multi option type on the identical column holds an ARRAY, so + // it keeps the remedy and is planned. + const arrayValued = diffManagedTable({ + table: TABLE, + fields: { [COLUMN]: { type: 'tags' } as any }, + columns: STALE.postgres, + dialect: 'postgres', + }); + const arrayPlan = planStaleColumnTargets(arrayValued, SQL); + expect(arrayPlan.refusals).toEqual([]); + expect(arrayPlan.targets).toHaveLength(1); + expect(arrayPlan.targets[0].statements).toEqual( + splitRemedyStatements(manualJsonConversionSql('postgres', TABLE, COLUMN)), + ); + }); + it('--table narrows to the tables named, and drops the rest silently', () => { const a = engineFinding('postgres'); const b = { ...a, table: 'crm_case', op: { ...(a.op as any), table: 'crm_case' } } as ManagedDriftEntry; diff --git a/packages/drivers/driver-sql/src/schema-drift.base-type-mismatch.test.ts b/packages/drivers/driver-sql/src/schema-drift.base-type-mismatch.test.ts index cda1b21639..0a80480b5d 100644 --- a/packages/drivers/driver-sql/src/schema-drift.base-type-mismatch.test.ts +++ b/packages/drivers/driver-sql/src/schema-drift.base-type-mismatch.test.ts @@ -51,6 +51,7 @@ import { SqlDriver } from './sql-driver.js'; import { diffManagedTable, manualJsonConversionSql, + JSON_COLUMN_FIELD_TYPES, MULTI_VALUE_COLUMN_REMEDY_COMMAND, type PhysicalColumn, type SqlDialectName, @@ -297,6 +298,172 @@ describe('diffManagedTable — multi-value field over a stale textual column (#1 }); }); +// ── #15771: the SINGLE-VALUE half of the same fork ───────────────────────── +// +// The writer asks `JSON_COLUMN_TYPES.has(type) || !!field.multiple`; the +// detector above asked only `multiple`. So a single-value JSON-class field on a +// `varchar`/`text` column was written as JSON and reported by NOTHING — and, as +// with #11535, the defect's shape is SILENCE, so "a finding was produced" is +// again the weak assertion. Every case below pins the finding on the right +// column with the right diagnosis, and the shapes that must stay silent are +// pinned as silent in the same breath. + +describe('diffManagedTable — a SINGLE-VALUE JSON-class field over a stale textual column (#15771)', () => { + /** The card's two, named rather than swept, because these are the ones a deployment meets. */ + const SINGLE_VALUE_JSON = ['file', 'location'] as const; + + it('reports single-value `file` and `location` on a varchar column, on BOTH enforcing dialects', () => { + // Measured on the pre-fix tree: every one of these four returned `[]`. + for (const type of SINGLE_VALUE_JSON) { + for (const dialect of ['postgres', 'mysql'] as const) { + const out = diffTags({ type }, staleColumn('character varying', 2048), dialect); + expect(out, `${type}/${dialect}`).toHaveLength(1); + expect(out[0]).toMatchObject({ + kind: 'type_mismatch', + table: 'proj_task', + column: 'tags', + expected: 'json', + actual: 'character varying', + severity: 'error', + // ⛔ NOT `destructive`, for the reason the multi-value case's category + // pin argues in full: the boot gate reads CATEGORY, every database + // this describes is already serving, and reporting the corruption + // must not be the thing that takes the app down. + category: 'needs_confirm', + op: { type: 'manual_column_type_change', table: 'proj_task', column: 'tags', to: 'json', from: 'character varying' }, + }); + } + } + }); + + it('SQLITE stays silent — the reverse control that proves the pin discriminates', () => { + // ⛔ Do not drop this leg. Without it every assertion above is equally + // satisfied by a detector that reports unconditionally. And the silence is + // MEASURED rather than scoped away: on an in-memory cell a single-value + // `file` id round-trips through the stale `varchar(2048)` column and + // through the driver's own `text` column with byte-identical stored bytes + // (`"file_01HXYZ"` in both), because SQLite's read path `JSON.parse`s a + // textual column regardless of what it calls itself. + for (const type of SINGLE_VALUE_JSON) { + expect(diffTags({ type }, staleColumn('varchar', 2048), 'sqlite'), type).toEqual([]); + } + }); + + it('says nothing once the column IS json — the repaired database', () => { + for (const type of SINGLE_VALUE_JSON) { + expect(diffTags({ type }, staleColumn('json'), 'postgres'), type).toEqual([]); + expect(diffTags({ type }, staleColumn('json'), 'mysql'), type).toEqual([]); + } + }); + + it('closes the blind spot for EVERY JSON-class type the spec declares, not just the file family', () => { + // The card's scope, asserted rather than described: the fork applies to + // every single-value member of the writer's set, whichever way the + // `VARCHAR(2048)`-vs-json generator divergence (#15041) is ruled. + const jsonClassTypes = [...JSON_COLUMN_FIELD_TYPES].filter((t) => t !== 'object' && t !== 'array'); + expect(jsonClassTypes.length).toBeGreaterThan(10); + + const blind = jsonClassTypes.filter( + (type) => diffTags({ type }, staleColumn('character varying', 2048), 'postgres').length === 0, + ); + expect(blind).toEqual([]); + + // Non-vacuity: a type OUTSIDE the set is still silent in the same run, so + // the zero above is a discriminating measurement, not an always-report. + expect(diffTags({ type: 'string' }, staleColumn('character varying', 2048), 'postgres')).toEqual([]); + expect(diffTags({ type: 'integer' }, staleColumn('character varying', 2048), 'postgres')).toEqual([]); + }); + + // ── the remedy split: the message a broken remedy must NOT carry ────────── + // + // ⚠️ MEASURED before this branch shipped, with the command's own planner: + // `planStaleColumnTargets` accepted a single-value finding carrying the + // MULTI-VALUE message as a target and planned + // `... ELSE json_build_array("doc") END`. That wraps the stored value in a + // ONE-ELEMENT ARRAY — right for a field declaring `multiple: true`, wrong for + // a field whose value is a scalar or an object. So the finding is emitted for + // both populations and the remedy is offered to ONE. + + it('the single-value message names NEITHER the command NOR its statement, so the command refuses it', () => { + for (const type of SINGLE_VALUE_JSON) { + for (const dialect of ['postgres', 'mysql'] as const) { + const [entry] = diffTags({ type }, staleColumn('character varying', 2048), dialect); + expect(entry.message, `${type}/${dialect}`).not.toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND); + // This is the CLI's dialect probe, reproduced: no dialect's statement + // is present, so `planStaleColumnTargets` recovers no dialect and + // refuses the entry (`remedy_not_recognized`) instead of running array + // SQL over scalar rows. Pinned from the consumer side too, in + // `packages/cli/.../multi-value-columns.dialect-probe.test.ts`. + for (const d of ['postgres', 'mysql'] as const) { + expect(entry.message).not.toContain(manualJsonConversionSql(d, 'proj_task', 'tags')); + } + } + } + }); + + it('the single-value message still carries the whole diagnosis an operator acts on', () => { + // A finding with no remedy is still a finding: the operator has to be able + // to act from the one line a restart prints. + const [entry] = diffTags({ type: 'file' }, staleColumn('character varying', 2048), 'postgres'); + expect(entry.message).toContain('proj_task.tags'); + expect(entry.message).toContain('`file`'); + expect(entry.message).toContain('character varying'); + expect(entry.message).toContain('json'); + // ⛔ NOT the issue id — `check:doc-authoring` refuses a tracker id in + // runtime prose (maintainer ruling 2026-08-12: an operator can resolve + // neither the number nor the tracker). The anchor lives in the `//` + // comment beside the emission and in git history. What the operator DOES + // need is the cause, in words: + expect(entry.message).toMatch(/JSON-ENCODED/); + expect(entry.message).toMatch(/backup/i); + expect(entry.message).toMatch(/btree/i); + // And it says WHY the automated route is withheld, rather than leaving the + // operator to discover the refusal by running it. + expect(entry.message).toMatch(/ONE-ELEMENT JSON ARRAY/); + }); + + it('an INHERENTLY-MULTI option type keeps the array remedy — the split is by value shape, not by `multiple`', () => { + // `multiselect` / `checkboxes` / `tags` hold a list with or without the + // flag, so the wrapping remedy is the right repair for them and the + // message that names it is the right message. Getting this leg wrong in + // either direction is a real cost: withheld, an operator loses a working + // command; offered to a scalar field, it corrupts. + for (const type of ['multiselect', 'checkboxes', 'tags'] as const) { + const [entry] = diffTags({ type }, staleColumn('character varying', 255), 'postgres'); + expect(entry.message, type).toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND); + expect(entry.message, type).toContain(manualJsonConversionSql('postgres', 'proj_task', 'tags')); + } + }); + + it('leaves the MULTI-VALUE message byte-identical — the CLI recovers the dialect from it', () => { + // ⚠️ The contract this card was most able to break. Widening the finding's + // population must not move one character of the message the array half + // emits, because `planStaleColumnTargets` reads the dialect by containment. + // (Proven against `origin/main` itself while the change was written: the + // whole entry — message included — compared equal for `lookup`, `string` + // and `file` with `multiple: true`, on both dialects.) + for (const dialect of ['postgres', 'mysql'] as const) { + const [entry] = diffTags({ type: 'lookup', multiple: true }, staleColumn('character varying', 255), dialect); + expect(entry.message).toContain(manualJsonConversionSql(dialect, 'proj_task', 'tags')); + expect(entry.message).toContain(MULTI_VALUE_COLUMN_REMEDY_COMMAND); + expect(entry.message).toContain('metadata declares a multi-value field (stored as `json`)'); + } + }); + + it('a single-value JSON-class field with a maxLength reports the base type ONCE, never `narrow_varchar`', () => { + // The same trap the multi-value case fell into, one population to the left: + // before this change `{ type: 'file', maxLength: 50 }` over a + // `varchar(255)` column reported `narrow_varchar` at category + // **destructive** — inviting `os migrate apply --allow-destructive` to + // rewrite the column to `varchar(50)`, the exact opposite of the repair it + // needs. `createColumn` never sizes a JSON-class column from `maxLength`. + for (const dialect of ['postgres', 'mysql'] as const) { + const out = diffTags({ type: 'file', maxLength: 50 }, staleColumn('character varying', 255), dialect); + expect(out.map((d) => d.op.type)).toEqual(['manual_column_type_change']); + } + }); +}); + // ── Half 2: end to end, on every provisioned dialect ──────────────────────── const TABLE = 'os11535_task'; diff --git a/packages/drivers/driver-sql/src/schema-drift.json-column-parity.test.ts b/packages/drivers/driver-sql/src/schema-drift.json-column-parity.test.ts new file mode 100644 index 0000000000..93dfa55ca7 --- /dev/null +++ b/packages/drivers/driver-sql/src/schema-drift.json-column-parity.test.ts @@ -0,0 +1,121 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15771] The DETECTOR's JSON-class predicate is the WRITER's, and stays it. + * + * The defect was a fork, not a missing rule. `createColumn` gives a field a + * json column when `JSON_COLUMN_TYPES.has(type)` — the type ALONE — and + * `isJsonField`, the read-side deserializer, is `JSON_COLUMN_TYPES.has(type) || + * !!field.multiple`. `diffManagedTable`'s base-type branch asked only + * `field.multiple === true`. So a SINGLE-VALUE JSON-class field (`file`, + * `location`, `record`, `vector`, the option families) on a `varchar`/`text` + * column was written as JSON by the writer and did not exist to the differ — + * permanently, because the additive sync never revisits a column, and silently, + * because nothing else reports it. + * + * Measured on the pre-fix tree, one `diffManagedTable` call per type: all + * FIFTEEN JSON-class types the spec declares returned ZERO entries over a + * `character varying(2048)` column on `postgres`, while the same column under a + * `{ multiple: true }` field returned one in the same run. + * + * ## Why this file exists rather than a list in the source + * + * `sql-driver.ts` imports `schema-drift.ts`, so the differ cannot import the + * writer's `JSON_COLUMN_TYPES` — the same cycle `UNBOUNDED_TEXT_FIELD_TYPES` + * documents. {@link JSON_COLUMN_FIELD_TYPES} is therefore a second constant, + * seeded from the SAME `@objectstack/spec` sets, and a second constant is only + * as good as the pin that holds it equal. Both directions are load-bearing: + * + * - `⊇` — a value-shape class added to the spec that reaches the writer but + * not the differ re-opens exactly this blind spot, by the door it came in. + * - `⊆` — a type listed here that the writer does NOT give a json column + * would be reported as needing a conversion to a column shape the platform + * would never create: a finding an operator can act on and be left with + * drift. + * + * The classification PROBES the driver rather than restating its cases (the + * technique `schema-drift.unbounded-text-column.test.ts` and + * `sql-driver-12017-bounded-string-spec-parity.test.ts` use), and the last case + * probes the DIFFER's observable verdict rather than the constant, so the two + * halves are compared where they actually meet. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { FieldType } from '@objectstack/spec/data'; +import { SqlDriver } from './sql-driver.js'; +import { diffManagedTable, JSON_COLUMN_FIELD_TYPES, type PhysicalColumn } from './schema-drift.js'; +import { dialectCell } from './live-dialect-matrix.testkit.js'; + +/** `SqlDriver.isJsonField` is protected — the writer's own predicate, exposed unchanged. */ +class WriterProbe extends SqlDriver { + asksJson(type: string, field: Record = {}): boolean { + return this.isJsonField(type, field); + } +} + +const STALE: PhysicalColumn[] = [{ name: 'doc', type: 'character varying', nullable: true, maxLength: 2048 }]; + +/** Does the DIFFER report the base-type divergence for this declaration? */ +const differReports = (field: Record): boolean => + diffManagedTable({ table: 'proj_task', fields: { doc: field } as never, columns: STALE, dialect: 'postgres' }) + .some((d) => d.op.type === 'manual_column_type_change'); + +describe('the JSON-class predicate the differ reads is the one the writer reads (#15771)', () => { + let driver: WriterProbe; + afterEach(async () => { + await driver?.disconnect().catch(() => {}); + }); + + it('holds the set equal to `isJsonField` over every FieldType the spec declares', () => { + driver = new WriterProbe(dialectCell('sqlite').config()); + const types = FieldType.options as readonly string[]; + expect(types.length).toBeGreaterThan(40); // the spec registry really was read + + const writerSaysJson = types.filter((t) => driver.asksJson(t)).sort(); + const declared = [...JSON_COLUMN_FIELD_TYPES].filter((t) => types.includes(t)).sort(); + + // Non-vacuity: the writer answered NO for a large part of the vocabulary, + // so an equality between two everything-sets cannot pass for a measurement. + expect(types.filter((t) => !driver.asksJson(t)).length).toBeGreaterThan(10); + expect(writerSaysJson).toEqual(declared); + expect(writerSaysJson.length).toBeGreaterThan(10); + }); + + it('the only NON-FieldType members are the two driver-internal aliases, and the writer owns them too', () => { + driver = new WriterProbe(dialectCell('sqlite').config()); + const types = FieldType.options as readonly string[]; + + // `object` / `array` name introspected external columns, not authorable + // types — they are the one hand-written part of the set, so they are the + // one part that can silently gain a third member. + const aliases = [...JSON_COLUMN_FIELD_TYPES].filter((t) => !types.includes(t)).sort(); + expect(aliases).toEqual(['array', 'object']); + for (const alias of aliases) expect(driver.asksJson(alias), alias).toBe(true); + }); + + it('the DIFFER agrees with the writer over the whole vocabulary, `multiple` and not', () => { + // ⭐ The pin that matters: not "two constants match" but "the two halves + // reach the same verdict about the same declaration". Probed through + // `diffManagedTable`'s output, so it fails if the branch stops consulting + // the set as much as if the set drifts. + driver = new WriterProbe(dialectCell('sqlite').config()); + const types = FieldType.options as readonly string[]; + + const disagreements: string[] = []; + for (const type of types) { + for (const multiple of [false, true]) { + const field = multiple ? { type, multiple: true } : { type }; + const writer = driver.asksJson(type, field); + if (writer !== differReports(field)) disagreements.push(`${type}${multiple ? ' multiple' : ''}`); + } + } + expect(disagreements).toEqual([]); + + // Non-vacuity in both directions, in the same run: the loop above saw real + // trues and real falses rather than passing over a uniform answer. + expect(differReports({ type: 'file' })).toBe(true); + expect(differReports({ type: 'string', multiple: true })).toBe(true); + expect(differReports({ type: 'string' })).toBe(false); + expect(differReports({ type: 'integer' })).toBe(false); + }); +}); diff --git a/packages/drivers/driver-sql/src/schema-drift.ts b/packages/drivers/driver-sql/src/schema-drift.ts index 428a1b64c7..aa617f4032 100644 --- a/packages/drivers/driver-sql/src/schema-drift.ts +++ b/packages/drivers/driver-sql/src/schema-drift.ts @@ -30,7 +30,13 @@ import { createHash } from 'node:crypto'; -import { isAppResolvedDefaultToken, isUniqueDeclared } from '@objectstack/spec/data'; +import { + isAppResolvedDefaultToken, + isUniqueDeclared, + STRUCTURED_JSON_TYPES, + FILE_REFERENCE_TYPES, + MULTI_OPTION_TYPES, +} from '@objectstack/spec/data'; import type { SchemaDiffEntry } from '@objectstack/spec/shared'; // ─────────────────────────────────────────────────────────────────────── @@ -641,8 +647,53 @@ export const UNBOUNDED_TEXT_FIELD_TYPES: ReadonlySet = new Set([ ]); /** - * Does a multi-value field's JSON column carry its type on THIS dialect — i.e. - * does a stale textual column silently corrupt the value (#11535)? + * The field types the EMITTER materialises as a JSON column — the writer's own + * predicate, restated on this side of the cycle (#15771). + * + * `createColumn`'s catch-all is `JSON_COLUMN_TYPES.has(type) ? jsonColumn(...) + * : table.string(...)` and `isJsonField` — the read-side deserializer — is + * `JSON_COLUMN_TYPES.has(type) || !!field.multiple`. So the writer has always + * asked about the TYPE as well as `multiple`, while the base-type branch below + * asked only `field.multiple === true`. A single-value JSON-class field on a + * `varchar`/`text` column was therefore written as JSON and did not exist to + * the differ, permanently and silently — the additive sync never revisits a + * column. Two halves disagreeing about which declarations get a json column is + * the defect class this module exists to close; this constant is the deletion + * of that fork, not a new criterion. + * + * ⚠️ Seeded from `@objectstack/spec` — the SAME three sets `JSON_COLUMN_TYPES` + * is seeded from — rather than hand-listed, so a value-shape class added to the + * spec becomes a JSON column and a DETECTABLE one in one edit. Only the two + * driver-internal aliases are hand-written, because they are not authorable + * `FieldType`s at all: `object` / `array` name introspected external columns. + * + * ⚠️ A second constant rather than an import, for the reason + * {@link UNBOUNDED_TEXT_FIELD_TYPES} gives in full — `sql-driver.ts` imports + * THIS module, so the import would be a cycle. It is therefore PINNED rather + * than trusted: `schema-drift.json-column-parity.test.ts` asserts set equality + * against the driver's own `isJsonField`, in both directions, over every + * `FieldType` the spec declares. + * + * ⛔ Module-exported so this package's own suites can pin it, and deliberately + * NOT added to `index.ts` — the same call {@link UNBOUNDED_TEXT_FIELD_TYPES} + * and {@link MULTI_VALUE_COLUMN_REMEDY_COMMAND} make. + */ +export const JSON_COLUMN_FIELD_TYPES: ReadonlySet = new Set([ + ...STRUCTURED_JSON_TYPES, ...FILE_REFERENCE_TYPES, ...MULTI_OPTION_TYPES, + 'object', 'array', +]); + +/** + * Does a JSON-class field's column carry its type on THIS dialect — i.e. does a + * stale textual column silently corrupt the value (#11535 / #15771)? + * + * The question is about the DIALECT, so widening the population it is asked + * about (from `multiple` alone to the emitter's own JSON-class predicate, + * #15771) changes nothing here: what makes SQLite exempt is that its read path + * `JSON.parse`s a textual column regardless, which is as true of a single-value + * `file` id as of an array — re-measured on an in-memory cell, the stale + * `varchar(2048)` column and the driver's own `text` column store BYTE-IDENTICAL + * bytes and read back identically. * * Postgres and MySQL: yes. Measured end to end on live Postgres 16.13 and MySQL * 8.0.46 — a field that gained `multiple: true` over a pre-existing @@ -661,7 +712,7 @@ export const UNBOUNDED_TEXT_FIELD_TYPES: ReadonlySet = new Set([ * not an enforced type, which is the same reason * {@link enforcesVarcharLength} excludes it. */ -function multiValueColumnTypeIsLoadBearing(dialect: SqlDialectName): boolean { +function jsonColumnTypeIsLoadBearing(dialect: SqlDialectName): boolean { return dialect === 'postgres' || dialect === 'mysql'; } @@ -941,8 +992,49 @@ export function diffManagedTable(args: { // `remedy_not_recognized`, i.e. the remedy this text points at stops // working. Pinned from this side by the `toContain(manualJsonConversionSql(…))` // cases in `schema-drift.base-type-mismatch.test.ts`. - const declaresJsonColumn = field.multiple === true; - if (declaresJsonColumn && multiValueColumnTypeIsLoadBearing(dialect) && acceptsStringifiedJson(col.type)) { + // + // ── #15771: the predicate is the WRITER'S, not `multiple` alone ───────── + // + // See {@link JSON_COLUMN_FIELD_TYPES}: the emitter gives a json column to + // every JSON-class TYPE, `multiple` or not, so a single-value `file` / + // `location` / `record` / `vector` field on a `varchar` column was written + // as JSON and reported by nothing. Measured on the pre-fix tree, one + // `diffManagedTable` call per type: all fifteen JSON-class types the spec + // declares returned ZERO entries over a `character varying(2048)` column on + // `postgres` and `mysql`, while the same column under `{ multiple: true }` + // returned one — the differ was working and this whole population was + // invisible to it. + // + // ## Why the message SPLITS, and why the array half is byte-identical + // + // The remedy named below repairs a stale column by WRAPPING each stored + // value in a one-element JSON array — `json_build_array(col)` / + // `JSON_ARRAY(col)`, the ELSE arm of {@link manualJsonConversionSql}. That + // is the right repair for a field whose value IS an array and the wrong one + // for a field whose value is a scalar or an object: the write path + // `JSON.stringify`s a single-value `file` id into the column, so it holds + // the JSON scalar `"file_01HXYZ"` — measured byte-for-byte on an in-memory + // SQLite cell — and wrapping that yields `["\"file_01HXYZ\""]`, a shape + // the field never declared. Measured with the command's OWN planner: a + // single-value finding carrying this message is accepted as a target and + // planned with `json_build_array`. + // + // So both populations are REPORTED and only one is offered the remedy. The + // array half keeps its message CHARACTER FOR CHARACTER — the contract note + // above is why — and the single-value half carries neither the command nor + // the statement, so the same probe recovers no dialect and the command + // REFUSES the entry (`remedy_not_recognized`) rather than run array SQL over + // scalar rows. That refusal is the command's designed branch for a message + // it cannot read, and it is pinned from both sides. + const declaredType = field.type || 'string'; + const declaresJsonColumn = JSON_COLUMN_FIELD_TYPES.has(declaredType) || field.multiple === true; + // Is the declared VALUE an array? `multiple: true` on any type, plus the + // inherently-multi option types, whose value is a list with or without the + // flag (`MULTI_OPTION_TYPES` — the spec's own class). This, and never + // JSON-class membership (which both populations share), is what decides + // whether the wrapping remedy is the right repair. + const declaresArray = field.multiple === true || MULTI_OPTION_TYPES.has(declaredType); + if (declaresJsonColumn && jsonColumnTypeIsLoadBearing(dialect) && acceptsStringifiedJson(col.type)) { out.push({ kind: 'type_mismatch', remoteName: table, @@ -953,8 +1045,8 @@ export function diffManagedTable(args: { severity: 'error', category: 'needs_confirm', op: { type: 'manual_column_type_change', table, column: fieldName, to: 'json', from: col.type }, - message: - `${table}.${fieldName}: metadata declares a multi-value field (stored as \`json\`) but the ` + + message: declaresArray + ? `${table}.${fieldName}: metadata declares a multi-value field (stored as \`json\`) but the ` + `column is \`${col.type}\` — the database was created while the field was single-value and the ` + `additive sync never migrates a column's type. Arrays are being written as the STRINGIFIED ` + `literal (e.g. '["a","b"]') and read back as a string, so anything consuming the value ` + @@ -967,7 +1059,22 @@ export function diffManagedTable(args: { `a json column cannot carry a plain btree: ` + `${manualJsonConversionSql(dialect, table, fieldName)} ` + `Rows written while the column was stale may already hold a stringified array in a RELATED ` + - `single-value column; neither route repairs those.`, + `single-value column; neither route repairs those.` + : `${table}.${fieldName}: metadata declares \`${declaredType}\`, a field ObjectStack stores ` + + `in a \`json\` column whether or not it is multi-value, but the column is \`${col.type}\` ` + + `and the additive sync never migrates a column's type. The value is written JSON-ENCODED ` + + `into that textual column — a single-value \`file\` id lands as the quoted text ` + + `'"file_01HXYZ"' — while on this dialect the read path decodes a json COLUMN rather than ` + + `the text inside a textual one, so it comes back with its quotes and every consumer that ` + + `matches the raw stored form (file resolution, ownership claims) refuses it. ` + + `The automated column migration ObjectStack ships is NOT offered for this column and will ` + + `refuse it: it repairs a stale column by wrapping each value in a ONE-ELEMENT JSON ARRAY, ` + + `which is right for a field declaring \`multiple: true\` and wrong here — it would leave ` + + `an array where the declaration promises a single value. Convert the column by hand ` + + `instead, in a transaction and with a backup taken first: CAST the stored text to json ` + + `rather than wrapping it — rows written through ObjectStack already hold valid JSON — and ` + + `drop any index on the column first, since a json column cannot carry a plain btree. ` + + `"os migrate plan" lists this entry; "os migrate apply" reports it as skipped.`, }); }