Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/migrate-multi-value-columns-command.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
---
"@objectstack/cli": minor
"@objectstack/driver-sql": minor
---

New operator-run command `os migrate multi-value-columns`: migrates a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — the `manual_column_type_change` drift `os migrate apply` reports and deliberately never reconciles for you (#11535, ruled C on #11700). Flags: `--apply` (default off), `--yes`/`-y`, `--force`, `--table <name>` (repeatable), `--database-url`, `--json`. **Dry-run contract: without `--apply` the command executes nothing at all** — it prints the exact statements and the database they would run against, opens no seam and issues no probe, and a run is verified to have left the column type and every row unchanged. `--apply` runs `@objectstack/driver-sql`'s own `manualJsonConversionSql` — newly re-exported from that package's index for this consumer, its only other change — i.e. the statement the drift finding itself prints (Postgres: one `ALTER … USING (CASE …)` with `json_build_array`; MySQL: the two row-shaping `UPDATE`s then `ALTER … MODIFY … json`), refuses to execute anything the finding does not contain verbatim, re-runs detection afterwards and exits non-zero if the finding has not cleared. SQLite is excluded — the stale column round-trips a real array there, so the finding is never raised. Rows corrupted before the column is migrated are out of scope, and the command is never invoked automatically: nothing on the boot path reaches it.
74 changes: 74 additions & 0 deletions content/docs/deployment/cli.mdx
Original file line numberDiff line numberDiff line change
Expand Up@@ -543,6 +543,7 @@ diverges from the live schema, and the physical column wins at write time.
|---------|-------------|
| `os migrate plan` | Dry-run: show how the database has drifted from metadata, categorised safe / needs-confirm / destructive (no changes applied) |
| `os migrate apply` | Reconcile the database to metadata. Applies loosening changes; destructive ones require `--allow-destructive` |
| `os migrate multi-value-columns` | Migrate a stale `varchar`/`text` column to `json` where the field declares `multiple: true` — the one drift op `apply` never reconciles for you. Dry run by default; `--apply` runs the statement the finding prints |

```bash
os migrate plan # Preview drift (no changes)
Expand DownExpand Up@@ -664,6 +665,79 @@ first. It never drops a table that is absent from your metadata, and on SQLite
it reconciles via a table rebuild (copy → swap) that preserves your data.
</Callout>

#### `os migrate multi-value-columns`

The one drift op `os migrate apply` will **never** apply for you.

A field that gains `multiple: true` over a database that already exists keeps
its old `varchar` / `text` column: the additive sync adds columns, and never
changes the type of one that is already there. The write path then stores an
array as the **stringified literal** `'["a","b"]'` and reads it back as a
string, so whatever consumes the value receives one opaque id instead of a
list — a hook copying it into a child record's single-value lookup writes the
whole string as one id. `os migrate plan` reports it as
`manual_column_type_change`, at severity `error` and category `needs-confirm`,
which is why it neither refuses your boot nor is ever reconciled automatically:
changing a column's type on a serving production database, unattended, is not
something the platform will do while you are not watching.

```bash
os migrate multi-value-columns # Dry run: the exact statements, executed NOT AT ALL
os migrate multi-value-columns --json # The same, machine-readable
os migrate multi-value-columns --apply # Run them (prompts)
os migrate multi-value-columns --apply --yes --json # CI / scripts
os migrate multi-value-columns --table crm_case # Restrict to one physical table (repeatable)
os migrate multi-value-columns --database-url postgres://…
```

**Take a backup first.** The dry run is the default and writes nothing at all —
not a probe, not a temporary table — so run it, read the statements it prints,
and only then re-run with `--apply`.

The statement is the one the drift finding itself prints, per dialect, and the
command refuses to run anything else: if the finding no longer contains a
statement the command recognises, it says so and tells you to apply the
finding's statement by hand rather than falling back to SQL of its own.

| Dialect | What runs |
|---------|-----------|
| PostgreSQL | One `ALTER TABLE … ALTER COLUMN … TYPE json USING (CASE …)`. Legacy single values become one-element arrays (`json_build_array`, **not** `to_json`, which would produce a JSON *scalar* that is still not an array); an already-stringified array is cast through; `NULL` and `''` both become `NULL` |
| MySQL | Three statements in order: `UPDATE … JSON_ARRAY(…)` over the legacy single values, `UPDATE … SET … = NULL` over the empty strings, then `ALTER TABLE … MODIFY … json`. MySQL will not cast text to json implicitly, so the rows have to move first or the `ALTER` dies on the first legacy value |
| SQLite | Nothing — and nothing is needed. SQLite's read path parses the value regardless of what the column calls itself, so the same stale column round-trips a real array. The finding is never raised there |

After a successful run the command re-runs detection and requires the finding to
be **gone**; a run whose statements succeeded while the column is still reported
exits non-zero rather than telling you it migrated something it did not.

<Callout type="warn">
**Rollback.** The conversion is not information-preserving: both `NULL` and the
empty string become `NULL`, so once it succeeds those two states cannot be told
apart again — **restoring your backup is the only faithful rollback**, which is
why there is no `--undo`.

Reverting only the column *type* (Postgres: `ALTER TABLE … ALTER COLUMN … TYPE
text USING …::text`) leaves JSON text in a text column; metadata still declares
the field multi-value, so the finding returns on the next boot and the
corruption resumes on the next write. Treat it as an incident stopgap, not a
rollback.

On **PostgreSQL** the whole remedy is one statement: if it fails, the column is
untouched and there is nothing to roll back. On **MySQL** it is three, and DDL
there commits implicitly — a failure midway leaves the table partly converted.
Re-run the command: each statement skips the rows a previous run already moved,
so finishing an interrupted run is safe.

If the `ALTER` fails naming an **index**, drop the index on that column first (a
json column cannot carry a plain btree) and re-run, then recreate it in a shape
your dialect supports for json.
</Callout>

**Rows corrupted before you migrate the column are yours to repair.** This
command converts the column and the values *in* it. A stringified array that a
hook or an integration already copied into some *other* single-value column is
not something it looks for, and it is deliberately not something it will grow
into: that repair is specific to what your automations did with the value.

#### Data migrations

The commands above reconcile **schema**. A *data* migration rewrites rows, and
Expand Down
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* [#11733] The command runs the ENGINE's statement, and works out WHICH dialect
* it is running by reading the engine's own finding.
*
* ## What this file used to be, and why it is smaller
*
* It began as a fidelity suite: `manualJsonConversionSql` was not on
* `@objectstack/driver-sql`'s public surface, so the command carried a copy of
* the statement and this suite held the copy to the engine's, byte for byte.
* The one-line re-export landed with this card, the copy is gone, and with it
* every assertion whose only job was to compare two spellings of one statement.
* Three cases were deleted rather than left behind:
*
* - "identifiers are the finding's own table and column" — it tested the
* CLI's builder handling its arguments. There is no CLI builder now; the
* call site's argument passing is covered by the plan cases below, which
* compare a real plan against `manualJsonConversionSql(dialect, …)`.
* - "postgres keeps both corrections measurement forced" and the content half
* of the MySQL case (`JSON_ARRAY` present, `json_build_array` absent). Both
* now assert `driver-sql`'s CONTENT from a consumer's suite. They are not
* vacuous — the engine could change that text — but that is precisely the
* problem: they could only ever fail for a reason that has nothing to do
* with this package, turning a deliberate engine correction into a red CLI
* suite. `driver-sql` owns those, and pins them in
* `schema-drift.base-type-mismatch.test.ts`, where they are also EXECUTED
* against live Postgres 16.13 and MySQL 8.0.46.
*
* ## What is left is not fidelity, and can still fail
*
* Two claims, both about this package:
*
* 1. **the coupling the dialect probe reads** — the finding's message still
* EMBEDS the remedy. Nothing in the CLI can keep that true, and everything
* in the CLI depends on it: the probe decides Postgres from MySQL by which
* dialect's statement the message contains, because a `ManagedDriftEntry`
* carries no dialect and a client-spelling table copied out of the driver
* could only disagree with it. If the engine ever stops interpolating the
* statement, these go red here — where the consumer that would silently
* lose its dialect lives.
* 2. **the split, and the refusal** — how this command turns one engine
* statement into the statements a seam takes, and what it does with a
* finding it cannot read a dialect from.
*/

import { describe, it, expect } from 'vitest';
import {
diffManagedTable,
manualJsonConversionSql,
type ManagedDriftEntry,
type PhysicalColumn,
} from '@objectstack/driver-sql';
import { splitRemedyStatements, planStaleColumnTargets, CORRUPTING_DIALECTS } from './multi-value-columns.js';

const TABLE = 'proj_task';
const COLUMN = 'tags';

/** Exactly what the command hands the planner in `run()`. */
const SQL = { sql: manualJsonConversionSql };

/** The stale column, in each dialect's own type spelling (#11720's fixtures). */
const STALE: Record<'postgres' | 'mysql', PhysicalColumn[]> = {
postgres: [{ name: COLUMN, type: 'character varying', nullable: true, maxLength: 255 }],
mysql: [{ name: COLUMN, type: 'varchar', nullable: true, maxLength: 255 }],
};

const engineFinding = (dialect: 'postgres' | 'mysql'): ManagedDriftEntry => {
const out = diffManagedTable({
table: TABLE,
fields: { [COLUMN]: { type: 'lookup', multiple: true } as any },
columns: STALE[dialect],
dialect,
});
// Non-vacuity: if the engine stopped reporting this shape, every assertion
// below would pass against nothing.
expect(out).toHaveLength(1);
expect(out[0].op.type).toBe('manual_column_type_change');
return out[0];
};

describe('the coupling the dialect probe depends on (#11733)', () => {
for (const dialect of CORRUPTING_DIALECTS) {
it(`${dialect}: the finding's message still EMBEDS the remedy, which is what the probe matches on`, () => {
// Not "the CLI agrees with the engine" — there is one function now, so
// that could not fail. This is the engine's MESSAGE against the engine's
// FUNCTION: it fails the day the message stops carrying the statement,
// which is the day this command can no longer tell Postgres from MySQL.
expect(engineFinding(dialect).message).toContain(manualJsonConversionSql(dialect, TABLE, COLUMN));
});
}

it('the two dialect forms are distinguishable — a probe reading one cannot match the other', () => {
// The premise of reading the dialect off the message. If the forms were
// substrings of one another the probe would resolve the wrong dialect and
// run the wrong DDL, so this is asserted rather than assumed.
const pg = manualJsonConversionSql('postgres', TABLE, COLUMN);
const my = manualJsonConversionSql('mysql', TABLE, COLUMN);
expect(pg).not.toBe(my);
expect(engineFinding('postgres').message).not.toContain(my);
expect(engineFinding('mysql').message).not.toContain(pg);
});
});

describe('splitting the engine statement into what a seam can run (#11733)', () => {
it('mysql is three statements, in the order that makes the ALTER survivable', () => {
const statements = splitRemedyStatements(manualJsonConversionSql('mysql', TABLE, COLUMN));
expect(statements).toHaveLength(3);
expect(statements[0]).toMatch(/^UPDATE .* JSON_ARRAY/);
expect(statements[1]).toMatch(/^UPDATE .*= NULL WHERE/);
expect(statements[2]).toMatch(/^ALTER TABLE .*MODIFY .*json$/);
});

it('postgres is ONE statement — which is why a failed conversion leaves the column untouched', () => {
// The rollback notes state this as a fact about Postgres; it is a fact
// about the STATEMENT, so it is read off the statement.
expect(splitRemedyStatements(manualJsonConversionSql('postgres', TABLE, COLUMN))).toHaveLength(1);
});

it('the semicolon split loses nothing — neither form carries a semicolon inside a literal', () => {
// The split is the one #11720's live suite executes the remedy with. It is
// safe because of a property of THESE statements, so the property is pinned
// rather than assumed: re-joining the parts reproduces the original.
for (const dialect of CORRUPTING_DIALECTS) {
const sql = manualJsonConversionSql(dialect, TABLE, COLUMN);
expect(`${splitRemedyStatements(sql).join('; ')};`).toBe(sql.trim());
}
});
});

describe('planning refuses anything it cannot read a dialect from (#11733)', () => {
for (const dialect of CORRUPTING_DIALECTS) {
it(`${dialect}: a real finding plans the engine's statements and names the dialect`, () => {
const plan = planStaleColumnTargets([engineFinding(dialect)], SQL);
expect(plan.refusals).toEqual([]);
expect(plan.targets).toHaveLength(1);
expect(plan.targets[0]).toMatchObject({ table: TABLE, column: COLUMN, to: 'json', dialect });
// Also the call site's argument passing: these are the statements for
// THIS table and column, not for a pair fixed anywhere in the command.
expect(plan.targets[0].statements).toEqual(
splitRemedyStatements(manualJsonConversionSql(dialect, TABLE, COLUMN)),
);
expect(plan.targets[0].from).toBe(dialect === 'postgres' ? 'character varying' : 'varchar');
});
}

it('a finding whose message carries no statement we can read a dialect from is REFUSED', () => {
// The failure this closes: the engine rewords the message, the probe can no
// longer tell Postgres from MySQL, and the command picks one anyway and
// runs the wrong dialect's DDL against a customer's table. It refuses
// instead — and says what to do by hand.
const entry = engineFinding('postgres');
const mutated = { ...entry, message: entry.message.replace('json_build_array', 'to_json') };

const plan = planStaleColumnTargets([mutated], 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');
});

it('ignores every drift op that is not this one', () => {
const others = diffManagedTable({
table: TABLE,
fields: { [COLUMN]: { type: 'string', maxLength: 50 } as any },
columns: STALE.postgres,
dialect: 'postgres',
});
expect(others.map((d) => d.op.type)).toEqual(['narrow_varchar']); // the instrument found something
expect(planStaleColumnTargets(others, SQL)).toEqual({ targets: [], refusals: [] });
});

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;
// `b`'s message still carries `proj_task`'s statement, so it can only be
// planned if the filter lets it through — which it must not.
expect(planStaleColumnTargets([a, b], { ...SQL, tables: [TABLE] }).targets.map((t) => t.table)).toEqual([TABLE]);
expect(planStaleColumnTargets([a, b], { ...SQL, tables: ['nothing_here'] }).targets).toEqual([]);
});
});
Loading
Loading