From 69e473eb2b967ef0d35218f5e6a87f08f7b36032 Mon Sep 17 00:00:00 2001 From: Jasdeep Date: Wed, 21 Oct 2026 23:19:37 +0000 Subject: [PATCH] fix: --prove reads a constant array cast whole or element by element alike `kind IN ('web', 'shop')` on a varchar column is printed with the cast on the whole array, but SQL written from that print can come back with the cast on each element; both are stable from then on. A CHECK, index or policy made from a previous sync's SQL never read like the source's, so the proof refused correct migrations on it. Both spellings are now read as one where the proof compares: for an array of constants, casting the array and casting each element are the same. Fixes #151 Co-Authored-By: Claude Opus 5.5 --- packages/cli/src/expression-forms.ts | 20 ++++++++++++++++++ packages/cli/src/prove.ts | 5 +++-- packages/cli/src/state-diff.ts | 19 ++++++++++------- packages/cli/test/expression-forms.test.ts | 24 ++++++++++++++++++++++ 4 files changed, 59 insertions(+), 9 deletions(-) create mode 100644 packages/cli/src/expression-forms.ts create mode 100644 packages/cli/test/expression-forms.test.ts diff --git a/packages/cli/src/expression-forms.ts b/packages/cli/src/expression-forms.ts new file mode 100644 index 0000000..b3f268c --- /dev/null +++ b/packages/cli/src/expression-forms.ts @@ -0,0 +1,20 @@ +/** + * One spelling for two ways PostgreSQL prints the same constant array. + * + * `x IN ('a', 'b')` on a varchar column is printed with the cast on the whole + * array — `(ARRAY['a'::character varying, 'b'::character varying])::text[]` — + * but SQL written from that print, parsed again, can come back with the cast + * on each element instead — `ARRAY[('a'::character varying)::text, …]`. Both + * forms are stable from then on, so a CHECK, index or policy made from a + * previous sync's SQL never reads like the source's. Only an array of quoted + * constants is rewritten: for those, casting the array and casting each + * element are the same. + */ +const LITERAL = `'(?:[^']|'')*'(?:::[a-z_][\\w ]*(?:\\(\\d+(?:,\\s*\\d+)?\\))?)?` +const CAST_ARRAY = new RegExp(`\\(ARRAY\\[(${LITERAL}(?:, ${LITERAL})*)\\]\\)::([a-z_][\\w ]*?(?:\\(\\d+(?:,\\s*\\d+)?\\))?)\\[\\]`, 'g') +const ELEMENT = new RegExp(LITERAL, 'g') + +export function canonicalArrays(text: string): string { + return text.replace(CAST_ARRAY, (_match, elements: string, type: string) => + `ARRAY[${(elements.match(ELEMENT) ?? []).map(e => `(${e})::${type}`).join(', ')}]`) +} diff --git a/packages/cli/src/prove.ts b/packages/cli/src/prove.ts index dad7256..e8db19e 100644 --- a/packages/cli/src/prove.ts +++ b/packages/cli/src/prove.ts @@ -27,6 +27,7 @@ import { promisify } from 'node:util' import { fingerprintSql, stateSql, type SchemaState } from '@akal/pg-conformance' import { pgQuery, type QueryFn } from './db' import { runLikeApply, type PlannedSql } from './promote' +import { canonicalArrays } from './expression-forms.js' import { diffState } from './state-diff' import { resolvePgDumpPath, getServerMajorVersion, describeMissingPgDump } from './pg-tools' import { rolesNamedBy, sqlSkeleton } from './sql-deps' @@ -328,7 +329,7 @@ export async function proveConvergence(opts: { // The verdict stays with the fingerprint. It is the comparison already // trusted, and keeping it means the structured diff below can only change // how a difference is *described*, never whether one is detected. - if (want === got) return reported({ converged: true, residual: [], cloneName }) + if (canonicalArrays(want) === canonicalArrays(got)) return reported({ converged: true, residual: [], cloneName }) // The clone's objects have been through a dump and restore, or were // recreated from the SQL a rendering produced; the source's have not, and @@ -344,7 +345,7 @@ export async function proveConvergence(opts: { sourceCopyCreated = true if (roundTrip.ok) { const wantRoundTripped = await fingerprint(roundTripUrl, schemas) - if (wantRoundTripped === got) return reported({ converged: true, residual: [], cloneName }) + if (canonicalArrays(wantRoundTripped) === canonicalArrays(got)) return reported({ converged: true, residual: [], cloneName }) const residual = await describeDifference(roundTripUrl, cloneUrl, schemas, wantRoundTripped, got) return reported(withinScope(residual, before && diffState(await schemaState(roundTripUrl, schemas), before), opts.checks, cloneName)) } diff --git a/packages/cli/src/state-diff.ts b/packages/cli/src/state-diff.ts index ccb4832..10115d6 100644 --- a/packages/cli/src/state-diff.ts +++ b/packages/cli/src/state-diff.ts @@ -18,7 +18,12 @@ * pair of schemas the fingerprint distinguishes, so this reports the same * differences — it does not find more of them, and it must not find fewer. */ + import type { SchemaState, StateTable, StateColumn } from '@akal/pg-conformance' +import { canonicalArrays } from './expression-forms.js' + +/** A definition with its constant arrays spelt one way — see expression-forms. */ +const canonical = (d: string | null | undefined) => (typeof d === 'string' ? canonicalArrays(d) : d) /** How a value reads when absent, so `null` never reaches the output raw. */ const show = (v: unknown): string => { @@ -106,26 +111,26 @@ function compareTable(out: string[], subject: string, a: StateTable, b: StateTab compareKeyed(out, `constraint on ${prefix}`, a.constraints, b.constraints, c => c.type === 'not_null' ? c.definition : c.name, (o, s, x, y) => { - compareField(o, s, 'definition', x.definition, y.definition) + compareField(o, s, 'definition', canonical(x.definition), canonical(y.definition)) compareField(o, s, 'validated', x.validated, y.validated) compareField(o, s, 'deferrable', x.deferrable, y.deferrable) compareField(o, s, 'deferred', x.deferred, y.deferred) }) compareKeyed(out, `index on ${prefix}`, a.indexes, b.indexes, i => i.name, - (o, s, x, y) => compareField(o, s, 'definition', x.definition, y.definition)) + (o, s, x, y) => compareField(o, s, 'definition', canonical(x.definition), canonical(y.definition))) compareKeyed(out, `policy on ${prefix}`, a.policies, b.policies, p => p.name, (o, s, x, y) => { compareField(o, s, 'command', x.command, y.command) compareField(o, s, 'permissive', x.permissive, y.permissive) compareField(o, s, 'roles', x.roles, y.roles) - compareField(o, s, 'using', x.using, y.using) - compareField(o, s, 'with check', x.with_check, y.with_check) + compareField(o, s, 'using', canonical(x.using), canonical(y.using)) + compareField(o, s, 'with check', canonical(x.with_check), canonical(y.with_check)) }) compareKeyed(out, `trigger on ${prefix}`, a.triggers, b.triggers, t => t.name, - (o, s, x, y) => compareField(o, s, 'definition', x.definition, y.definition)) + (o, s, x, y) => compareField(o, s, 'definition', canonical(x.definition), canonical(y.definition))) } /** @@ -145,7 +150,7 @@ export function diffState(before: SchemaState, after: SchemaState): string[] { v => qualify(v.schema, v.name), (o, s, a, b) => { compareField(o, s, 'materialized', a.materialized, b.materialized) - compareField(o, s, 'definition', a.definition, b.definition) + compareField(o, s, 'definition', canonical(a.definition), canonical(b.definition)) compareField(o, s, 'options', a.options, b.options) }) @@ -163,7 +168,7 @@ export function diffState(before: SchemaState, after: SchemaState): string[] { r => `${qualify(r.schema, r.name)}(${r.arguments})`, (o, s, a, b) => { compareField(o, s, 'kind', a.kind, b.kind) - compareField(o, s, 'definition', a.definition, b.definition) + compareField(o, s, 'definition', canonical(a.definition), canonical(b.definition)) }) compareKeyed(out, 'type', before.types, after.types, diff --git a/packages/cli/test/expression-forms.test.ts b/packages/cli/test/expression-forms.test.ts new file mode 100644 index 0000000..6a6cb65 --- /dev/null +++ b/packages/cli/test/expression-forms.test.ts @@ -0,0 +1,24 @@ +import { describe, it, expect } from 'vitest' +import { canonicalArrays } from '../src/expression-forms.js' + +// The two ways PostgreSQL prints `kind IN ('web', 'shop')` on a varchar +// column, after one round of SQL written from a print and parsed again. +const WHOLE = `CHECK (((kind)::text = ANY ((ARRAY['web'::character varying, 'shop'::character varying])::text[])))` +const EACH = `CHECK (((kind)::text = ANY (ARRAY[('web'::character varying)::text, ('shop'::character varying)::text])))` + +describe('canonicalArrays', () => { + it('spells a cast on a constant array as a cast on each element', () => { + expect(canonicalArrays(WHOLE)).toBe(EACH) + expect(canonicalArrays(EACH)).toBe(EACH) + }) + + it('keeps quotes, types with a length, and several arrays apart', () => { + expect(canonicalArrays(`a = ANY ((ARRAY['it''s'::varchar(10), 'x'])::text[]) AND b = ANY ((ARRAY['1'::text])::integer[])`)) + .toBe(`a = ANY (ARRAY[('it''s'::varchar(10))::text, ('x')::text]) AND b = ANY (ARRAY[('1'::text)::integer])`) + }) + + it('leaves an array of anything but constants alone', () => { + const columns = `a = ANY ((ARRAY[b, c])::text[])` + expect(canonicalArrays(columns)).toBe(columns) + }) +})