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
36 changes: 32 additions & 4 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,34 @@ here.**

### Changed

- **On Postgres, `meta verify --db` and `meta migrate` ask Postgres whether a CHECK or an index
expression is the one stored, instead of comparing rewritten text.** For each table the plan
creates a temporary table with the live columns inside a transaction it rolls back (a savepoint
when the connection is already in one), adds the expected CHECK to it, creates each expected
expression-index key and partial-index predicate as a temporary index, reads back
`pg_get_constraintdef`, `pg_get_indexdef` and `pg_get_expr`, and compares them with the live
definitions as plain strings. Every rewrite Postgres makes, including ones nobody has hand-coded,
now compares equal, and the string normaliser stops growing Postgres-shaped special cases. It
also ends the normaliser's two verified false negatives: a precedence change (`(a OR b) AND c`
against `a OR (b AND c)`) and a regrouped `NOT` (`NOT (a AND b)` against `NOT a AND b`) now report
as drift, as does the same change in an index `@where`. A database that held such a change and
verified clean before now reports it once: run `meta migrate` to bring it back.
The string normaliser stays, and decides, on SQLite and D1, in the offline `meta migrate`
(which has no database), on a read-only connection or hot standby, for a role without the `TEMP`
privilege, and for an expression that does not compile against the live columns. A probe that
cannot answer never reports drift or fails the command. An expression holding a `;` anywhere, or
an index `@using` that is not a plain access-method name, is never sent to the database. A refusal
for one table (such as no `SELECT` on it) gives that table no answer and does not stop the other
tables being probed. `meta migrate baseline --from-db` now records the generated text, not
Postgres's spelling, for a CHECK or an index expression the live database proves equal to the
metadata's, so the offline plan after a baseline compares like with like; it is best effort and
writes the introspected snapshot as before if the metadata does not load.
`meta verify --db` also uses the probe for its committed-snapshot check. New exports from
`@metaobjectsdev/migrate-ts`: `postgresExprCanonProbe`, `adoptGeneratedSpelling` and the
`ExprCanonProbe` types, with a `DiffArgs.exprCanonProbe` option. Schema comparison is
TypeScript-owned, so no other port carries this comparison. MySQL, SQL Server and Oracle are not
migrate dialects, so they have no probe.

- **A bare `verify` runs the same default gate set in every port: codegen drift, prompt
templates and the requirement ledger.** Until now a bare run covered one gate: `meta verify`
and `dotnet meta verify` ran only templates, so "run `verify` in CI" did not catch generated
Expand Down Expand Up @@ -273,10 +301,10 @@ here.**
`meta migrate` to bring it back. Separately, `char(n)` is no longer treated as a text column
for the `(col)::text` fold: its own regex operator sees the blank padding the cast trims, and
Postgres never adds the cast there, so an explicit `(col)::text ~ '…'` on a `char(n)` column
now reads as drift. A precedence change (`(a OR b) AND c` against `a OR (b AND c)`) is still
not detected; that needs Postgres to canonicalise the expected side. A new Postgres test
migrates, applies and verifies every CHECK shape `migrate` generates. Schema comparison is
TypeScript-owned, so no other port carries this comparison.
now reads as drift. A precedence change (`(a OR b) AND c` against `a OR (b AND c)`) is not
detected by the string comparison; on Postgres Postgres now answers that (see "Changed", the
CHECK probe). A new Postgres test migrates, applies and verifies every CHECK shape `migrate`
generates. Schema comparison is TypeScript-owned, so no other port carries this comparison.

### Documentation

Expand Down
39 changes: 39 additions & 0 deletions docs/features/migrations-and-drift.md
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,45 @@ opened by an older SQLite (Python 3.10's stdlib, Ubuntu 22.04's `sqlite3`). The
behind `--allow drop-check` and raises no data hazard, because the rule is the same — and
the re-diff is empty afterwards. Postgres keeps `IS [NOT] DISTINCT FROM`.

## How CHECK constraints and expression indexes are compared

Postgres rewrites an expression before it stores it (`"k" IN ('x')` becomes `k = 'x'::text`,
`a IS NOT DISTINCT FROM b` becomes `NOT (a IS DISTINCT FROM b)`, an `IN` list becomes
`= ANY (ARRAY[…])`, and casts and brackets are added), so the text `meta migrate` generates never
comes back from the database as written. On Postgres, a plan against the database
(`meta verify --db`, `meta migrate --from-db` and `--apply`) therefore **asks Postgres** instead of
guessing. For each table it creates a **temporary table with the live table's columns** inside a
transaction it rolls back (a savepoint, when the connection is already in one), adds the expected
CHECK to it, creates each expected expression or partial-index predicate as a temporary index,
and reads back `pg_get_constraintdef` / `pg_get_indexdef` / `pg_get_expr`. Those are compared with
the live definitions as plain strings. Nothing persists, and nothing is written to your table.
Two expressions that mean different things never compare equal: a precedence change
(`(a OR b) AND c` against `a OR (b AND c)`), a regrouped `NOT`, and a changed enum-member or regex
literal are all drift.

Where the question cannot be asked, the comparison falls back to a **string normaliser**
(`check-expr-compare.ts`), which rewrites both texts toward one shape. The fallback applies to:

- SQLite and D1, which store the text as written;
- the default **offline** `meta migrate`, which diffs the committed snapshot and has no database;
- a read-only connection or a hot standby, and a role without the `TEMP` privilege;
- a table the role cannot read (no `SELECT` on it, or no `USAGE` on its schema): that table only,
the other tables are still asked;
- an expected expression that does not compile against the live columns (the column diff reports
that case);
- an expression containing a `;` anywhere, quoted or not, or an index `@using` that is not a plain
access-method name: neither is ever sent to the database.

The normaliser cannot tell a precedence change from a bracket-only change (it deletes every
bracket), so on the fallback path `(a OR b) AND c` and `a OR (b AND c)` compare as equal. A fallback
never reports drift of its own: a probe that fails changes nothing about the answer.

`meta migrate baseline --from-db` on Postgres closes the offline gap. Where the live database
proves a CHECK, or an expression index's key or predicate, is what the metadata would generate, the
snapshot records the **generated** text rather than Postgres's spelling, so the offline plan
compares like with like and a baselined project starts clean. Anything it cannot prove keeps
Postgres's text. If the metadata does not load, the baseline is written as before.

## Drift verify commands per port

| Port | Command | What it does |
Expand Down
83 changes: 63 additions & 20 deletions server/typescript/packages/cli/src/commands/migrate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,16 @@ import {
type AmbiguousResolution,
type Change,
type DataHazard,
type Dialect,
type D1Binding,
type EmitResult,
type D1Runner,
type SchemaProvenance,
type SchemaSnapshot,
type TableDescriptor,
postgresViewReplaceProbe,
postgresExprCanonProbe,
adoptGeneratedSpelling,
} from "@metaobjectsdev/migrate-ts";
import {
buildWranglerExecuteArgs,
Expand Down Expand Up @@ -825,6 +828,9 @@ export async function migrateCommand(
// Postgres decides whether a changed view can be replaced in place: the probe
// creates the new definition as a temporary view in a transaction it rolls back.
...(kysely.dialect === "postgres" ? { viewReplaceProbe: postgresViewReplaceProbe(kysely.db) } : {}),
// ...and the same for CHECK / index expressions: Postgres says how it stores the expected
// text, so a stored rewrite no string rule knows is not proposed as a drop and re-add.
...(kysely.dialect === "postgres" ? { exprCanonProbe: postgresExprCanonProbe(kysely.db) } : {}),
allow: tokensToAllowOptions(config.allow),
renames: config.renames,
// #258 — adopting a live DB whose PRIMARY KEY differs from the metadata identity
Expand Down Expand Up @@ -1113,6 +1119,54 @@ export async function migrateCommand(
}
}

/**
* Load the project's metadata the way `baseline` does: `metaobjects.config.ts` is read ONCE, up
* front, for the consumer providers/libraries (mirroring the DB path and `meta gen`, so
* config-registered custom subtypes resolve too, #157), and the result is deliberately NOT
* scoped — `baseline` records a STARTING POINT.
*
* Throws what loading throws; a `RemovedNamingConfigError` from the config propagates unchanged.
*/
async function loadBaselineMetadata(metaRoot: string, genRoot: string) {
let baselineLoadOptions: ReturnType<typeof loadMemoryOptionsFrom> = {};
try {
const cfg = await loadMetaobjectsConfig(genRoot);
baselineLoadOptions = loadMemoryOptionsFrom(cfg);
} catch (e) {
if (e instanceof RemovedNamingConfigError) throw e;
// config absent — no custom providers, no libraries, default snake_case
}
const collection = await resolveCollection(metaRoot);
return await loadMemory(collection.configDir, {
...collectionLoadOptions(collection),
...baselineLoadOptions,
});
}

/**
* Postgres `baseline --from-db`: where the live database proves a CHECK or index expression is
* exactly what the metadata would generate, record the GENERATED text instead of Postgres's
* spelling of it, so the offline plan compares like with like (see migrate-ts
* `adoptGeneratedSpelling`). Best effort and never fatal: baseline worked without metadata
* before, so metadata that will not load, or a database that cannot answer the probe, leaves
* the introspected snapshot exactly as it was.
*/
async function withGeneratedSpelling(
snapshot: SchemaSnapshot,
db: Parameters<typeof introspect>[0],
dialect: Dialect,
metaRoot: string,
genRoot: string,
): Promise<SchemaSnapshot> {
try {
const metadata = await loadBaselineMetadata(metaRoot, genRoot);
const expected = baselineFromMetadata(metadata, dialect, buildProjectionViews(metadata, { dialect }));
return await adoptGeneratedSpelling(snapshot, expected, postgresExprCanonProbe(db));
} catch {
return snapshot;
}
}

/**
* `meta migrate baseline [--from-db]` — seed the committed reference snapshot.
* `--from-metadata` (default) derives it from metadata; `--from-db` introspects
Expand Down Expand Up @@ -1150,6 +1204,9 @@ export async function runBaseline(
try {
snapshot = await introspect(kysely.db, kysely.dialect);
if (config.format === "flyway") snapshot = withoutFlywayHistory(snapshot);
if (kysely.dialect === "postgres") {
snapshot = await withGeneratedSpelling(snapshot, kysely.db, config.dialect, metaRoot, genRoot);
}
} finally {
await kysely.close();
}
Expand Down Expand Up @@ -1181,21 +1238,6 @@ export async function runBaseline(
`baseline — this emits NO CREATE TABLE. Use it only if your database already matches ` +
`your metadata; for a new/empty database run \`${greenfieldCreateCmd(config.dialect)}\` instead.`,
);
let metadata;
// Load metaobjects.config.ts ONCE, up front, for the consumer providers/libraries —
// mirroring the DB path (and `meta gen`) so offline baseline resolves
// config-registered custom subtypes too (#157).
let baselineLoadOptions: ReturnType<typeof loadMemoryOptionsFrom> = {};
try {
const cfg = await loadMetaobjectsConfig(genRoot);
baselineLoadOptions = loadMemoryOptionsFrom(cfg);
} catch (e) {
if (e instanceof RemovedNamingConfigError) {
log.error(e.message);
return 2;
}
// config absent — no custom providers, no libraries, default snake_case
}
// `baseline` records a STARTING POINT, so it is deliberately NOT scoped: the
// `--from-db` arm captures whatever the database holds (there is no provenance
// for an introspected table), and an offline baseline that recorded less would
Expand All @@ -1205,13 +1247,14 @@ export async function runBaseline(
// it — which is what makes that true. Committing the narrowed schema instead
// would delete the entry, and removing the scope later would then propose
// CREATE TABLE for a table that exists.
let metadata;
try {
const collection = await resolveCollection(metaRoot);
metadata = await loadMemory(collection.configDir, {
...collectionLoadOptions(collection),
...baselineLoadOptions,
});
metadata = await loadBaselineMetadata(metaRoot, genRoot);
} catch (err) {
if (err instanceof RemovedNamingConfigError) {
log.error(err.message);
return 2;
}
reportLoadError(log, "migrate baseline: failed to load metadata", err);
return 1;
}
Expand Down
12 changes: 11 additions & 1 deletion server/typescript/packages/cli/src/commands/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ import {
type D1Binding,
type D1Runner,
type DriftResult,
type ExprCanonProbe,
postgresViewReplaceProbe,
postgresExprCanonProbe,
withoutFlywayHistory,
} from "@metaobjectsdev/migrate-ts";
import {
Expand Down Expand Up @@ -1347,6 +1349,8 @@ export async function verifyCommand(
// Expected views from the single view-SQL source (codegen-ts), so view-body
// drift is detected against the live DB.
const expectedViews = buildProjectionViews(root, { dialect: kysely.dialect });
// One probe for the whole run, so a server that refuses scratch tables is asked once.
const exprCanonProbe = kysely.dialect === "postgres" ? postgresExprCanonProbe(kysely.db) : undefined;
let driftResult;
let actual: SchemaSnapshot;
try {
Expand All @@ -1363,6 +1367,8 @@ export async function verifyCommand(
views: expectedViews,
// The same question `meta migrate` asks, so the report names the change it plans.
...(kysely.dialect === "postgres" ? { viewReplaceProbe: postgresViewReplaceProbe(kysely.db) } : {}),
// ...and asks Postgres, not a string rewrite, how it stores each expected CHECK and index expression.
...(exprCanonProbe !== undefined ? { exprCanonProbe } : {}),
...(schemaScope !== undefined ? { inScope: schemaScope } : {}),
// The import exclusion reaches scopeExpectedSchema through this option bag —
// there is no direct call on this path to attach it to.
Expand All @@ -1376,7 +1382,7 @@ export async function verifyCommand(

const snapshotDrift =
driftResult.changes.length === 0
? await checkCommittedSnapshot(actual, kysely.dialect, kysely.displayUrl, driftResult, migrateConfig)
? await checkCommittedSnapshot(actual, kysely.dialect, kysely.displayUrl, driftResult, migrateConfig, exprCanonProbe)
: [];

return reportSchemaDrift(driftResult, [...ledgerDrift, ...snapshotDrift], kysely.displayUrl);
Expand Down Expand Up @@ -1503,6 +1509,9 @@ export async function verifyCommand(
// Resolved by the caller and threaded through — see the `verify --db` branch: the same
// run's flyway filter reads this config too, and `tryLoadConfig` does not cache.
migrateConfig: ResolvedMigrateConfig,
// Postgres: the snapshot may hold either spelling of an expression (generated, or
// Postgres's own from a `baseline --from-db`); the database settles which is equal.
exprCanonProbe?: ExprCanonProbe,
): Promise<string[]> {
if (dialect === "d1") return []; // d1 keeps migrations Wrangler-native; no offline snapshot
// The migrations dir comes from migrate's OWN precedence (flag > config > default)
Expand Down Expand Up @@ -1541,6 +1550,7 @@ export async function verifyCommand(
// Note `unmanagedNames` is NOT restated here: `scopedDiffInputs` above supplies it
// MERGED with the out-of-scope set, and a second key would silently drop that half.
dialect,
...(exprCanonProbe !== undefined ? { exprCanonProbe } : {}),
});
// A view recreated only around a table change matches the snapshot (D3) — not a difference.
const changes = withoutViewRecreates(result.changes);
Expand Down
Loading
Loading