diff --git a/README.md b/README.md index 04f10b3..871b4b9 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ Built by **[Akal](https://github.com/akalsoftware)**. - RLS policies and RLS coverage; - storage buckets and their policies; - auth config and Edge Functions; - - cron jobs, webhooks and Realtime; + - cron jobs, queues, webhooks and Realtime; - Vault, extensions, roles and grants; - reference data and migration history. - **Fixes, not just reports.** Fixes run in dependency order, in one diff --git a/packages/cli/README.md b/packages/cli/README.md index c19a013..9f5934e 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -9,13 +9,16 @@ order, in one transaction, and can be proved on a throwaway copy before anything changes. With a single project, it snapshots, restores and clones it. - **The whole project.** It covers schema, RLS, storage, auth, Edge Functions, - cron, webhooks, Realtime, Vault, extensions, grants, reference data and - migration history. + cron, webhooks, Realtime, queues, partition sets, Vault, extensions, grants, + reference data and migration history. - **Safe by default.** Every write needs `--apply`, and anything that loses data or opens up access also needs `--allow-destructive`. A failed apply leaves the target exactly as it was. +- **Production-aware.** Each fix says what it does to a live database: the + lock it holds and on which table, whether it may rewrite the table or scan + every row, whether it can't run in a transaction. - **Provable.** `--prove` rehearses the migration and refuses to apply one - that wouldn't reproduce the source. + that wouldn't reproduce the source or would lose rows. - **Anywhere.** It works with Supabase Cloud, self-hosted stacks, plain PostgreSQL targets, CI pipelines and AI agents (MCP). @@ -63,6 +66,8 @@ Docker image needs nothing at all, and carries the PostgreSQL client tools | Storage | Buckets and their policies (objects are never copied) | API and SQL | | Auth config | Management API, or GoTrue on self-hosted | API (hosted) | | Cron jobs | `cron.job` | SQL | +| Queues | Supabase Queues (`pgmq.meta`), created and dropped through pgmq | SQL | +| Partition sets | pg_partman registrations (`part_config`), made with `create_parent` | SQL | | Webhooks | Triggers calling `supabase_functions.http_request` | SQL | | Realtime | Publications, and policies on `realtime.messages` | SQL | | Vault | Secret names and descriptions (values can't be read) | Command to run | @@ -71,10 +76,17 @@ Docker image needs nothing at all, and carries the PostgreSQL client tools | Roles and grants | Custom roles, and grants on tables, views, sequences and functions, including `anon`, `authenticated`, `service_role` | SQL | The schema check covers tables, columns, keys, indexes, partitions, views, -materialized views, functions, triggers, sequences, enums, composite types, -domains, policies, extensions and comments. It runs in every schema except -Supabase's own (`auth`, `storage`, and so on) and those in `ignoreSchemas`. -Objects owned by an extension belong to it and aren't compared. +materialized views, functions, aggregates, triggers, event triggers, rules, +sequences, enums, composite and range types, domains, casts, extended +statistics, text search configurations and dictionaries, foreign servers, +user mappings and foreign tables, policies, default privileges, replica +identity, statistics targets, extensions and comments. A renamed column, +table or sequence is renamed, keeping its rows, rather than dropped and +created again. It runs in every schema except Supabase's own (`auth`, +`storage`, and so on) and those in `ignoreSchemas`. Objects owned by an +extension belong to it and aren't compared. Objects of a kind it doesn't +compare, such as operator classes, are named under the check so you can look +at them by hand; they aren't counted as drift. A check that can't run, for example without API credentials, says so and why. It's never reported as a clean pass. @@ -109,6 +121,7 @@ Useful `diff` flags: | `--only=` | Apply only these findings (ids come from `--json`, globs allowed) | | `--dry-run` | Print the fixes in the order they'd run, run nothing | | `--prove` | Rehearse the fixes on a throwaway copy first | +| `--source-dir ` | Compare a directory of schema files (such as `supabase/schemas`) with the target instead of a source database | | `--allow-destructive` | Also apply drops and deletes | | `--apply-posture` | Also apply the target-only checks' fixes | | `--no-transaction` | Apply fix by fix, keeping what works | @@ -129,11 +142,13 @@ Useful `diff` flags: is unchanged. A fix the target can't hold, because it needs an extension or a Supabase schema the target lacks, is skipped with the reason, and the rest still applies. -- **`--prove`.** The fixes are replayed on a structure-only copy of the target, - made on the target's own server, the way the apply will run them. Nothing is - applied unless the result matches the source, and nothing is applied if the - proof can't run. Changes a copy can't hold, such as cron jobs (pg_cron lives - in one database per server), are listed as not proved. +- **`--prove`.** The fixes are replayed on a copy of the target's structure, + made on the target's own server, the way the apply will run them, with a + sample of each table's rows. Nothing is applied unless the result matches + the source and the rows survive, and nothing is applied if the proof can't + run. It also names the tables the fixes rewrite. Changes a copy can't hold, + such as cron jobs (pg_cron lives in one database per server), are listed as + not proved. ```bash supaforge diff --dry-run # the plan, in order @@ -260,6 +275,7 @@ supaforge migrate run --env=prod --apply | `SUPAFORGE_DBDIFF_MEMORY` | `1G` | Memory limit for `@dbdiff/cli` (`2G`, `-1` for none) | | `SUPAFORGE_CONNECT_TIMEOUT` | `15` | Seconds to wait for a connection | | `SUPAFORGE_CHECK_CONCURRENCY` | `4` | Checks run at once. Lower it behind a tight pooler | +| `SUPAFORGE_PROVE_ROWS` | `1000` | Rows of each table `--prove` copies to check they survive; `0` checks structure only | | `SUPAFORGE_PG_BIN` | — | A directory of PostgreSQL client tools (`pg_dump`, `psql`), used ahead of `PATH` — for a server newer than the tools on `PATH` | ## Self-hosted Supabase diff --git a/packages/cli/src/checks/base.ts b/packages/cli/src/checks/base.ts index 1edf035..d30a2bd 100644 --- a/packages/cli/src/checks/base.ts +++ b/packages/cli/src/checks/base.ts @@ -18,6 +18,8 @@ export interface CheckContext { * fine-grained to report simply ignore it. */ onDetail?: (detail: string) => void + /** Sink for what the check could not compare — see CheckResult.notices. */ + onNotice?: (notice: string) => void source: EnvironmentConfig target: EnvironmentConfig config: SupaForgeConfig diff --git a/packages/cli/src/checks/extensions.ts b/packages/cli/src/checks/extensions.ts index 055bab9..4c791a4 100644 --- a/packages/cli/src/checks/extensions.ts +++ b/packages/cli/src/checks/extensions.ts @@ -22,7 +22,8 @@ export class ExtensionsCheck extends Check { this.fetchExtensions(ctx.source.dbUrl), this.fetchExtensions(ctx.target.dbUrl), ]) - return diffExtensions(source, target) + const ignored = new Set(ctx.config.ignoreExtensions ?? []) + return diffExtensions(source, target.filter(e => !ignored.has(e.name) || source.some(s => s.name === e.name))) } private async fetchExtensions(dbUrl: string): Promise { diff --git a/packages/cli/src/checks/index.ts b/packages/cli/src/checks/index.ts index 97c43ae..f8e5b19 100644 --- a/packages/cli/src/checks/index.ts +++ b/packages/cli/src/checks/index.ts @@ -13,6 +13,8 @@ import { VaultCheck } from './vault.js' import { ExtensionsCheck } from './extensions.js' import { MigrationsCheck } from './migrations.js' import { RolesCheck } from './roles.js' +import { QueuesCheck } from './queues.js' +import { PartitionsCheck } from './partitions.js' export interface RegistryOptions { /** Enable file-level drift detection in the storage check. */ @@ -35,6 +37,8 @@ export function createDefaultRegistry(options: RegistryOptions = {}): CheckRegis registry.register(new ExtensionsCheck()) registry.register(new MigrationsCheck()) registry.register(new RolesCheck()) + registry.register(new QueuesCheck()) + registry.register(new PartitionsCheck()) return registry } @@ -54,3 +58,5 @@ export { VaultCheck } from './vault.js' export { ExtensionsCheck } from './extensions.js' export { MigrationsCheck } from './migrations.js' export { RolesCheck } from './roles.js' +export { QueuesCheck } from './queues.js' +export { PartitionsCheck } from './partitions.js' diff --git a/packages/cli/src/checks/partitions.ts b/packages/cli/src/checks/partitions.ts new file mode 100644 index 0000000..90a5b75 --- /dev/null +++ b/packages/cli/src/checks/partitions.ts @@ -0,0 +1,179 @@ +import type { QueryFn } from '../db' +import { pgQuery } from '../db' +import type { DriftIssue } from '../types/drift' +import { quoteIdent, quoteLiteral } from '../utils/sql' +import { Check, CheckSkipped, type CheckContext } from './base' + +/** A pg_partman partition set, as its part_config row records it. */ +export interface PartitionSet { + parent_table: string + control: string + partition_interval: string + partition_type: string + epoch: string + premake: number + automatic_maintenance: string + constraint_cols: string[] | null + jobmon: boolean + date_trunc_interval: string | null + time_encoder: string | null + time_decoder: string | null + has_default: boolean + /** The settings part_config takes by UPDATE, by column. */ + settings: Record +} + +/** What decides how the set is partitioned: changed, it has to be rebuilt. */ +const SHAPE = ['control', 'partition_interval', 'partition_type', 'epoch', 'time_encoder', 'time_decoder'] as const + +/** part_config columns pg_partman documents setting with UPDATE. */ +const SETTINGS = [ + 'premake', 'automatic_maintenance', 'retention', 'retention_schema', 'retention_keep_index', 'retention_keep_table', + 'optimize_constraint', 'infinite_time_partitions', 'inherit_privileges', 'constraint_valid', 'ignore_default_data', + 'maintenance_order', 'retention_keep_publication', 'detach_before_drop', 'jobmon', 'date_trunc_interval', +] + +/** + * pg_partman partition sets, compared by their registration. + * + * The partitions themselves are pg_partman's to make and drop, so the schema + * diff leaves them alone; what makes a set is its row in part_config, written + * by create_parent(). A set the target lacks is registered through that + * function, and its settings put as the source has them; a setting that + * differs is updated where it stands. How the set is partitioned — its + * control column, interval or type — cannot be changed in place, so a + * difference there, and a set only the target has, are left to the user. + */ +export class PartitionsCheck extends Check { + readonly name = 'partitions' as const + + constructor(private queryFn: QueryFn = pgQuery) { + super() + } + + async scan(ctx: CheckContext): Promise { + const [source, target] = await Promise.all([this.fetch(ctx.source.dbUrl), this.fetch(ctx.target.dbUrl)]) + if (source.schema === null && target.schema === null) { + throw new CheckSkipped('pg_partman extension not installed') + } + return diffPartitionSets(source.sets, target.sets, target.schema) + } + + private async fetch(dbUrl: string): Promise<{ schema: string | null; sets: PartitionSet[] }> { + const rows = await this.queryFn(dbUrl, `SELECT n.nspname AS schema FROM pg_extension e + JOIN pg_namespace n ON n.oid = e.extnamespace WHERE e.extname = 'pg_partman'`) as Array<{ schema: string }> + const schema = rows[0]?.schema ?? null + if (schema === null) return { schema, sets: [] } + const sets = await this.queryFn(dbUrl, ` + SELECT (SELECT n.nspname || '.' || c.relname FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.oid = to_regclass(pc.parent_table)) AS parent_table, pc.control, pc.partition_interval, pc.partition_type, + pc.epoch, pc.premake, pc.automatic_maintenance, pc.constraint_cols, pc.jobmon, pc.date_trunc_interval, + pc.time_encoder, pc.time_decoder, + to_regclass(pc.parent_table || '_default') IS NOT NULL AS has_default, + (to_jsonb(pc) - ARRAY(SELECT k FROM jsonb_object_keys(to_jsonb(pc)) k WHERE k <> ALL($1::text[]))) AS settings + FROM ${quoteIdent(schema)}.part_config pc + WHERE to_regclass(pc.parent_table) IS NOT NULL + ORDER BY 1`, [SETTINGS]) as unknown as PartitionSet[] + return { schema, sets } + } +} + +const literal = (v: unknown): string => + v === null || v === undefined ? 'NULL' + : typeof v === 'boolean' || typeof v === 'number' ? String(v) + : Array.isArray(v) ? `ARRAY[${v.map(x => quoteLiteral(String(x))).join(', ')}]::text[]` + : quoteLiteral(String(v)) + +function registerSql(set: PartitionSet, schema: string): string { + const args = [ + `p_parent_table => ${quoteLiteral(set.parent_table)}`, + `p_control => ${quoteLiteral(set.control)}`, + `p_interval => ${quoteLiteral(set.partition_interval)}`, + `p_type => ${quoteLiteral(set.partition_type)}`, + `p_epoch => ${quoteLiteral(set.epoch)}`, + `p_premake => ${set.premake}`, + `p_default_table => ${set.has_default}`, + `p_automatic_maintenance => ${quoteLiteral(set.automatic_maintenance)}`, + `p_jobmon => ${set.jobmon}`, + ...(set.constraint_cols?.length ? [`p_constraint_cols => ${literal(set.constraint_cols)}`] : []), + ...(set.date_trunc_interval ? [`p_date_trunc_interval => ${quoteLiteral(set.date_trunc_interval)}`] : []), + ...(set.time_encoder ? [`p_time_encoder => ${quoteLiteral(set.time_encoder)}`] : []), + ...(set.time_decoder ? [`p_time_decoder => ${quoteLiteral(set.time_decoder)}`] : []), + ] + return `SELECT ${quoteIdent(schema)}.create_parent(${args.join(', ')});` +} + +function updateSql(schema: string, parent: string, settings: Record): string { + const sets = Object.entries(settings).map(([k, v]) => `${quoteIdent(k)} = ${literal(v)}`) + return `UPDATE ${quoteIdent(schema)}.part_config SET ${sets.join(', ')} WHERE parent_table = ${quoteLiteral(parent)};` +} + +function undoSql(schema: string, parent: string): string { + return `DELETE FROM ${quoteIdent(schema)}.part_config WHERE parent_table = ${quoteLiteral(parent)};` +} + +export function diffPartitionSets(source: PartitionSet[], target: PartitionSet[], targetSchema: string | null): DriftIssue[] { + const issues: DriftIssue[] = [] + const targetMap = new Map(target.map(s => [s.parent_table, s])) + const sourceNames = new Set(source.map(s => s.parent_table)) + + for (const s of source) { + const t = targetMap.get(s.parent_table) + if (!t) { + issues.push({ + id: `partitions-missing-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set not registered: ${s.parent_table}`, + description: `pg_partman manages ${s.parent_table} in source (${s.partition_type} by ${s.control}, every ${s.partition_interval}) but not in target.`, + sourceValue: s, + ...(targetSchema + ? { sql: { up: `${registerSql(s, targetSchema)}\n${updateSql(targetSchema, s.parent_table, s.settings)}`, down: undoSql(targetSchema, s.parent_table) } } + : { manualOnly: 'the target does not have pg_partman; install it, then re-run to register the partition set' }), + }) + continue + } + const reshaped = SHAPE.filter(k => String(s[k] ?? '') !== String(t[k] ?? '')) + if (reshaped.length > 0) { + issues.push({ + id: `partitions-shape-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set differs: ${s.parent_table}`, + description: `${reshaped.join(', ')} differ${reshaped.length === 1 ? 's' : ''} for ${s.parent_table}. pg_partman cannot change that in place.`, + sourceValue: s, + targetValue: t, + manualOnly: 'changing how a set is partitioned means undoing and recreating it with pg_partman (undo_partition, then create_parent)', + }) + continue + } + const changed = Object.fromEntries(Object.entries(s.settings).filter(([k, v]) => JSON.stringify(v) !== JSON.stringify(t.settings[k]))) + if (Object.keys(changed).length > 0 && targetSchema) { + const previous = Object.fromEntries(Object.keys(changed).map(k => [k, t.settings[k]])) + issues.push({ + id: `partitions-settings-${s.parent_table}`, + check: 'partitions', + severity: 'warning', + title: `Partition set settings differ: ${s.parent_table}`, + description: `${Object.keys(changed).join(', ')} differ for ${s.parent_table}.`, + sourceValue: s.settings, + targetValue: t.settings, + sql: { up: updateSql(targetSchema, s.parent_table, changed), down: updateSql(targetSchema, s.parent_table, previous) }, + }) + } + } + + for (const t of target) { + if (sourceNames.has(t.parent_table)) continue + issues.push({ + id: `partitions-extra-${t.parent_table}`, + check: 'partitions', + severity: 'info', + title: `Extra partition set: ${t.parent_table}`, + description: `pg_partman manages ${t.parent_table} in target but not in source.`, + targetValue: t, + manualOnly: 'unregistering a set leaves its partitions as ordinary tables; use pg_partman\'s undo_partition if that is what you want', + }) + } + return issues +} diff --git a/packages/cli/src/checks/queues.ts b/packages/cli/src/checks/queues.ts new file mode 100644 index 0000000..f95fcee --- /dev/null +++ b/packages/cli/src/checks/queues.ts @@ -0,0 +1,115 @@ +import type { QueryFn } from '../db' +import { pgQuery } from '../db' +import type { DriftIssue } from '../types/drift' +import { quoteLiteral } from '../utils/sql' +import { Check, CheckSkipped, readOrAbsent, type CheckContext } from './base' + +/** A pgmq queue, as `pgmq.meta` records it. */ +export interface Queue { + queue_name: string + is_partitioned: boolean + is_unlogged: boolean +} + +/** + * Supabase Queues: pgmq's queues, compared by name and kind. + * + * A queue is tables pgmq makes and registers as its extension's own, so a + * schema diff — which leaves an extension's objects to the extension — + * never saw one: a queue the target lacked went unreported. Compared here + * from `pgmq.meta`, and created or dropped through pgmq's own functions, + * which keep its registry and the queue's tables in step. Dropping a queue + * drops its messages, so it is held back like any destructive fix. + */ +export class QueuesCheck extends Check { + readonly name = 'queues' as const + + constructor(private queryFn: QueryFn = pgQuery) { + super() + } + + async scan(ctx: CheckContext): Promise { + const [source, target] = await Promise.all([ + this.fetchQueues(ctx.source.dbUrl), + this.fetchQueues(ctx.target.dbUrl), + ]) + if (!source.available && !target.available) { + throw new CheckSkipped('pgmq extension not installed') + } + return diffQueues(source.queues, target.queues, target.available) + } + + private async fetchQueues(dbUrl: string): Promise<{ queues: Queue[]; available: boolean }> { + return readOrAbsent<{ queues: Queue[]; available: boolean }>( + async () => ({ queues: await this.queryFn(dbUrl, QUEUES_SQL) as unknown as Queue[], available: true }), + { queues: [], available: false }, + ) + } +} + +const QUEUES_SQL = ` + SELECT queue_name, is_partitioned, is_unlogged + FROM pgmq.meta + ORDER BY queue_name +` + +/** The statement pgmq creates one with, or undefined for a partitioned queue. */ +function createSql(q: Queue): string | undefined { + if (q.is_partitioned) return undefined + return `SELECT pgmq.${q.is_unlogged ? 'create_unlogged' : 'create'}(${quoteLiteral(q.queue_name)});` +} + +const dropSql = (q: Queue) => `SELECT pgmq.drop_queue(${quoteLiteral(q.queue_name)});` + +const kind = (q: Queue) => q.is_partitioned ? 'partitioned' : q.is_unlogged ? 'unlogged' : 'standard' + +export function diffQueues(source: Queue[], target: Queue[], targetHasPgmq = true): DriftIssue[] { + const issues: DriftIssue[] = [] + const sourceMap = new Map(source.map(q => [q.queue_name, q])) + const targetMap = new Map(target.map(q => [q.queue_name, q])) + const needsExtension = targetHasPgmq ? '' : 'CREATE EXTENSION IF NOT EXISTS pgmq;\n' + + for (const [name, q] of sourceMap) { + const t = targetMap.get(name) + if (!t) { + const up = createSql(q) + issues.push({ + id: `queues-missing-${name}`, + check: 'queues', + severity: 'warning', + title: `Missing queue: ${name}`, + description: `Queue "${name}" (${kind(q)}) exists in source but not in target.`, + sourceValue: q, + ...(up + ? { sql: { up: needsExtension + up, down: dropSql(q) } } + : { manualOnly: 'a partitioned queue is created with pg_partman settings pgmq.meta does not record — create it with pgmq.create_partitioned on the target' }), + }) + } else if (kind(q) !== kind(t)) { + issues.push({ + id: `queues-kind-${name}`, + check: 'queues', + severity: 'warning', + title: `Queue kind differs: ${name}`, + description: `Queue "${name}" is ${kind(q)} in source and ${kind(t)} in target. Changing it means dropping the queue and its messages, so it is left to you.`, + sourceValue: q, + targetValue: t, + manualOnly: 'changing a queue\'s kind drops it and its messages; recreate it by hand when it is empty', + }) + } + } + + for (const [name, t] of targetMap) { + if (sourceMap.has(name)) continue + const down = createSql(t) + issues.push({ + id: `queues-extra-${name}`, + check: 'queues', + severity: 'critical', + title: `Extra queue: ${name}`, + description: `Queue "${name}" exists in target but not in source. Dropping it drops its messages.`, + targetValue: t, + sql: { up: dropSql(t), down: down ?? '' }, + }) + } + return issues +} diff --git a/packages/cli/src/checks/schema.ts b/packages/cli/src/checks/schema.ts index 472e024..2bfc3f4 100644 --- a/packages/cli/src/checks/schema.ts +++ b/packages/cli/src/checks/schema.ts @@ -1,7 +1,7 @@ import type { QueryFn } from '../db' import { pgQuery } from '../db' import type { DriftIssue } from '../types/drift' -import { runDbDiff, sqlToIssues, type DbDiffOptions } from '../dbdiff' +import { runDbDiff, schemaNotices, sqlToIssues, type DbDiffOptions } from '../dbdiff' import { Check, CheckSkipped, type CheckContext } from './base' import { DEFAULT_IGNORE_SCHEMAS } from '../defaults' @@ -47,6 +47,7 @@ export class SchemaCheck extends Check { }), this.fetchIgnoredSchemaTables(ctx.target.dbUrl, ignoreSchemas), ]) + for (const notice of schemaNotices(result.notices ?? [])) ctx.onNotice?.(notice) return sqlToIssues(result, 'schema', ignoreSchemas, ignoredSchemaTables) } catch (err) { if (err instanceof Error && err.message.includes('@dbdiff/cli is not installed')) { diff --git a/packages/cli/src/ci.ts b/packages/cli/src/ci.ts index 99ee87f..b19687a 100644 --- a/packages/cli/src/ci.ts +++ b/packages/cli/src/ci.ts @@ -123,6 +123,8 @@ export function formatCiSummary(result: ScanResult): { skipped: Array<{ check: string; reason: string }> /** How many checks compared the two environments, over how many were attempted. */ coverage: { compared: number; total: number } + /** What a check could not compare, said beside its findings — see CheckResult.notices. */ + notices: Array<{ check: string; notice: string }> criticalIssues: Array<{ check: string; id: string; title: string }> warningIssues: Array<{ check: string; id: string; title: string }> } { @@ -130,6 +132,7 @@ export function formatCiSummary(result: ScanResult): { const warningIssues: Array<{ check: string; id: string; title: string }> = [] const errors: Array<{ check: string; message: string }> = [] const skipped: Array<{ check: string; reason: string }> = [] + const notices: Array<{ check: string; notice: string }> = [] for (const check of result.checks ?? []) { if (check?.status === 'error') { @@ -138,6 +141,7 @@ export function formatCiSummary(result: ScanResult): { if (check?.status === 'skipped') { skipped.push({ check: check.check, reason: check.skipReason ?? 'no reason given' }) } + for (const notice of check?.notices ?? []) notices.push({ check: check.check, notice }) for (const issue of check?.issues ?? []) { const entry = { check: check.check, id: issue.id, title: issue.title } if (issue.severity === 'critical') { @@ -156,6 +160,7 @@ export function formatCiSummary(result: ScanResult): { errors, skipped, coverage: coverage(result), + notices, criticalIssues, warningIssues, } diff --git a/packages/cli/src/commands/diff.ts b/packages/cli/src/commands/diff.ts index fbf7fd9..06e9410 100644 --- a/packages/cli/src/commands/diff.ts +++ b/packages/cli/src/commands/diff.ts @@ -1,4 +1,4 @@ -import { Flags } from '@oclif/core' +import { Flags, type Interfaces } from '@oclif/core' import { BaseCommand } from '../base-command.js' import { createDefaultRegistry } from '../checks/index.js' import { scan } from '../scanner.js' @@ -20,6 +20,14 @@ import { CLONE_SKIP_FLAGS, DEFAULT_IGNORE_SCHEMAS, schemaDiffIgnores } from '../ import { Preflight } from '../preflight.js' import type { SupaForgeConfig } from '../types/config.js' import { redactUrls, describeFailure } from '../utils/error.js' +import { openSchemaDir } from '../schema-dir.js' + + +/** The source environment a --source-dir run compares from. */ +const SCHEMA_FILES_ENV = 'schema files' + +/** What schema files can describe: the database's structure, its policies, its extensions. */ +const SCHEMA_FILE_CHECKS: CheckName[] = ['schema', 'rls', 'extensions'] /** * Glyph and text for a finished check, so the three outcomes are visually @@ -136,6 +144,11 @@ export default class Diff extends BaseCommand { ] static override flags = { + 'source-dir': Flags.string({ + description: 'Compare a directory of schema files (such as supabase/schemas) with the target, ' + + 'instead of a source database. They are loaded into a throwaway database on the target\'s server.', + exclusive: ['source'], + }), check: Flags.string({ char: 'l', description: 'Limit to a specific check', @@ -286,10 +299,40 @@ export default class Diff extends BaseCommand { } const config = await this.loadConfigOrFail() + + // Schema files as the source: loaded into a throwaway database on the + // target's server, compared, and dropped — see schema-dir.ts. + if (flags['source-dir']) { + const { envName: targetName } = this.resolveEnv(config, flags.target ?? config.target) + const loaded = await openSchemaDir(flags['source-dir'], config.environments[targetName].dbUrl) + this.log(` ${dim(`Loaded ${loaded.statements} statement(s) from ${loaded.files} file(s) in ${flags['source-dir']}.`)}\n`) + config.environments = { ...config.environments, [SCHEMA_FILES_ENV]: { dbUrl: loaded.url } } + // What the platform owns and the copy could not hold is not the files'. + config.ignoreSchemas = [...(config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS), ...loaded.unavailable.schemas] + config.ignoreExtensions = [...(config.ignoreExtensions ?? []), ...loaded.unavailable.extensions] + config.source = SCHEMA_FILES_ENV + config.target = targetName + try { + await this.compare(flags, config, flags.check ? undefined : SCHEMA_FILE_CHECKS) + } finally { + await loaded.close() + } + return + } + this.validateDualEnvConfig(config, flags.source, flags.target) + await this.compare(flags, config) + } + + /** The comparison itself, between the config's source and target. */ + private async compare( + flags: Interfaces.InferredFlags, + config: SupaForgeConfig, + defaultChecks?: CheckName[], + ): Promise { const registry = createDefaultRegistry({ includeFiles: flags['include-files'] }) - const checks = flags.check ? [flags.check as CheckName] : undefined + const checks = flags.check ? [flags.check as CheckName] : defaultChecks const skip = flags.skip?.length ? (flags.skip as CheckName[]) : undefined const tableFilter = resolveTableFilter(config, { @@ -466,6 +509,15 @@ export default class Diff extends BaseCommand { this.log(` ${ok('Converged')} — the migration reproduces the source exactly.\n`) } } + if (proof.data && !proof.skipped) { + const { sampled, unchecked, rewritten } = proof.data + if (sampled.length > 0) this.log(` ${ok('Rows kept')} in the ${sampled.length} table(s) sampled.`) + if (rewritten.length > 0) { + this.log(` ${warn('Rewritten')} on the copy — a long lock on the real target: ${rewritten.join(', ')}`) + } + if (unchecked.length > 0) this.log(` ${dim(`Rows not checked (the sample would not load): ${unchecked.join(', ')}`)}`) + this.log('') + } } const result = await promote({ diff --git a/packages/cli/src/commands/migrate/create.ts b/packages/cli/src/commands/migrate/create.ts index b26a7e0..1e85d60 100644 --- a/packages/cli/src/commands/migrate/create.ts +++ b/packages/cli/src/commands/migrate/create.ts @@ -8,6 +8,7 @@ import { ok, warn, dim, bold, cmd } from '../../ui.js' import { splitSqlStatements } from '../../utils/sql-split.js' import { truncateSql } from './run.js' import { slugify } from '../../utils/strings.js' +import { targetFingerprint, writtenAgainstLine } from '../../written-against.js' /** * Generate a new migration file from the current schema drift. @@ -78,10 +79,13 @@ export default class MigrateCreate extends BaseCommand { const destructive = splitSqlStatements(result.up).filter(isDestructiveSql) // Build file content with UP and optional DOWN + // The target as the file was written against it, for migrate run to check. + const digest = await targetFingerprint(targetEnv.dbUrl, ignoreSchemas).catch(() => undefined) const lines: string[] = [ `-- Migration: ${flags.name}`, `-- Generated by supaforge migrate create`, `-- Source: ${config.source!} → Target: ${config.target!}`, + ...(digest ? [writtenAgainstLine(config.target!, digest)] : []), ...(destructive.length > 0 ? [`-- WARNING: ${destructive.length} destructive statement(s) — drops or deletes data. Review before running.`] : []), diff --git a/packages/cli/src/commands/migrate/run.ts b/packages/cli/src/commands/migrate/run.ts index 360eead..f1f7976 100644 --- a/packages/cli/src/commands/migrate/run.ts +++ b/packages/cli/src/commands/migrate/run.ts @@ -11,6 +11,8 @@ import { isDestructiveSql } from '../../dbdiff.js' import { splitSqlStatements } from '../../utils/sql-split.js' import { readFile } from 'node:fs/promises' import { errMsg } from '../../utils/error.js' +import { DEFAULT_IGNORE_SCHEMAS } from '../../defaults.js' +import { targetFingerprint, writtenAgainst } from '../../written-against.js' /** * Execute pending migrations against a Supabase environment. @@ -56,6 +58,10 @@ export default class MigrateRun extends BaseCommand { description: 'Permit migrations that drop tables or columns, delete rows, or drop policies', default: false, }), + force: Flags.boolean({ + description: 'Run a migration even though the target has changed since it was written against it', + default: false, + }), } async run(): Promise { @@ -116,6 +122,19 @@ export default class MigrateRun extends BaseCommand { } } + // The first pending migration written against this environment is + // checked against the target as it is now; the ones after it follow it. + const stale = await this.changedSinceWritten(pending, envName, env.dbUrl, config.ignoreSchemas ?? DEFAULT_IGNORE_SCHEMAS) + if (stale) { + this.log('') + this.log(warn(`${stale} was written against ${envName} as it was then, and ${envName} has changed since.`)) + this.log(` ${dim('Re-create it with migrate create, or pass --force to run it anyway.')}`) + if (flags.apply && !flags.force) { + this.log(`\n${warn('Nothing was applied.')}`) + this.exit(1) + } + } + if (!flags.apply) { this.log(`\n${dim('Preview only — nothing was applied.')} Re-run with ${bold('--apply')} to execute` + (destructive.length > 0 ? `, and ${bold('--allow-destructive')} for the statements above.` : '.')) @@ -145,6 +164,18 @@ export default class MigrateRun extends BaseCommand { this.log(`\n${ok(`Applied ${applied_count} migration(s) successfully.`)} ✓`) } + + /** The first pending migration written against `envName`, if the target has changed since. */ + private async changedSinceWritten( + pending: Array<{ filename: string; path: string }>, envName: string, dbUrl: string, ignoreSchemas: readonly string[], + ): Promise { + const [first] = pending + if (!first) return undefined + const header = writtenAgainst(await readFile(first.path, 'utf-8').catch(() => '')) + if (!header || header.env !== envName) return undefined + const now = await targetFingerprint(dbUrl, ignoreSchemas).catch(() => header.digest) + return now === header.digest ? undefined : first.filename + } } /** diff --git a/packages/cli/src/dbdiff.ts b/packages/cli/src/dbdiff.ts index 0508330..fd24f8e 100644 --- a/packages/cli/src/dbdiff.ts +++ b/packages/cli/src/dbdiff.ts @@ -45,6 +45,21 @@ export interface DbDiffOptions { export interface DbDiffResult { up: string down: string + /** + * What dbdiff warned about without changing anything: kinds of object it + * does not compare, columns or tables it would not take for renames. + */ + notices?: string[] +} + +/** dbdiff's warnings (`⚠ ...` lines), colour codes stripped. */ +export function dbdiffNotices(stdout: string): string[] { + return stdout + .replace(/\u001b\[[0-9;]*m/g, '') + .split('\n') + .map(line => line.trim()) + .filter(line => line.startsWith('⚠')) + .map(line => line.replace(/^⚠\s*/, '')) } /** @@ -285,16 +300,17 @@ export async function runDbDiff(options: DbDiffOptions): Promise { // so timeout, maxBuffer and every error path stay exactly as they were. attachProgress(running, options.onProgress) - await running + const { stdout } = await running + const notices = dbdiffNotices(String(stdout ?? '')) // When schemas are identical, dbdiff exits 0 but doesn't write the file const fileExists = await access(outputFile).then(() => true, () => false) if (!fileExists) { - return { up: '', down: '' } + return { up: '', down: '', notices } } const output = await readFile(outputFile, 'utf8') - return parseDbDiffOutput(output) + return { ...parseDbDiffOutput(output), notices } } catch (err: unknown) { const errObj = (err ?? {}) as Record const message = errMsg(err) @@ -408,6 +424,11 @@ export function destructiveReason( sql: string, recreatedElsewhere: ReadonlySet = new Set(), ): string | undefined { + // A queue's messages are rows, and pgmq.drop_queue drops its tables. A + // function call, so neither reading below would see it as a DROP. + if (/\bPGMQ\s*\.\s*"?DROP_QUEUE"?\s*\(/.test(sqlSkeleton(sql).toUpperCase())) { + return 'drops a queue and the messages in it' + } const statements = parseStatements(sql) return statements ? destructiveInTree(statements, recreatedElsewhere) @@ -567,6 +588,25 @@ export function sqlToIssues( return check === 'schema' ? foldCommentsIntoCreates(issues) : issues } +/** Kinds dbdiff does not compare that another check does: publications, the Realtime check. */ +const COMPARED_ELSEWHERE = /^Publication objects are not compared/ + +/** + * dbdiff's warnings, as said beside the schema check's findings: what it + * did not compare, or would not take for a rename, is worth knowing before + * an apply rather than after. `Operator objects are not compared (…) — check + * them by hand` reads `Not compared: Operator (…) — check them by hand.` + */ +export function schemaNotices(notices: string[]): string[] { + return notices + .filter(n => !COMPARED_ELSEWHERE.test(n)) + .map(notice => { + const [what, advice] = notice.split(' — ') + const title = /not compared/.test(what) ? `Not compared: ${what.replace(/ objects are not compared/, '')}` : what + return advice ? `${title} — ${advice}.` : `${title}.` + }) +} + /** What a comment title names: `public.orders.total (column)` → the name and kind. */ const COMMENT_TITLE = /^(?:Comment changed|Comment missing): (?:schema (.+)|(.+) \(([a-z ]+)\))$/ /** @@ -758,6 +798,19 @@ function issuesFromUnits( } } + const modified = modifiedTitle(up, statements) + if (modified) { + return { + id: `${check}-alter-${up.kind.replace(/^Alter/, '').toLowerCase()}-${i + 1}`, + check, + severity: 'warning' as const, + title: modified, + description: 'Definition differs between source and target.', + sql: { up: upSql, down: downSql }, + ...manual, + } + } + const lead = leadStatement(up, statements) const type = classifyStatement(lead) return { @@ -845,6 +898,36 @@ function replacedView(unit: DbDiffUnit, statements: string[]): { kind: Replaceab /** A name without its schema or quotes, for matching one to dbdiff's unit object. */ const bareName = (name: string) => name.slice(name.lastIndexOf('.') + 1).replace(/"/g, '').toLowerCase() +/** Changed objects dbdiff replaces, or changes where they stand, by unit kind. */ +const MODIFIED_KINDS: Record = { + AlterRangeType: 'Type modified', + AlterAggregate: 'Aggregate modified', + AlterRule: 'Rule modified', + AlterStatistics: 'Statistics modified', + AlterEventTrigger: 'Event trigger modified', + AlterCast: 'Cast modified', + AlterTextSearchConfiguration: 'Text search configuration modified', + AlterTextSearchDictionary: 'Text search dictionary modified', + AlterForeignServer: 'Foreign server modified', + AlterUserMapping: 'User mapping modified', + AlterForeignTable: 'Foreign table modified', + AlterDefaultPrivileges: 'Default privileges differ', +} + +/** + * " modified: " for a changed object of the kinds above, named + * from the statement that makes it — its DROP would read as the object + * being removed. + */ +function modifiedTitle(unit: DbDiffUnit, statements: string[]): string | undefined { + const label = MODIFIED_KINDS[unit.kind] + if (!label) return undefined + const making = statements.find(sql => !/^\s*DROP\b/i.test(sql)) ?? statements[0] + const summary = making ? summariseStatement(making, 'schema') : '' + const name = summary.slice(summary.indexOf(': ') + 2) + return `${label}: ${name || unit.object}` +} + /** * The statement a unit is named by: for a table change, its first ALTER * TABLE (a column type change opens by standing views aside; a serial column @@ -1358,6 +1441,8 @@ interface SummaryRule { match: RegExp label: string name: (sql: string, downSql?: string) => string + /** The name is not a schema object's — a role's privileges, a cast — so no schema is added. */ + unqualified?: boolean } /** Head patterns for the identifier that follows each keyword. */ @@ -1371,12 +1456,58 @@ const AFTER = { index: /\bINDEX\s+(?:CONCURRENTLY\s+)?(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, policy: /\bPOLICY\s+(?:IF\s+EXISTS\s+)?/i, collation: /\bCOLLATION\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + eventTrigger: /\bEVENT\s+TRIGGER\s+(?:IF\s+EXISTS\s+)?/i, + aggregate: /\bAGGREGATE\s+(?:IF\s+EXISTS\s+)?/i, + statistics: /\bSTATISTICS\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + textSearch: /\bTEXT\s+SEARCH\s+(?:CONFIGURATION|DICTIONARY)\s+(?:IF\s+EXISTS\s+)?/i, + server: /\bSERVER\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, + foreignTable: /\bFOREIGN\s+TABLE\s+(?:IF\s+(?:NOT\s+)?EXISTS\s+)?/i, on: /\bON\s+(?:ONLY\s+)?/i, } /** Name an object by the identifier following `head`. */ const named = (head: RegExp) => (sql: string) => extractQualifiedName(sql, head) +/** + * `public.people.name → full_name`, `public.clients → customers`: what a + * rename statement renames, and to what. + */ +function renameLabel(sql: string): string { + const table = extractQualifiedName(sql, /\b(?:TABLE|INDEX)\s+(?:IF\s+EXISTS\s+)?(?:ONLY\s+)?/i) + const to = /\bTO\s+("(?:[^"]|"")*"|[\w$]+)\s*;?\s*$/i.exec(sql.trim())?.[1] + const from = /\bRENAME\s+(?:COLUMN|CONSTRAINT)\s+("(?:[^"]|"")*"|[\w$]+)/i.exec(sql)?.[1] + return `${qualifySchemaName(table)}${from ? '.' + unquoted(from) : ''} → ${to ? unquoted(to) : UNKNOWN_NAME}` +} + +/** `role in schema on tables` for an ALTER DEFAULT PRIVILEGES. */ +function defaultPrivilegesLabel(sql: string): string { + const role = /\bFOR\s+ROLE\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] + const schema = /\bIN\s+SCHEMA\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] + const kind = /\bON\s+(TABLES|SEQUENCES|FUNCTIONS|ROUTINES|TYPES|SCHEMAS)\b/i.exec(sql)?.[1] + return [role && unquoted(role), schema ? `in ${unquoted(schema)}` : 'in every schema', kind && `on ${kind.toLowerCase()}`] + .filter(Boolean).join(' ') +} + +/** `table.rule` for a rule statement. */ +function ruleLabel(sql: string): string { + const rule = extractQualifiedName(sql, /\bRULE\s+(?:IF\s+EXISTS\s+)?/i) + const table = /\bTO\s+((?:"(?:[^"]|"")*"|[\w$]+)(?:\.(?:"(?:[^"]|"")*"|[\w$]+))?)/i.exec(sql)?.[1] + ?? /\bON\s+((?:"(?:[^"]|"")*"|[\w$]+)(?:\.(?:"(?:[^"]|"")*"|[\w$]+))?)/i.exec(sql)?.[1] + return table ? `${unquoted(table.split('.').pop() ?? table)}.${rule.split('.').pop()}` : rule +} + +/** `(a AS b)` for a cast statement. */ +function castLabel(sql: string): string { + return /\(([^()]+?\s+AS\s+[^()]+?)\)/i.exec(sql)?.[1] ?? UNKNOWN_NAME +} + +/** `user@server` for a user mapping statement. */ +function userMappingLabel(sql: string): string { + const user = /\bFOR\s+("(?:[^"]|"")*"|\S+)/i.exec(sql)?.[1] ?? 'unknown' + const server = /\bSERVER\s+("(?:[^"]|"")*"|[^\s;]+)/i.exec(sql)?.[1] ?? 'unknown' + return `${unquoted(user)}@${unquoted(server)}` +} + /** * `table.policy` for a policy statement. * @@ -1488,6 +1619,10 @@ const SCHEMA_RULES: SummaryRule[] = [ { match: /^\s*ALTER\s+DOMAIN\b/i, label: 'Domain altered', name: named(AFTER.domain) }, { match: /^\s*DROP\s+DOMAIN\b/i, label: 'Extra domain', name: named(AFTER.domain) }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+COLUMN\b/is, label: 'Column renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+CONSTRAINT\b/is, label: 'Constraint renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+TABLE\b[^;]*\bRENAME\s+TO\b/is, label: 'Table renamed', name: renameLabel, unqualified: true }, + { match: /^\s*ALTER\s+INDEX\b[^;]*\bRENAME\s+TO\b/is, label: 'Index renamed', name: renameLabel, unqualified: true }, { match: /^\s*ALTER\s+TABLE\b/i, label: 'Table altered', name: named(AFTER.table) }, { match: /^\s*CREATE\s+TABLE\b/i, label: 'Table missing', name: named(AFTER.table) }, { match: /^\s*DROP\s+TABLE\b/i, label: 'Extra table', name: named(AFTER.table) }, @@ -1514,6 +1649,29 @@ const SCHEMA_RULES: SummaryRule[] = [ { match: /^\s*ALTER\s+COLLATION\b/i, label: 'Collation altered', name: named(AFTER.collation) }, { match: /^\s*DROP\s+COLLATION\b/i, label: 'Extra collation', name: named(AFTER.collation) }, + { match: /^\s*ALTER\s+DEFAULT\s+PRIVILEGES\b/i, label: 'Default privileges differ', name: defaultPrivilegesLabel, unqualified: true }, + { match: /^\s*CREATE\s+EVENT\s+TRIGGER\b/i, label: 'Event trigger missing', name: named(AFTER.eventTrigger) }, + { match: /^\s*DROP\s+EVENT\s+TRIGGER\b/i, label: 'Extra event trigger', name: named(AFTER.eventTrigger) }, + { match: /^\s*CREATE\s+(?:OR\s+REPLACE\s+)?RULE\b/i, label: 'Rule missing', name: ruleLabel }, + { match: /^\s*DROP\s+RULE\b/i, label: 'Extra rule', name: ruleLabel }, + { match: /^\s*CREATE\s+(?:OR\s+REPLACE\s+)?AGGREGATE\b/i, label: 'Aggregate missing', name: named(AFTER.aggregate) }, + { match: /^\s*DROP\s+AGGREGATE\b/i, label: 'Extra aggregate', name: named(AFTER.aggregate) }, + { match: /^\s*CREATE\s+STATISTICS\b/i, label: 'Statistics missing', name: named(AFTER.statistics) }, + { match: /^\s*DROP\s+STATISTICS\b/i, label: 'Extra statistics', name: named(AFTER.statistics) }, + { match: /^\s*(?:CREATE|ALTER)\s+TEXT\s+SEARCH\s+CONFIGURATION\b/i, label: 'Text search configuration missing', name: named(AFTER.textSearch) }, + { match: /^\s*DROP\s+TEXT\s+SEARCH\s+CONFIGURATION\b/i, label: 'Extra text search configuration', name: named(AFTER.textSearch) }, + { match: /^\s*CREATE\s+TEXT\s+SEARCH\s+DICTIONARY\b/i, label: 'Text search dictionary missing', name: named(AFTER.textSearch) }, + { match: /^\s*DROP\s+TEXT\s+SEARCH\s+DICTIONARY\b/i, label: 'Extra text search dictionary', name: named(AFTER.textSearch) }, + { match: /^\s*CREATE\s+CAST\b/i, label: 'Cast missing', name: castLabel, unqualified: true }, + { match: /^\s*DROP\s+CAST\b/i, label: 'Extra cast', name: castLabel, unqualified: true }, + { match: /^\s*CREATE\s+SERVER\b/i, label: 'Foreign server missing', name: named(AFTER.server) }, + { match: /^\s*ALTER\s+SERVER\b/i, label: 'Foreign server altered', name: named(AFTER.server) }, + { match: /^\s*DROP\s+SERVER\b/i, label: 'Extra foreign server', name: named(AFTER.server) }, + { match: /^\s*(?:CREATE|ALTER)\s+USER\s+MAPPING\b/i, label: 'User mapping missing', name: userMappingLabel, unqualified: true }, + { match: /^\s*DROP\s+USER\s+MAPPING\b/i, label: 'Extra user mapping', name: userMappingLabel, unqualified: true }, + { match: /^\s*CREATE\s+FOREIGN\s+TABLE\b/i, label: 'Foreign table missing', name: named(AFTER.foreignTable) }, + { match: /^\s*DROP\s+FOREIGN\s+TABLE\b/i, label: 'Extra foreign table', name: named(AFTER.foreignTable) }, + { match: /^\s*CREATE\s+SEQUENCE\b/i, label: 'Sequence missing', name: named(AFTER.sequence) }, { match: /^\s*ALTER\s+SEQUENCE\b/i, label: 'Sequence altered', name: named(AFTER.sequence) }, { match: /^\s*DROP\s+SEQUENCE\b/i, label: 'Extra sequence', name: named(AFTER.sequence) }, @@ -1592,7 +1750,8 @@ export function summariseStatement(sql: string, check: 'schema' | 'data', downSq for (const rule of SCHEMA_RULES) { if (rule.match.test(sql)) { const label = rule.name === commentLabel ? commentChange(sql, downSql) : rule.label - return `${label}: ${qualifySchemaName(rule.name(sql, downSql))}` + const name = rule.name(sql, downSql) + return `${label}: ${rule.unqualified ? name : qualifySchemaName(name)}` } } diff --git a/packages/cli/src/hazards.ts b/packages/cli/src/hazards.ts new file mode 100644 index 0000000..fb62d20 --- /dev/null +++ b/packages/cli/src/hazards.ts @@ -0,0 +1,131 @@ +import { destructiveReason } from './dbdiff.js' +import { parseStatements, relationName, type Statement } from './sql-ast.js' + +/** + * What applying a fix does to a live database, beyond what it changes. + * + * A fix can be correct and still take production down: an ALTER TABLE holds + * a lock that stops every read and write while it runs, a column type change + * may rewrite the whole table under it, a new CHECK scans every row. None of + * that showed in a report, so a one-line fix and an hour-long outage looked + * the same. Read from PostgreSQL's own parse of the fix, per statement, from + * the lock levels PostgreSQL documents. A table the same fix creates has no + * one waiting on it, so it is not flagged. + */ +export type HazardKind = 'data-loss' | 'blocks-reads-and-writes' | 'blocks-writes' | 'rewrite' | 'full-scan' | 'not-transactional' + +export interface Hazard { + kind: HazardKind + /** The table it concerns, where there is one. */ + on?: string + /** For a person. */ + detail: string +} + +/** Defaults whose value differs per row, so adding the column writes every row. */ +const VOLATILE = new Set(['random', 'gen_random_uuid', 'uuid_generate_v4', 'uuid_generate_v1', 'clock_timestamp', 'nextval', 'timeofday']) + +const table = (rv: Record | undefined) => + rv ? (rv.schemaname ? `${rv.schemaname}.${relationName(rv)}` : relationName(rv)) : '' + +export function hazardsOf(sql: string): Hazard[] { + const out: Hazard[] = [] + const add = (h: Hazard) => { + if (!out.some(o => o.kind === h.kind && o.on === h.on)) out.push(h) + } + const loss = destructiveReason(sql) + if (loss) add({ kind: 'data-loss', detail: loss }) + + const statements = parseStatements(sql) + if (!statements) return out + const created = new Set(statements.filter(s => s.kind === 'CreateStmt').map(s => table(s.node.relation))) + const existing = (name: string) => name !== '' && !created.has(name) + + for (const s of statements) { + for (const h of statementHazards(s)) { + if (h.on === undefined || existing(h.on)) add(h) + } + } + return out +} + +function statementHazards(s: Statement): Hazard[] { + const n = s.node + switch (s.kind) { + case 'AlterTableStmt': return alterTableHazards(table(n.relation), n.cmds ?? []) + case 'IndexStmt': + return n.concurrent + ? [{ kind: 'not-transactional', on: table(n.relation), detail: `builds an index concurrently, which cannot run inside a transaction` }] + : [{ kind: 'blocks-writes', on: table(n.relation), detail: `blocks writes to ${table(n.relation)} while the index builds` }] + case 'CreateTrigStmt': + return [{ kind: 'blocks-writes', on: table(n.relation), detail: `blocks writes to ${table(n.relation)} briefly` }] + case 'CreatePolicyStmt': + case 'AlterPolicyStmt': + case 'RuleStmt': + return [{ kind: 'blocks-reads-and-writes', on: table(n.table ?? n.relation), detail: `blocks reads and writes on ${table(n.table ?? n.relation)} briefly` }] + case 'RenameStmt': + return n.relation ? [{ kind: 'blocks-reads-and-writes', on: table(n.relation), detail: `blocks reads and writes on ${table(n.relation)} briefly` }] : [] + case 'RefreshMatViewStmt': + return n.concurrent ? [] : [{ kind: 'blocks-reads-and-writes', on: table(n.relation), detail: `blocks reads of ${table(n.relation)} while it refreshes` }] + case 'VacuumStmt': + return [{ kind: 'not-transactional', detail: 'VACUUM cannot run inside a transaction' }] + case 'ReindexStmt': + return (n.params ?? []).some((p: any) => p.DefElem?.defname === 'concurrently') + ? [{ kind: 'not-transactional', detail: 'REINDEX CONCURRENTLY cannot run inside a transaction' }] : [] + default: + return [] + } +} + +function alterTableHazards(name: string, cmds: Array>): Hazard[] { + const out: Hazard[] = [] + let exclusive = false + let writesOnly = false + for (const { AlterTableCmd: cmd } of cmds) { + if (!cmd) continue + switch (cmd.subtype) { + // Lighter locks: other DDL waits, reads and writes go on. + case 'AT_SetStatistics': case 'AT_SetOptions': case 'AT_ResetOptions': case 'AT_SetRelOptions': + case 'AT_ResetRelOptions': case 'AT_ValidateConstraint': case 'AT_ClusterOn': case 'AT_DropCluster': + continue + case 'AT_AlterColumnType': + out.push({ kind: 'rewrite', on: name, detail: `may rewrite ${name}: a column's type changes, unless the new type is binary-compatible` }) + break + case 'AT_SetLogged': case 'AT_SetUnLogged': + out.push({ kind: 'rewrite', on: name, detail: `rewrites ${name}: LOGGED and UNLOGGED tables are stored differently` }) + break + case 'AT_AddColumn': { + const def = cmd.def?.ColumnDef + const constraints: any[] = def?.constraints ?? [] + const generated = constraints.some(c => c.Constraint?.contype === 'CONSTR_GENERATED') + const volatile = constraints.some(c => c.Constraint?.contype === 'CONSTR_DEFAULT' + && VOLATILE.has(String(c.Constraint.raw_expr?.FuncCall?.funcname?.at(-1)?.String?.sval ?? ''))) + if (generated || volatile) { + out.push({ kind: 'rewrite', on: name, detail: `rewrites ${name}: the new column's ${generated ? 'generated value' : 'default'} is computed for every row` }) + } + break + } + case 'AT_AddConstraint': { + const c = cmd.def?.Constraint + if (c && !c.skip_validation && (c.contype === 'CONSTR_CHECK' || c.contype === 'CONSTR_FOREIGN')) { + out.push({ kind: 'full-scan', on: name, detail: `scans every row of ${name} to check the new constraint (NOT VALID, then VALIDATE, avoids holding the lock)` }) + } + if (c?.contype === 'CONSTR_FOREIGN') { + writesOnly = true + continue + } + break + } + case 'AT_SetNotNull': + out.push({ kind: 'full-scan', on: name, detail: `scans every row of ${name} to check for NULLs` }) + break + } + exclusive = true + } + if (exclusive) { + out.unshift({ kind: 'blocks-reads-and-writes', on: name, detail: `blocks reads and writes on ${name} while it runs` }) + } else if (writesOnly) { + out.unshift({ kind: 'blocks-writes', on: name, detail: `blocks writes to ${name} while the key is checked` }) + } + return out +} diff --git a/packages/cli/src/prove-data.ts b/packages/cli/src/prove-data.ts new file mode 100644 index 0000000..57d06e2 --- /dev/null +++ b/packages/cli/src/prove-data.ts @@ -0,0 +1,170 @@ +import { spawn } from 'node:child_process' +import { pgQuery } from './db' +import { parseStatements, relationName } from './sql-ast.js' +import { quoteIdent, quoteLiteral } from './utils/sql.js' + +/** + * The data half of a proof: that the rows survive the fixes. + * + * A schema can converge and the data not: a column dropped and added again + * under a new name, a table recreated, a type changed with a lossy cast. The + * proof's copy held no rows, so none of that showed. Now a bounded sample of + * the target's rows — the first `rows` of each table — is copied into it + * first. After the fixes, a table they keep must hold as many rows, and the + * same ones where its columns are unchanged — unless a fix deletes from or + * truncates it. A table rewritten on the copy (its storage replaced) is said + * so too: on the real target that is a long lock. + */ +export interface TableSnapshot { + rows: number + /** Per column, by name: its type and a hash of its values (as a multiset). */ + columns: Record + filenode: string +} + +export interface DataProof { + /** Tables sampled, and those whose sample could not be loaded (left unchecked). */ + sampled: string[] + unchecked: string[] + /** Kept tables that lost rows or whose rows changed, and how. */ + lost: string[] + /** Tables whose storage the fixes replaced on the copy. */ + rewritten: string[] +} + +export const DEFAULT_SAMPLE_ROWS = 1000 + +/** How many rows of each table to sample, from SUPAFORGE_PROVE_ROWS; 0 turns it off. */ +export function sampleRows(): number { + const raw = process.env.SUPAFORGE_PROVE_ROWS + const n = raw === undefined ? DEFAULT_SAMPLE_ROWS : Number(raw) + return Number.isInteger(n) && n >= 0 ? n : DEFAULT_SAMPLE_ROWS +} + +const TABLES_SQL = ` + SELECT n.nspname || '.' || c.relname AS name, quote_ident(n.nspname) || '.' || quote_ident(c.relname) AS ident, + (SELECT string_agg(quote_ident(a.attname), ', ' ORDER BY a.attnum) FROM pg_attribute a + WHERE a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped AND a.attgenerated = '') AS columns + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE c.relkind = 'r' AND NOT c.relispartition AND n.nspname = ANY($1::text[]) + ORDER BY 1` + +/** Copy up to `limit` rows of each table from `fromUrl` into the same tables at `intoUrl`. */ +export async function sampleInto(psql: string, fromUrl: string, intoUrl: string, schemas: string[], limit: number): Promise<{ sampled: string[]; unchecked: string[] }> { + const tables = await pgQuery(fromUrl, TABLES_SQL, [schemas]) as Array<{ name: string; ident: string; columns: string | null }> + const sampled: string[] = [] + const unchecked: string[] = [] + for (const t of tables) { + if (!t.columns) continue + try { + await pipeCopy(psql, fromUrl, intoUrl, + `\\copy (SELECT ${t.columns} FROM ${t.ident} LIMIT ${limit}) TO STDOUT`, + `\\copy ${t.ident} (${t.columns}) FROM STDIN`) + sampled.push(t.name) + } catch { + // A sample that will not load — a row whose foreign key points at one + // not sampled, where triggers cannot be set aside — is left unchecked. + unchecked.push(t.name) + } + } + return { sampled, unchecked } +} + +function pipeCopy(psql: string, fromUrl: string, intoUrl: string, out: string, into: string): Promise { + return new Promise((resolve, reject) => { + const reader = spawn(psql, [fromUrl, '-X', '-q', '-v', 'ON_ERROR_STOP=1', '-c', out], { stdio: ['ignore', 'pipe', 'pipe'] }) + const writer = spawn(psql, [intoUrl, '-X', '-q', '-v', 'ON_ERROR_STOP=1', '-f', '-'], { stdio: ['pipe', 'pipe', 'pipe'] }) + let stderr = '' + reader.stderr.on('data', (d: Buffer) => { stderr += d.toString() }) + writer.stderr.on('data', (d: Buffer) => { stderr += d.toString() }) + reader.on('error', reject) + writer.on('error', reject) + writer.stdin.on('error', () => undefined) + // Foreign keys and triggers set aside where the role may (a superuser), + // so a sampled row need not find its parent sampled too; then the COPY, + // whose rows follow on the same stream until it ends. + writer.stdin.write(`\\set ON_ERROR_STOP 0\nSET session_replication_role = replica;\n\\set ON_ERROR_STOP 1\n${into}\n`) + reader.stdout.pipe(writer.stdin) + let done = 0 + let failed = false + const finish = (code: number | null) => { + if (code !== 0) failed = true + if (++done === 2) failed ? reject(new Error(stderr.trim() || 'copy failed')) : resolve() + } + reader.on('close', finish) + writer.on('close', finish) + }) +} + +/** + * Each table's row count, a hash of each column's values, and its storage. + * A generated column is left out: its values follow its expression, which a + * fix may mean to change. + */ +export async function snapshotTables(dbUrl: string, schemas: string[]): Promise> { + const rows = await pgQuery(dbUrl, ` + SELECT n.nspname || '.' || c.relname AS name, quote_ident(n.nspname) || '.' || quote_ident(c.relname) AS ident, + json_agg(json_build_array(a.attname, format_type(a.atttypid, a.atttypmod)) ORDER BY a.attnum) AS columns + FROM pg_class c JOIN pg_namespace n ON n.oid = c.relnamespace + JOIN pg_attribute a ON a.attrelid = c.oid AND a.attnum > 0 AND NOT a.attisdropped AND a.attgenerated = '' + WHERE c.relkind = 'r' AND NOT c.relispartition AND n.nspname = ANY($1::text[]) + GROUP BY 1, 2 ORDER BY 1`, [schemas]) as Array<{ name: string; ident: string; columns: Array<[string, string]> }> + const out = new Map() + for (const t of rows) { + // Each value hashed, a NULL as 'n' (no hash is one letter long). + const hashes = t.columns.map(([col], i) => { + const value = `coalesce(md5(x.${quoteIdent(col)}::text), 'n')` + return `coalesce(md5(string_agg(${value}, '' ORDER BY ${value})), '') AS h${i}` + }) + const [row] = await pgQuery(dbUrl, + `SELECT count(*)::text AS rows, pg_relation_filenode(${quoteLiteral(t.ident)}::regclass)::text AS filenode${hashes.length ? ', ' + hashes.join(', ') : ''} + FROM ${t.ident} x`) as Array> + out.set(t.name, { + rows: Number(row.rows), + filenode: row.filenode, + columns: Object.fromEntries(t.columns.map(([col, type], i) => [col, { type, hash: row[`h${i}`] }])), + }) + } + return out +} + +/** The tables a fix deletes from or empties, whose rows are meant to go. */ +export function emptiedTables(sql: string): Set { + const out = new Set() + for (const s of parseStatements(sql) ?? []) { + if (s.kind === 'DeleteStmt') out.add(relationName(s.node.relation)) + if (s.kind === 'TruncateStmt') for (const r of s.node.relations ?? []) out.add(relationName(r.RangeVar)) + } + return out +} + +/** + * What became of the sampled rows: fewer of them in a table the fixes keep, + * or, in a column that is there before and after with the same type, + * different values. + */ +export function compareTables(before: Map, after: Map, sampled: string[], migrationSql: string): Pick { + const emptied = emptiedTables(migrationSql) + const lost: string[] = [] + const rewritten: string[] = [] + for (const name of sampled) { + const b = before.get(name) + const a = after.get(name) + if (!b || !a) continue + if (b.filenode !== a.filenode) rewritten.push(name) + if (emptied.has(name.slice(name.indexOf('.') + 1))) continue + if (a.rows < b.rows) { + lost.push(`${name}: ${b.rows} row(s) before, ${a.rows} after`) + continue + } + if (a.rows !== b.rows) continue + const changed = Object.entries(b.columns) + .filter(([col, c]) => a.columns[col] && a.columns[col].type === c.type && a.columns[col].hash !== c.hash) + .map(([col]) => col) + if (changed.length > 0) { + lost.push(`${name}: values changed in ${changed.join(', ')}`) + } + } + return { lost, rewritten } +} + diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index 11267a6..842d10b 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -34,6 +34,7 @@ import { splitSqlStatements, isCommentOnly } from './utils/sql-split.js' import { quoteName } from './utils/sql.js' import { globToRegExp } from './utils/strings.js' import { join, dirname } from 'node:path' +import { compareTables, sampleInto, sampleRows, snapshotTables, type DataProof } from './prove-data.js' const exec = promisify(execFile) @@ -87,10 +88,15 @@ export interface ProofResult { * outsideTheRun(). They do not count against convergence. */ outOfScope?: string[] + /** + * What became of a sample of the target's rows — see prove-data.ts. A + * kept table that lost rows fails the proof. + */ + data?: DataProof } /** Replace the database name in a libpq URL, keeping everything else. */ -function withDatabase(dbUrl: string, database: string): string { +export function withDatabase(dbUrl: string, database: string): string { const url = new URL(dbUrl) url.pathname = `/${database}` return url.toString() @@ -110,7 +116,7 @@ function withDatabase(dbUrl: string, database: string): string { * lives in one place that both projects depend on. */ -async function fingerprint(dbUrl: string, schemas: string[]): Promise { +export async function fingerprint(dbUrl: string, schemas: string[]): Promise { const rows = await pgQuery(dbUrl, fingerprintSql(schemas)) as unknown as Array<{ fingerprint: string | null }> @@ -278,6 +284,11 @@ export async function proveConvergence(opts: { const scoped = opts.checks !== undefined && !opts.checks.includes('schema') const before = scoped ? await schemaState(cloneUrl, schemas) : undefined + // A sample of the target's rows, to see that they survive. + const limit = sampleRows() + const sample = limit > 0 ? await sampleInto(psql, opts.targetUrl, cloneUrl, schemas, limit) : undefined + const tablesBefore = sample ? await snapshotTables(cloneUrl, schemas) : undefined + // The migration under test. let notApplied: ProofResult['notApplied'] if (opts.fixes) { @@ -297,7 +308,17 @@ export async function proveConvergence(opts: { } else { await runSql(psql, cloneUrl, opts.migrationSql) } - const reported = (r: ProofResult): ProofResult => (notApplied?.length ? { ...r, notApplied } : r) + let data: DataProof | undefined + if (sample && tablesBefore) { + data = { ...sample, ...compareTables(tablesBefore, await snapshotTables(cloneUrl, schemas), sample.sampled, opts.migrationSql) } + if (data.lost.length > 0) { + return { converged: false, cloneName, data, residual: data.lost.map(l => `rows lost in ${l}`) } + } + } + const reported = (r: ProofResult): ProofResult => { + const withData = data ? { ...r, data } : r + return notApplied?.length ? { ...withData, notApplied } : withData + } const [want, got] = await Promise.all([ fingerprint(opts.sourceUrl, schemas), @@ -353,7 +374,7 @@ export async function proveConvergence(opts: { } } -interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMajor: number } +export interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMajor: number } /** * Copy `fromUrl`'s structure in the given schemas into the empty database at @@ -367,7 +388,7 @@ interface PgTools { pgDump: string; psql: string; clientMajor: number; serverMaj * into a clone that deliberately has no such schema. `--prove` therefore * failed on every real Supabase project, whatever the client version. */ -async function copyStructure( +export async function copyStructure( fromUrl: string, intoUrl: string, schemas: string[], tools: PgTools, migratingFrom?: string, ): Promise { // Prepare the copy to receive the proved schemas, and nothing else. @@ -504,7 +525,7 @@ async function copyOfSource( } /** Drop a throwaway database, ending any session still attached first. */ -async function dropDatabase(adminUrl: string, name: string): Promise { +export async function dropDatabase(adminUrl: string, name: string): Promise { await pgQuery(adminUrl, `SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = '${name}' AND pid <> pg_backend_pid()`).catch(() => undefined) @@ -520,7 +541,7 @@ async function dropDatabase(adminUrl: string, name: string): Promise { * loudly instead of producing a partially-migrated clone that then reports * confusing residual drift. */ -function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { +export function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { const trimmed = sql.trim() if (!trimmed) return Promise.resolve() @@ -544,7 +565,7 @@ function runSql(psqlPath: string, dbUrl: string, sql: string): Promise { } /** psql ships alongside pg_dump; reuse the version-matched directory. */ -function psqlBeside(pgDumpPath: string): string { +export function psqlBeside(pgDumpPath: string): string { return pgDumpPath === 'pg_dump' ? 'psql' : join(dirname(pgDumpPath), 'psql') } @@ -688,6 +709,11 @@ export function explainStructureFailure( /** `DROP [IF EXISTS] ` and `DROP COLUMN [IF EXISTS] `, capturing the bare name. */ const DROPPED_NAME = /\bDROP\s+(?:TABLE|VIEW|MATERIALIZED\s+VIEW|TYPE|DOMAIN|SEQUENCE|FUNCTION|PROCEDURE|INDEX|COLUMN|CONSTRAINT|POLICY|TRIGGER)\s+(?:IF\s+EXISTS\s+)?(?:(?:"[^"]+"|[\w$]+)\s*\.\s*)?("[^"]+"|[\w$]+)/gi +/** `table.column` as written by format('%I.%I'), each part unquoted and lower-cased. */ +function identParts(qualified: string): string[] { + return [...qualified.matchAll(/"((?:[^"]|"")+)"|([^."]+)/g)].map(m => (m[1]?.replace(/""/g, '"') ?? m[2]).toLowerCase()) +} + /** `CREATE [OR REPLACE] [IF NOT EXISTS] `, capturing the bare name. */ const CREATED_NAME = /\bCREATE\s+(?:OR\s+REPLACE\s+)?(?:UNIQUE\s+)?(?:TABLE|VIEW|MATERIALIZED\s+VIEW|TYPE|DOMAIN|SEQUENCE|FUNCTION|PROCEDURE|INDEX|POLICY|TRIGGER|EXTENSION)\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:(?:"[^"]+"|[\w$]+)\s*\.\s*)?("[^"]+"|[\w$]+)/gi @@ -721,10 +747,12 @@ export function residualHeldBack( const heldBack: string[] = [] const unexplained: string[] = [] for (const line of residual) { - const subject = /^[a-z ]+?\s([^\s:]+(?:\s[^\s:]+)?):\s*unexpected$/i.exec(line)?.[1] + const [, subject, owner] = /^[a-z ]+?\s([^\s:]+(?:\s[^\s:]+)?):\s*unexpected(?: \(owned by (.+)\))?$/i.exec(line) ?? [] const name = subject?.split(/[.\s]/).pop()?.replace(/^"|"$/g, '').toLowerCase() + // A sequence a column owns goes with the column, or its table. + const owners = owner ? identParts(owner) : [] const missing = /^(.*):\s*missing$/i.exec(line)?.[1].toLowerCase() - if (name && dropped.has(name)) heldBack.push(line) + if ((name && dropped.has(name)) || owners.some(o => dropped.has(o))) heldBack.push(line) else if (missing && created.some(n => missing.endsWith(` ${n}`) || missing.endsWith(`.${n}`))) heldBack.push(line) else unexplained.push(line) } diff --git a/packages/cli/src/render.ts b/packages/cli/src/render.ts index 4dc2a7e..1aeb7ba 100644 --- a/packages/cli/src/render.ts +++ b/packages/cli/src/render.ts @@ -143,6 +143,7 @@ export function renderSummary(result: ScanResult): string { for (const lr of result.checks) { lines.push(formatCheckLine(lr)) + for (const notice of lr.notices ?? []) lines.push(` ${dim(`ⓘ ${notice}`)}`) } lines.push('') @@ -242,6 +243,9 @@ function formatIssue(issue: DriftIssue): string { const lines: string[] = [] lines.push(` ${severityIcon(issue.severity)} ${colorSeverity(issue.severity)} ${bold(issue.title)}`) lines.push(` ${dim(issue.description)}`) + for (const hazard of issue.hazards ?? []) { + lines.push(` ${warn(hazard.kind === 'data-loss' ? '⛔' : '⏳')} ${warn(hazard.detail)}`) + } if (issue.sql) { lines.push('') diff --git a/packages/cli/src/restore.ts b/packages/cli/src/restore.ts index a971dbb..cbb48eb 100644 --- a/packages/cli/src/restore.ts +++ b/packages/cli/src/restore.ts @@ -307,7 +307,7 @@ export async function restoreFromSnapshot(options: RestoreOptions): Promise { + if (!issue.sql?.up) return issue + const hazards = hazardsOf(issue.sql.up) + return hazards.length > 0 ? { ...issue, hazards } : issue + }) +} + async function runCheck( check: Check, ctx: CheckContext, @@ -90,12 +101,14 @@ async function runCheck( sourceDbUrl: string, ): Promise { const start = performance.now() + const notices: string[] = [] try { - const issues = await check.scan(ctx) + const issues = withHazards(await check.scan({ ...ctx, onNotice: n => notices.push(n) })) return { check: name, status: issues.length > 0 ? 'drifted' : 'clean', issues, + ...(notices.length > 0 ? { notices } : {}), durationMs: Math.round(performance.now() - start), } } catch (err) { diff --git a/packages/cli/src/schema-dir.ts b/packages/cli/src/schema-dir.ts new file mode 100644 index 0000000..b73faa6 --- /dev/null +++ b/packages/cli/src/schema-dir.ts @@ -0,0 +1,127 @@ +import { randomBytes } from 'node:crypto' +import { readdir, readFile } from 'node:fs/promises' +import { join, relative } from 'node:path' +import { pgQuery } from './db' +import { SUPABASE_PLATFORM_SCHEMAS } from './defaults' + +import { describeMissingPgDump, getServerMajorVersion, resolvePgDumpPath } from './pg-tools' +import { copyStructure, dropDatabase, psqlBeside, withDatabase } from './prove.js' +import { isCommentOnly, splitSqlStatements } from './utils/sql-split.js' +import { quoteName } from './utils/sql.js' +import { DiagnosticError, errMsg } from './utils/error.js' + +/** + * A directory of schema files — Supabase's `supabase/schemas` — as a + * database to compare against. + * + * Declarative schemas keep the desired state as SQL files. To compare them + * with a live target they are loaded, in a throwaway database on the + * target's own server (as `--prove` makes its copy), on top of the target's + * platform schemas — so a table referencing auth.users, or a policy calling + * auth.uid(), finds them — and that database is the source. Files load in + * name order; a statement that fails because something it needs comes later + * is tried again once the others have run, so the order the files are in + * matters as little as it can. + */ +export interface SchemaDirSource { + url: string + /** + * The target's extensions the copy could not install (pg_cron lives in one + * database per server), and the schemas holding their tables: schema files + * do not declare what the platform owns, so they are not compared. + */ + unavailable: { extensions: string[]; schemas: string[] } + /** Statements loaded, and how many files they came from. */ + statements: number + files: number + close(): Promise +} + +export async function openSchemaDir(dir: string, targetUrl: string): Promise { + const files = await sqlFiles(dir) + if (files.length === 0) throw new DiagnosticError(`--source-dir: no .sql files in ${dir}`) + + const serverMajor = await getServerMajorVersion(targetUrl) + const resolved = await resolvePgDumpPath(serverMajor) + if (!resolved) throw new DiagnosticError(`--source-dir needs pg_dump to copy the target's platform schemas: ${await describeMissingPgDump(serverMajor)}`) + const tools = { pgDump: resolved.path, psql: psqlBeside(resolved.path), clientMajor: resolved.major, serverMajor } + + const name = `supaforge_schema_${randomBytes(4).toString('hex')}` + const adminUrl = withDatabase(targetUrl, 'postgres') + const url = withDatabase(targetUrl, name) + try { + await pgQuery(adminUrl, `CREATE DATABASE ${quoteName(name)}`) + } catch (err) { + throw new DiagnosticError(`--source-dir: could not create a throwaway database on the target's server: ${errMsg(err)}`) + } + const close = () => dropDatabase(adminUrl, name) + try { + const present = new Set((await pgQuery(targetUrl, 'SELECT nspname FROM pg_namespace') as Array<{ nspname: string }>).map(r => r.nspname)) + const platform = SUPABASE_PLATFORM_SCHEMAS.filter(s => present.has(s) && s !== 'pg_catalog' && s !== 'information_schema') + if (platform.length > 0) await copyStructure(targetUrl, url, platform, tools) + const statements = await loadStatements(url, files) + return { url, statements, files: files.length, close, unavailable: await unavailableExtensions(targetUrl, url) } + } catch (err) { + await close() + throw err + } +} + +/** The directory's .sql files, at any depth, in name order. */ +async function sqlFiles(dir: string): Promise> { + const entries = await readdir(dir, { recursive: true, withFileTypes: true }) + return entries + .filter(e => e.isFile() && e.name.endsWith('.sql')) + .map(e => { + const path = join(e.parentPath ?? (e as unknown as { path: string }).path, e.name) + return { path, name: relative(dir, path) } + }) + .sort((a, b) => a.name.localeCompare(b.name)) +} + +/** + * Run every statement, each on its own; what fails is tried again after the + * rest, as long as each round gets further. What still fails is reported + * with its file. + */ +async function loadStatements(url: string, files: Array<{ path: string; name: string }>): Promise { + let pending: Array<{ file: string; sql: string; error?: string }> = [] + for (const f of files) { + for (const sql of splitSqlStatements(await readFile(f.path, 'utf8'))) { + if (!isCommentOnly(sql)) pending.push({ file: f.name, sql }) + } + } + const total = pending.length + for (;;) { + const failed: typeof pending = [] + for (const p of pending) { + try { + await pgQuery(url, p.sql) + } catch (err) { + failed.push({ ...p, error: errMsg(err) }) + } + } + if (failed.length === 0) return total + if (failed.length === pending.length) { + const first = failed[0] + throw new DiagnosticError( + `--source-dir: ${failed.length} statement(s) could not be loaded. First, in ${first.file}: ${first.error}\n` + + ` ${first.sql.slice(0, 200).replace(/\s+/g, ' ')}`, + ) + } + pending = failed + } +} + +/** The target's extensions the copy lacks, and the schemas their tables are in. */ +async function unavailableExtensions(targetUrl: string, copyUrl: string): Promise<{ extensions: string[]; schemas: string[] }> { + const sql = `SELECT e.extname AS name, + ARRAY(SELECT DISTINCT n.nspname FROM pg_depend d JOIN pg_class c ON c.oid = d.objid + JOIN pg_namespace n ON n.oid = c.relnamespace + WHERE d.classid = 'pg_class'::regclass AND d.refobjid = e.oid AND d.deptype = 'e')::text[] AS schemas + FROM pg_extension e` + const [target, copy] = await Promise.all([targetUrl, copyUrl].map(url => pgQuery(url, sql))) as Array> + const have = new Set(copy.map(e => e.name)) + const missing = target.filter(e => !have.has(e.name)) + return { extensions: missing.map(e => e.name), schemas: [...new Set(missing.flatMap(e => e.schemas))] } +} diff --git a/packages/cli/src/state-diff.ts b/packages/cli/src/state-diff.ts index 203a497..9cbdca5 100644 --- a/packages/cli/src/state-diff.ts +++ b/packages/cli/src/state-diff.ts @@ -54,6 +54,8 @@ function compareKeyed( after: T[] | null | undefined, keyOf: (item: T) => string, compare: (out: string[], subject: string, a: T, b: T) => void, + /** Said after `unexpected`, of an object only the second side has. */ + note: (item: T) => string = () => '', ): void { const a = new Map((before ?? []).map(x => [keyOf(x), x])) const b = new Map((after ?? []).map(x => [keyOf(x), x])) @@ -62,8 +64,8 @@ function compareKeyed( if (!b.has(key)) out.push(`${kind} ${key}: missing`) else compare(out, `${kind} ${key}`, item, b.get(key) as T) } - for (const key of b.keys()) { - if (!a.has(key)) out.push(`${kind} ${key}: unexpected`) + for (const [key, item] of b) { + if (!a.has(key)) out.push(`${kind} ${key}: unexpected${note(item)}`) } } @@ -150,7 +152,9 @@ export function diffState(before: SchemaState, after: SchemaState): string[] { for (const k of ['type', 'start', 'increment', 'min', 'max', 'cache', 'cycle', 'owned_by'] as const) { compareField(o, s, k.replace('_', ' '), a[k], b[k]) } - }) + }, + // A serial or identity column's: it goes with its table. + q => q.owned_by ? ` (owned by ${q.owned_by})` : '') compareKeyed(out, 'routine', before.routines, after.routines, r => `${qualify(r.schema, r.name)}(${r.arguments})`, diff --git a/packages/cli/src/types/config.ts b/packages/cli/src/types/config.ts index 33c2615..ca127b7 100644 --- a/packages/cli/src/types/config.ts +++ b/packages/cli/src/types/config.ts @@ -84,6 +84,8 @@ export interface SupaForgeConfig { source?: string target?: string ignoreSchemas?: string[] + /** Extensions the extensions check does not report as extra on the target. */ + ignoreExtensions?: string[] checks?: ChecksConfig } diff --git a/packages/cli/src/types/drift.ts b/packages/cli/src/types/drift.ts index 5c7cbf6..1a9a9e3 100644 --- a/packages/cli/src/types/drift.ts +++ b/packages/cli/src/types/drift.ts @@ -1,6 +1,6 @@ export const CHECK_NAMES = [ 'schema', 'rls', 'rls-coverage', 'edge-functions', 'storage', 'auth', 'cron', 'data', - 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', + 'webhooks', 'realtime', 'vault', 'extensions', 'migrations', 'roles', 'queues', 'partitions', ] as const export type CheckName = (typeof CHECK_NAMES)[number] @@ -22,13 +22,15 @@ export const CHECK_META: Record = { 'extensions': 'comparison', 'migrations': 'posture', 'roles': 'comparison', + 'queues': 'comparison', + 'partitions': 'comparison', } /** Checks that compare source against target. */ @@ -107,6 +111,11 @@ export interface DriftIssue { * its SQL: a webhook is a trigger, and dropping a trigger usually isn't. */ destructive?: string + /** + * What applying the fix does to a live database beyond what it changes — + * the locks it holds, a table rewrite, a full scan. See hazards.ts. + */ + hazards?: Array<{ kind: string; on?: string; detail: string }> } export interface CheckResult { @@ -130,6 +139,12 @@ export interface CheckResult { * the difference from the live progress line is explained. */ folded?: Array<{ count: number; into: CheckName }> + /** + * What the check could not compare or decide, said beside its findings: + * objects of a kind it does not compare, a rename it would not guess. Not + * drift — nothing to apply — so neither counted nor scored. + */ + notices?: string[] durationMs: number } diff --git a/packages/cli/src/written-against.ts b/packages/cli/src/written-against.ts new file mode 100644 index 0000000..a42648f --- /dev/null +++ b/packages/cli/src/written-against.ts @@ -0,0 +1,32 @@ +import { createHash } from 'node:crypto' +import { comparedSchemas, fingerprint } from './prove.js' + +/** + * Which target a migration file was written against, and as it was then. + * + * `migrate create` turns the drift between two databases into a file that + * runs later — after review, after a merge — against a target that may have + * moved on: a column added by hand, another migration run. Applied then, a + * file written for the old state could fail half way, or succeed and leave + * something wrong. The target's schema fingerprint is recorded in the file, + * and `migrate run` checks it before running the first pending migration + * written against the same environment. + */ +export const WRITTEN_AGAINST = '-- supaforge:written-against' + +/** A short digest of the target's structure, over the schemas a diff compares. */ +export async function targetFingerprint(dbUrl: string, ignoreSchemas: readonly string[]): Promise { + const schemas = await comparedSchemas(dbUrl, dbUrl, ignoreSchemas) + return createHash('sha256').update(await fingerprint(dbUrl, schemas)).digest('hex').slice(0, 16) +} + +export function writtenAgainstLine(env: string, digest: string): string { + return `${WRITTEN_AGAINST} ${env} ${digest}` +} + +/** The environment and digest a migration's header records, if it has one. */ +export function writtenAgainst(sql: string): { env: string; digest: string } | undefined { + const line = sql.split('\n').find(l => l.startsWith(WRITTEN_AGAINST + ' ')) + const [env, digest] = line?.slice(WRITTEN_AGAINST.length + 1).trim().split(/\s+/) ?? [] + return env && digest ? { env, digest } : undefined +} diff --git a/packages/cli/test/checks/partitions.test.ts b/packages/cli/test/checks/partitions.test.ts new file mode 100644 index 0000000..a57687b --- /dev/null +++ b/packages/cli/test/checks/partitions.test.ts @@ -0,0 +1,41 @@ +import { describe, expect, it } from 'vitest' +import { diffPartitionSets, type PartitionSet } from '../../src/checks/partitions.js' + +const set = (o: Partial = {}): PartitionSet => ({ + parent_table: 'public.events', control: 'created_at', partition_interval: '1 day', partition_type: 'range', + epoch: 'none', premake: 4, automatic_maintenance: 'on', constraint_cols: null, jobmon: true, + date_trunc_interval: null, time_encoder: null, time_decoder: null, has_default: true, + settings: { premake: 4, retention: null, retention_keep_table: true }, ...o, +}) + +describe('partition sets check', () => { + it('registers a set the target lacks through create_parent, with its settings', () => { + const [issue] = diffPartitionSets([set()], [], 'partman') + expect(issue.title).toBe('Partition set not registered: public.events') + expect(issue.sql!.up).toContain(`SELECT "partman".create_parent(p_parent_table => 'public.events', p_control => 'created_at', p_interval => '1 day'`) + expect(issue.sql!.up).toContain(`UPDATE "partman".part_config SET "premake" = 4, "retention" = NULL, "retention_keep_table" = true WHERE parent_table = 'public.events';`) + expect(issue.sql!.down).toBe(`DELETE FROM "partman".part_config WHERE parent_table = 'public.events';`) + }) + + it('updates only the settings that differ, and back', () => { + const [issue] = diffPartitionSets( + [set({ settings: { premake: 4, retention: '30 days', retention_keep_table: false } })], + [set()], 'partman') + expect(issue.sql).toEqual({ + up: `UPDATE "partman".part_config SET "retention" = '30 days', "retention_keep_table" = false WHERE parent_table = 'public.events';`, + down: `UPDATE "partman".part_config SET "retention" = NULL, "retention_keep_table" = true WHERE parent_table = 'public.events';`, + }) + }) + + it('leaves a change of interval, and a set only the target has, to the user', () => { + const issues = diffPartitionSets([set({ partition_interval: '1 week' })], [set(), set({ parent_table: 'public.logs' })], 'partman') + expect(issues.map(i => [i.title, Boolean(i.manualOnly)])).toEqual([ + ['Partition set differs: public.events', true], + ['Extra partition set: public.logs', true], + ]) + }) + + it('reports nothing for sets alike', () => { + expect(diffPartitionSets([set()], [set()], 'partman')).toEqual([]) + }) +}) diff --git a/packages/cli/test/checks/queues.test.ts b/packages/cli/test/checks/queues.test.ts new file mode 100644 index 0000000..9b9e5b0 --- /dev/null +++ b/packages/cli/test/checks/queues.test.ts @@ -0,0 +1,38 @@ +import { describe, expect, it } from 'vitest' +import { QueuesCheck, diffQueues, type Queue } from '../../src/checks/queues.js' +import { destructiveReason } from '../../src/dbdiff.js' + +const q = (queue_name: string, o: Partial = {}): Queue => ({ queue_name, is_partitioned: false, is_unlogged: false, ...o }) + +describe('queues check', () => { + it('creates a missing queue through pgmq, and drops it again on the way down', () => { + const [issue] = diffQueues([q('jobs')], []) + expect(issue).toMatchObject({ title: 'Missing queue: jobs', severity: 'warning' }) + expect(issue.sql).toEqual({ up: "SELECT pgmq.create('jobs');", down: "SELECT pgmq.drop_queue('jobs');" }) + }) + + it('creates an unlogged queue as one, and installs pgmq first where the target lacks it', () => { + const [issue] = diffQueues([q('fast', { is_unlogged: true })], [], false) + expect(issue.sql!.up).toBe("CREATE EXTENSION IF NOT EXISTS pgmq;\nSELECT pgmq.create_unlogged('fast');") + }) + + it('leaves a partitioned queue, and a change of kind, as manual steps', () => { + const issues = diffQueues([q('big', { is_partitioned: true }), q('k')], [q('k', { is_unlogged: true })]) + expect(issues.map(i => [i.title, Boolean(i.manualOnly), i.sql])).toEqual([ + ['Missing queue: big', true, undefined], + ['Queue kind differs: k', true, undefined], + ]) + }) + + it('reports an extra queue as critical, and the drop is held back as destructive', () => { + const [issue] = diffQueues([], [q('old')]) + expect(issue).toMatchObject({ title: 'Extra queue: old', severity: 'critical' }) + expect(destructiveReason(issue.sql!.up)).toBe('drops a queue and the messages in it') + }) + + it('is skipped when neither side has pgmq', async () => { + const check = new QueuesCheck(async () => { throw Object.assign(new Error('relation "pgmq.meta" does not exist'), { code: '42P01' }) }) + await expect(check.scan({ source: { dbUrl: 's' }, target: { dbUrl: 't' }, config: { environments: {} } } as never)) + .rejects.toThrow('pgmq extension not installed') + }) +}) diff --git a/packages/cli/test/ci.test.ts b/packages/cli/test/ci.test.ts index 413f9a5..dbe1047 100644 --- a/packages/cli/test/ci.test.ts +++ b/packages/cli/test/ci.test.ts @@ -465,3 +465,20 @@ describe('computeCiExitCode — drift and posture are different scopes (issue #6 expect(computeCiExitCode(mixed, 'critical')).toBe(1) }) }) + +describe('formatCiSummary carries what a check could not compare', () => { + it('lists each notice with its check, and none when there are none', () => { + const result: ScanResult = { + timestamp: '2026-03-21T00:00:00.000Z', source: 'dev', target: 'prod', score: 100, postureScore: null, + summary: { total: 0, critical: 0, warning: 0, info: 0 }, + checks: [ + { check: 'schema', status: 'clean', issues: [], notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], durationMs: 1 }, + { check: 'rls', status: 'clean', issues: [], durationMs: 1 }, + ], + } + expect(formatCiSummary(result).notices).toEqual([ + { check: 'schema', notice: 'Not compared: Operator (source 1, target 0: ===) — check them by hand.' }, + ]) + expect(formatCiSummary({ ...result, checks: [result.checks[1]] }).notices).toEqual([]) + }) +}) diff --git a/packages/cli/test/dbdiff-units.test.ts b/packages/cli/test/dbdiff-units.test.ts index c37270b..a846e74 100644 --- a/packages/cli/test/dbdiff-units.test.ts +++ b/packages/cli/test/dbdiff-units.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { pairUnits, parseUnits } from '../src/dbdiff-units.js' -import { buildDbDiffArgs, destructiveReason, sqlToIssues } from '../src/dbdiff.js' +import { buildDbDiffArgs, dbdiffNotices, destructiveReason, sqlToIssues, schemaNotices } from '../src/dbdiff.js' /** A section as `dbdiff --units` writes it. */ function units(...parts: Array<[kind: string, object: string, sql: string]>): string { @@ -246,3 +246,82 @@ describe('statements after a comment', () => { ]) }) }) + +describe('sqlToIssues: kinds dbdiff learned to compare, and renames', () => { + const titles = (...parts: Array<[string, string, string]>) => + sqlToIssues({ up: units(...parts), down: '' }, 'schema').map(i => i.title) + + it('names each kind', () => { + expect(titles( + ['CreateDefaultPrivileges', 'postgres.public.tables', 'ALTER DEFAULT PRIVILEGES FOR ROLE "postgres" IN SCHEMA "public" GRANT SELECT ON TABLES TO "anon";'], + ['CreateEventTrigger', 'audit', 'CREATE EVENT TRIGGER "audit" ON ddl_command_end EXECUTE FUNCTION audit_ddl();'], + ['CreateRule', 'orders.keep', 'CREATE RULE keep AS ON DELETE TO public.orders DO INSTEAD NOTHING;'], + ['DropRule', 'orders.old', 'DROP RULE IF EXISTS "old" ON "orders";'], + ['CreateAggregate', 'my_sum(integer)', 'CREATE AGGREGATE "my_sum"(integer) (SFUNC = add_int, STYPE = integer);'], + ['CreateStatistics', 'addr_stx', 'CREATE STATISTICS public.addr_stx (ndistinct) ON city, zip FROM addr;'], + ['CreateTextSearchConfiguration', 'my_cfg', 'CREATE TEXT SEARCH CONFIGURATION "my_cfg" (PARSER = pg_catalog."default");'], + ['CreateCast', 'flag AS boolean', 'CREATE CAST (flag AS boolean) WITH FUNCTION flag_to_bool(flag);'], + ['CreateForeignServer', 'warehouse', `CREATE SERVER "warehouse" FOREIGN DATA WRAPPER "postgres_fdw" OPTIONS ("host" 'db');`], + ['CreateUserMapping', 'reader@warehouse', `CREATE USER MAPPING FOR "reader" SERVER "warehouse" OPTIONS ("user" 'r');`], + ['CreateForeignTable', 'remote', 'CREATE FOREIGN TABLE "remote" (\n "id" integer\n) SERVER "warehouse";'], + )).toEqual([ + 'Default privileges differ: postgres in public on tables', + 'Event trigger missing: public.audit', + 'Rule missing: orders.keep', + 'Extra rule: orders.old', + 'Aggregate missing: public.my_sum', + 'Statistics missing: public.addr_stx', + 'Text search configuration missing: public.my_cfg', + 'Cast missing: flag AS boolean', + 'Foreign server missing: public.warehouse', + 'User mapping missing: reader@warehouse', + 'Foreign table missing: public.remote', + ]) + }) + + it('names renames by what they keep', () => { + expect(titles( + ['AlterTableRenameColumn', 'people.full_name', 'ALTER TABLE "people" RENAME COLUMN "name" TO "full_name";'], + ['AlterTableRename', 'customers', 'ALTER TABLE "clients" RENAME TO "customers";'], + )).toEqual(['Column renamed: public.people.name → full_name', 'Table renamed: public.clients → customers']) + }) + + it('names a changed object as modified, not by the DROP that replaces it', () => { + const issues = sqlToIssues({ up: units( + ['AlterAggregate', 'my_sum(integer)', 'DROP AGGREGATE IF EXISTS "my_sum"(integer);\nCREATE AGGREGATE "my_sum"(integer) (SFUNC = add_int, STYPE = integer);'], + ['AlterForeignServer', 'warehouse', `ALTER SERVER "warehouse" OPTIONS (DROP "host");\nALTER SERVER "warehouse" OPTIONS (ADD "host" 'db');`], + ), down: '' }, 'schema') + expect(issues.map(i => [i.title, i.severity])).toEqual([ + ['Aggregate modified: public.my_sum', 'warning'], + ['Foreign server modified: public.warehouse', 'warning'], + ]) + }) + + it('leaves dbdiff warnings out of the findings: they are not drift', () => { + const issues = sqlToIssues({ + up: '', down: '', + notices: ['Operator objects are not compared (source 1, target 0: ===) — check them by hand'], + }, 'schema') + expect(issues).toEqual([]) + }) +}) + +describe('schemaNotices', () => { + it('says what dbdiff did not compare, and leaves out what the Realtime check does', () => { + expect(schemaNotices([ + 'Operator objects are not compared (source 1, target 0: ===) — check them by hand', + 'Publication objects are not compared (source 1, target 1: supabase_realtime) — check them by hand', + 'public.a.b → c not taken for a rename', + ])).toEqual([ + 'Not compared: Operator (source 1, target 0: ===) — check them by hand.', + 'public.a.b → c not taken for a rename.', + ]) + }) +}) + +describe('dbdiffNotices', () => { + it('reads the warnings out of dbdiff\'s output, colour and all', () => { + expect(dbdiffNotices('\u001b[0;36mℹ Now generating UP migration\u001b[0m\n\u001b[0;33m⚠ Operator objects are not compared (source 1, target 0: ===) — check them by hand\u001b[0m\nWritten: x.sql')) + .toEqual(['Operator objects are not compared (source 1, target 0: ===) — check them by hand']) + }) +}) diff --git a/packages/cli/test/e2e/prove.test.ts b/packages/cli/test/e2e/prove.test.ts index 5f87365..cba821c 100644 --- a/packages/cli/test/e2e/prove.test.ts +++ b/packages/cli/test/e2e/prove.test.ts @@ -219,6 +219,25 @@ describeE2E('convergence proof', () => { expect(residual).toMatch(/table public\.sales_2026: missing/) }, 300_000) + // A sample of the target's rows goes through the fixes on the copy. A + // generated column's values follow its expression, so a fix that changes + // the expression changes them, as it means to; the base columns must hold. + it('lets a generated column take its new expression, and still guards the rest', async () => { + const table = (expr: string) => `CREATE TABLE t (id int PRIMARY KEY, a int, g int GENERATED ALWAYS AS (${expr}) STORED);` + await h.applySql('source', table('a * 3')) + await h.applySql('target', table('a * 2') + ' INSERT INTO t (id, a) SELECT i, i FROM generate_series(1, 20) i;') + const regenerate = 'ALTER TABLE t DROP COLUMN g; ALTER TABLE t ADD COLUMN g int GENERATED ALWAYS AS (a * 3) STORED;' + + const proof = await prove(regenerate) + expect(proof.data?.sampled).toContain('public.t') + expect(proof.data?.lost).toEqual([]) + expect(proof.converged).toBe(true) + + const clobbered = await prove(regenerate + ' UPDATE t SET a = a + 1;') + expect(clobbered.converged).toBe(false) + expect(clobbered.residual).toEqual(['rows lost in public.t: values changed in a']) + }, 300_000) + it('reports a migration that fails to execute, rather than claiming drift', async () => { await expect(prove('CREATE TABLE ;')).rejects.toThrow() }, 300_000) diff --git a/packages/cli/test/e2e/schema-dir.test.ts b/packages/cli/test/e2e/schema-dir.test.ts new file mode 100644 index 0000000..42b4474 --- /dev/null +++ b/packages/cli/test/e2e/schema-dir.test.ts @@ -0,0 +1,47 @@ +/** + * A directory of schema files as the source (`diff --source-dir`). + * + * The files are loaded into a throwaway database on the target's server and + * compared with the target; a file using what a later one creates still + * loads. Verdicts are read from the target, and the throwaway database must + * be gone afterwards. + */ +import { describe, it, expect, beforeAll, afterAll } from 'vitest' +import { mkdir, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { PgHarness } from '../harness/PgHarness.js' +import { describeWithContainers } from '../harness/containers.js' + +const describeE2E = describeWithContainers() + +describeE2E('e2e: schema files as the source', () => { + let h: PgHarness + + beforeAll(async () => { + h = new PgHarness({ verbose: !!process.env.E2E_VERBOSE, keep: !!process.env.E2E_KEEP }) + await h.up() + await h.applySql('target', 'CREATE TABLE public.tasks (id int PRIMARY KEY);') + }, 300_000) + + afterAll(async () => { await h?.down() }, 120_000) + + it('applies the files to the target, whatever order they name things in', async () => { + const ws = await h.workspace() + const dir = join(ws, 'supabase', 'schemas') + await mkdir(dir, { recursive: true }) + await writeFile(join(dir, '01_views.sql'), 'create view public.open_tasks as select id, title from public.tasks where not done;\n') + await writeFile(join(dir, '02_tasks.sql'), ` + create table public.tasks (id int primary key, title text not null default '', done boolean not null default false); + create index tasks_open on public.tasks (id) where not done; + `) + + const r = await h.cli(['diff', '--source-dir', dir, '--check', 'schema', '--apply'], { cwd: ws }) + expect(r.code, r.stdout + r.stderr).toBe(0) + expect(await h.sql('target', `SELECT string_agg(attname, ',' ORDER BY attnum) FROM pg_attribute WHERE attrelid = 'public.tasks'::regclass AND attnum > 0`)).toBe('id,title,done') + expect(await h.sql('target', `SELECT to_regclass('public.open_tasks') IS NOT NULL`)).toBe('t') + + const again = await h.cli(['diff', '--source-dir', dir, '--check', 'schema', '--json'], { cwd: ws }) + expect(JSON.parse(again.stdout).checks[0].issues).toEqual([]) + expect(await h.sql('target', `SELECT count(*) FROM pg_database WHERE datname LIKE 'supaforge_schema_%'`)).toBe('0') + }, 300_000) +}) diff --git a/packages/cli/test/hazards.test.ts b/packages/cli/test/hazards.test.ts new file mode 100644 index 0000000..5c344ad --- /dev/null +++ b/packages/cli/test/hazards.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' +import { hazardsOf } from '../src/hazards.js' + +const kinds = (sql: string) => hazardsOf(sql).map(h => `${h.kind}${h.on ? ' ' + h.on : ''}`) + +describe('hazardsOf', () => { + it('a column type change locks the table and may rewrite it', () => { + expect(kinds('ALTER TABLE "public"."orders" ALTER COLUMN "total" TYPE numeric(12,2);')) + .toEqual(['blocks-reads-and-writes public.orders', 'rewrite public.orders']) + }) + + it('a new CHECK scans the table, unless NOT VALID', () => { + expect(kinds('ALTER TABLE orders ADD CONSTRAINT pos CHECK (total > 0);')).toEqual(['blocks-reads-and-writes orders', 'full-scan orders']) + expect(kinds('ALTER TABLE orders ADD CONSTRAINT pos CHECK (total > 0) NOT VALID;')).toEqual(['blocks-reads-and-writes orders']) + }) + + it('a foreign key blocks writes only, and scans', () => { + expect(kinds('ALTER TABLE orders ADD CONSTRAINT fk FOREIGN KEY (c) REFERENCES customers (id);')) + .toEqual(['blocks-writes orders', 'full-scan orders']) + }) + + it('a column with a volatile default rewrites; a constant default does not', () => { + expect(kinds('ALTER TABLE t ADD COLUMN id uuid DEFAULT gen_random_uuid();')).toEqual(['blocks-reads-and-writes t', 'rewrite t']) + expect(kinds("ALTER TABLE t ADD COLUMN state text DEFAULT 'new';")).toEqual(['blocks-reads-and-writes t']) + }) + + it('statistics and storage options take no lock reads or writes wait on', () => { + expect(kinds('ALTER TABLE t ALTER COLUMN v SET STATISTICS 500;')).toEqual([]) + }) + + it('an index blocks writes; built concurrently, it cannot run in a transaction', () => { + expect(kinds('CREATE INDEX i ON orders (total);')).toEqual(['blocks-writes orders']) + expect(kinds('CREATE INDEX CONCURRENTLY i ON orders (total);')).toEqual(['not-transactional orders']) + }) + + it('a table the same fix creates is nobody\'s to wait on', () => { + expect(kinds('CREATE TABLE fresh (id int);\nALTER TABLE fresh ADD CONSTRAINT c CHECK (id > 0);\nCREATE INDEX f ON fresh (id);')).toEqual([]) + }) + + it('names data loss', () => { + expect(hazardsOf('DROP TABLE "old";')[0]).toMatchObject({ kind: 'data-loss', detail: 'drops a table and its rows' }) + }) + + it('policies, triggers and renames', () => { + expect(kinds('CREATE POLICY p ON orders USING (true);')).toEqual(['blocks-reads-and-writes orders']) + expect(kinds('CREATE TRIGGER t BEFORE INSERT ON orders FOR EACH ROW EXECUTE FUNCTION f();')).toEqual(['blocks-writes orders']) + expect(kinds('ALTER TABLE "people" RENAME COLUMN "name" TO "full_name";')).toEqual(['blocks-reads-and-writes people']) + }) +}) diff --git a/packages/cli/test/prove-data.test.ts b/packages/cli/test/prove-data.test.ts new file mode 100644 index 0000000..25a342e --- /dev/null +++ b/packages/cli/test/prove-data.test.ts @@ -0,0 +1,41 @@ +import { afterEach, describe, expect, it } from 'vitest' +import { compareTables, emptiedTables, sampleRows, type TableSnapshot } from '../src/prove-data.js' + +const snap = (rows: number, columns: Record, filenode = '1'): TableSnapshot => ({ + rows, filenode, columns: Object.fromEntries(Object.entries(columns).map(([k, [type, hash]]) => [k, { type, hash }])), +}) + +describe('the data half of a proof', () => { + afterEach(() => { delete process.env.SUPAFORGE_PROVE_ROWS }) + + it('passes rows kept as they were, and says which tables were rewritten', () => { + const before = new Map([['public.o', snap(3, { id: ['integer', 'a'], note: ['text', 'b'] }, '10')]]) + const after = new Map([['public.o', snap(3, { id: ['bigint', 'changed-text'], note: ['text', 'b'] }, '11')]]) + expect(compareTables(before, after, ['public.o'], 'ALTER TABLE o ALTER COLUMN id TYPE bigint;')) + .toEqual({ lost: [], rewritten: ['public.o'] }) + }) + + it('catches rows lost, and values changed in a column whose type did not', () => { + const before = new Map([['public.a', snap(5, { x: ['text', 'h'] })], ['public.b', snap(2, { y: ['text', 'h1'] })]]) + const after = new Map([['public.a', snap(0, { x: ['text', ''] })], ['public.b', snap(2, { y: ['text', 'h2'] })]]) + expect(compareTables(before, after, ['public.a', 'public.b'], '').lost).toEqual([ + 'public.a: 5 row(s) before, 0 after', + 'public.b: values changed in y', + ]) + }) + + it('expects rows to go from a table a fix deletes from or truncates', () => { + expect([...emptiedTables('DELETE FROM "public"."plans" WHERE id = 1;\nTRUNCATE flags;')]).toEqual(['plans', 'flags']) + const before = new Map([['public.plans', snap(5, { id: ['integer', 'h'] })]]) + const after = new Map([['public.plans', snap(4, { id: ['integer', 'h2'] })]]) + expect(compareTables(before, after, ['public.plans'], 'DELETE FROM plans WHERE id = 1;').lost).toEqual([]) + }) + + it('samples a thousand rows a table unless told otherwise, and none for 0', () => { + expect(sampleRows()).toBe(1000) + process.env.SUPAFORGE_PROVE_ROWS = '0' + expect(sampleRows()).toBe(0) + process.env.SUPAFORGE_PROVE_ROWS = 'lots' + expect(sampleRows()).toBe(1000) + }) +}) diff --git a/packages/cli/test/prove-system-schemas.test.ts b/packages/cli/test/prove-system-schemas.test.ts index 02afde6..9342129 100644 --- a/packages/cli/test/prove-system-schemas.test.ts +++ b/packages/cli/test/prove-system-schemas.test.ts @@ -164,6 +164,23 @@ describe('residual the held-back fixes account for', () => { ]) }) + // A serial or identity column's sequence goes with its table, or with the + // column: a held-back drop of either leaves the sequence too. + it('counts the sequence a held-back table or column owns', () => { + const r = residualHeldBack([ + 'sequence public.legacy_id_seq: unexpected (owned by legacy.id)', + 'sequence public.t_old_seq: unexpected (owned by t.old)', + 'sequence public."Odd_n_seq": unexpected (owned by "Odd"."n")', + 'sequence public.free_seq: unexpected', + 'sequence public.kept_id_seq: unexpected (owned by kept.id)', + ], [...held, 'DROP TABLE IF EXISTS "Odd";']) + expect(r.heldBack).toHaveLength(3) + expect(r.unexplained).toEqual([ + 'sequence public.free_seq: unexpected', + 'sequence public.kept_id_seq: unexpected (owned by kept.id)', + ]) + }) + it('accounts for nothing when nothing is held back', () => { expect(residualHeldBack(['table public.legacy: unexpected'], []).unexplained).toHaveLength(1) }) diff --git a/packages/cli/test/render.test.ts b/packages/cli/test/render.test.ts index 949b230..1439274 100644 --- a/packages/cli/test/render.test.ts +++ b/packages/cli/test/render.test.ts @@ -77,6 +77,17 @@ describe('renderSummary', () => { }) }) +describe('notices', () => { + it('are said under their layer, and the headline still reads clean', () => { + const out = renderSummary({ + ...cleanResult, + checks: [{ check: 'schema', status: 'clean', issues: [], notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], durationMs: 5 }], + }) + expect(out).toMatch(/Layer 1 \(Schema\):\s+0 issues\n\s+ⓘ Not compared: Operator \(source 1, target 0: ===\) — check them by hand\./) + expect(out).toMatch(/No drift detected/i) + }) +}) + describe('renderDetailed', () => { it('includes SQL fixes', () => { const output = renderDetailed(driftedResult) diff --git a/packages/cli/test/restore.test.ts b/packages/cli/test/restore.test.ts index 2e679e8..cb79fe0 100644 --- a/packages/cli/test/restore.test.ts +++ b/packages/cli/test/restore.test.ts @@ -474,3 +474,13 @@ describe('restore: recognising a migration with nothing in it', () => { expect(isCommentOnly('-- add the column\nALTER TABLE t ADD COLUMN c text;')).toBe(false) }) }) + +describe('restoreError', () => { + it('explains running out of locks, and the two ways round it', async () => { + const { restoreError } = await import('../src/restore.js') + const err = Object.assign(new Error('out of shared memory'), { code: '53200' }) + expect(restoreError(err, true)).toMatch(/more locks than the server allows in one transaction.*max_locks_per_transaction.*--no-transaction/) + expect(restoreError(err, false)).toBe('out of shared memory') + expect(restoreError(new Error('syntax error'), true)).toBe('syntax error') + }) +}) diff --git a/packages/cli/test/scanner.test.ts b/packages/cli/test/scanner.test.ts index 0ffdde7..f15299b 100644 --- a/packages/cli/test/scanner.test.ts +++ b/packages/cli/test/scanner.test.ts @@ -50,6 +50,29 @@ describe('scan', () => { expect(result.score).toBe(100) }) + // What a check could not compare is said beside its result, and is not + // drift: the check stays clean and the score whole. + it('keeps a check\'s notices beside its findings, out of the count', async () => { + class NoticingLayer extends Check { + readonly name = 'schema' as const + async scan(ctx: CheckContext): Promise { + ctx.onNotice?.('Not compared: Operator (source 1, target 0: ===) — check them by hand.') + return [] + } + } + const registry = new CheckRegistry() + registry.register(new NoticingLayer()) + + const result = await scan(registry, { config, checks: ['schema'] }) + + expect(result.checks[0]).toMatchObject({ + status: 'clean', + notices: ['Not compared: Operator (source 1, target 0: ===) — check them by hand.'], + }) + expect(result.summary.total).toBe(0) + expect(result.score).toBe(100) + }) + it('returns drifted status when issues found', async () => { const registry = new CheckRegistry() registry.register(new MockLayer('rls', [ diff --git a/packages/cli/test/state-diff.test.ts b/packages/cli/test/state-diff.test.ts index 611bf8c..18b7e01 100644 --- a/packages/cli/test/state-diff.test.ts +++ b/packages/cli/test/state-diff.test.ts @@ -92,4 +92,16 @@ describe('diffState', () => { 'extension btree_gist: missing', ]) }) + + it('names the column that owns an unexpected sequence', () => { + const seq = (name: string, owned_by: string | null) => + ({ schema: 'public', name, type: 'integer', start: 1, increment: 1, min: 1, max: 9, cache: 1, cycle: false, owned_by }) + const b = empty() + b.sequences = [seq('tasks_id_seq', 'tasks.id'), seq('free_seq', null)] + + expect(diffState(empty(), b)).toEqual([ + 'sequence public.tasks_id_seq: unexpected (owned by tasks.id)', + 'sequence public.free_seq: unexpected', + ]) + }) }) diff --git a/packages/cli/test/written-against.test.ts b/packages/cli/test/written-against.test.ts new file mode 100644 index 0000000..46de5de --- /dev/null +++ b/packages/cli/test/written-against.test.ts @@ -0,0 +1,13 @@ +import { describe, expect, it } from 'vitest' +import { writtenAgainst, writtenAgainstLine } from '../src/written-against.js' + +describe('written-against header', () => { + it('reads back what migrate create writes', () => { + const sql = `-- Migration: x\n${writtenAgainstLine('production', 'abc123')}\n\nCREATE TABLE t (id int);` + expect(writtenAgainst(sql)).toEqual({ env: 'production', digest: 'abc123' }) + }) + + it('is absent from a migration written by hand', () => { + expect(writtenAgainst('CREATE TABLE t (id int);')).toBeUndefined() + }) +})