fix(migrate-ts): stop false CHECK drift on Postgres and compare literals as written (#498)

* fix(migrate-ts): stop false CHECK drift on Postgres and compare literals as written

A one-value enum, presentIff and validators on varchar(n) columns reported drift
after migrate had just built them, because Postgres stores IN (x) as = x,
IS NOT DISTINCT FROM as NOT (IS DISTINCT FROM) and casts varchar columns to text.
The normaliser also lower-cased and bracket-stripped quoted literals, so an enum
member or regex case change read as clean. char(n) is no longer a text-family
column for the (col)::text fold. Adds a Postgres round-trip over every generated
CHECK shape.

* no-mistakes(review): Fold IS NOT DISTINCT FROM safely; keep text cast in renames

* no-mistakes(review): Rewrite only the depth-0 IS DISTINCT FROM operator

* no-mistakes(review): Fold authored bare col::text on text columns

* test(migrate-ts): pin that a stored length((col)::text) on char(n) is drift

Covers the non-regex bpchar case on real Postgres and offline.

Author: dmealing

CHANGELOG.md

@@ -106,6 +106,26 @@ here.**
   on any failure, so the previous state is intact. `meta deps check` also loads the model once
   its hashes match and exits 1 with the load error when it does not load.
 
+- **`meta verify --db` no longer reports false CHECK drift on Postgres for a one-value enum, a
+  `presentIff` validator, or a validator on a `varchar(n)` column; a changed enum-member case
+  or regex case no longer reads as clean.** Postgres stores `"k" IN ('x')` as `k = 'x'` and
+  `a IS NOT DISTINCT FROM b` as `NOT (a IS DISTINCT FROM b)`, and casts a `varchar` column to
+  text (`(region)::text`) in a comparison, a `length()` and a null-safe test. The comparison now
+  folds all three back, so a database `migrate` just built verifies clean. The same false drift
+  reached `meta migrate --from-db`, which proposed a drop-check and add-check on every run, and
+  the offline `migrate --dry-run` after a `baseline --from-db`. It has done so since 1.0.x. In
+  the other direction, the comparison lower-cased quoted literals and deleted every bracket,
+  quote and run of blanks inside them, so `IN ('OPEN')` against `IN ('open')` and `~ '^[A-Z]+$'`
+  against `~ '^[a-z]+$'` compared as equal. Literals are now compared as written. A check
+  stored in a different case than the model declares therefore shows as drift once: run
+  `meta migrate` to bring it back. Separately, `char(n)` is no longer treated as a text column
+  for the `(col)::text` fold: its own regex operator sees the blank padding the cast trims, and
+  Postgres never adds the cast there, so an explicit `(col)::text ~ '…'` on a `char(n)` column
+  now reads as drift. A precedence change (`(a OR b) AND c` against `a OR (b AND c)`) is still
+  not detected; that needs Postgres to canonicalise the expected side. A new Postgres test
+  migrates, applies and verifies every CHECK shape `migrate` generates. Schema comparison is
+  TypeScript-owned, so no other port carries this comparison.
+
 ### Documentation
 
 - **Two doc errors corrected against the code.** `docs/recipes/mysql.md` said Python ships a

server/typescript/packages/migrate-ts/src/check-expr-compare.ts

@@ -18,6 +18,14 @@
 //   - respells a cast TARGET canonically: `x::int`      →  `(x)::integer`
 // All five are canonicalized below so an enum/range CHECK introspected from PG
 // compares equal to the one we generate (idempotency on the --from-db / verify paths).
+// Three more are folded back by foldPgAnalyzerRewrites and COLUMN_TEXT_CAST: a
+// one-element `x IN (v)` stored as `x = v`, `a IS NOT DISTINCT FROM b` stored as
+// `NOT (a IS DISTINCT FROM b)`, and `(varchar_col)::text` stored for a text operator.
+//
+// Everything textual runs OUTSIDE single-quoted literals (mapOutsideLiterals). One thing is
+// still NOT canonicalized: dropping every bracket erases precedence, so `(a OR b) AND c` and
+// `a OR (b AND c)` compare equal. Fixing that takes the server's own canonical form, not more
+// string rules.
 //
 // The last two are why a cast-bearing `@expr` used to drift FOREVER. The paren-strip
 // turns PG's added parens into a space, so `(x)::integer` reduced to `x ::integer` while
@@ -43,38 +51,171 @@ function unwrapBacktickIdentifiers(expr: string): string {
 
 const NO_COLUMNS: ReadonlySet<string> = new Set();
 
+/**
+ * `(col)::text` — the form Postgres writes when it casts a `varchar` column to `text` to reach
+ * a text operator or function (`length((code)::text)`, `(a)::text < (b)::text`,
+ * `((region)::text IS DISTINCT FROM 'eu'::text)`) — and the bare `col::text` an author writes
+ * for the same cast. Matched BEFORE the brackets are stripped, while `(col)` can still be told
+ * from a call `f(col)`: the match must not follow a word character or a dot. It is dropped only for a column the caller names as text-family
+ * (`text` / `varchar`), where the cast changes nothing; on any other column it changes the
+ * operator (citext) or the value (`char(n)` trims its padding), so it stays and reads as drift.
+ */
+const COLUMN_TEXT_CAST = /(?<![\w.])(?:\(\s*([a-z_]\w*)\s*\)|([a-z_]\w*))\s*::\s*text\b/g;
+
+/** Split on single-quoted literals ('' escapes honored) and rewrite only the text between them. */
+function mapOutsideLiterals(s: string, fn: (part: string, index: number) => string): string {
+  return s
+    .split(/('(?:[^']|'')*')/)
+    .map((part, i) => (i % 2 === 1 ? part : fn(part, i)))
+    .join("");
+}
+
+/**
+ * Index of the `)` closing the `(` at `open`, skipping single-quoted literals and
+ * double-quoted identifiers; -1 when unbalanced.
+ */
+function matchingParen(s: string, open: number): number {
+  let depth = 0;
+  for (let i = open; i < s.length; i++) {
+    const ch = s[i]!;
+    if (ch === "'" || ch === '"') {
+      i++;
+      while (i < s.length && !(s[i] === ch && s[i + 1] !== ch)) i += s[i] === ch ? 2 : 1;
+    } else if (ch === "(") depth++;
+    else if (ch === ")" && --depth === 0) return i;
+  }
+  return -1;
+}
+
+/**
+ * `inner` with everything below depth 0 blanked (literals, quoted identifiers, bracketed
+ * groups). Same length as `inner`, so an offset found here is the same offset in `inner`.
+ */
+function topLevelText(inner: string): string {
+  let out = "";
+  let depth = 0;
+  for (let i = 0; i < inner.length; i++) {
+    const ch = inner[i]!;
+    if (ch === "'" || ch === '"') {
+      const start = i++;
+      while (i < inner.length && !(inner[i] === ch && inner[i + 1] !== ch)) i += inner[i] === ch ? 2 : 1;
+      i = Math.min(i, inner.length - 1);
+      out += " ".repeat(i - start + 1);
+    } else if (ch === "(" || ch === "[") { depth++; out += " "; }
+    else if (ch === ")" || ch === "]") { depth--; out += " "; }
+    else out += depth === 0 ? ch : " ";
+  }
+  return out;
+}
+
+/**
+ * `NOT (A IS DISTINCT FROM B)` → `(A IS NOT DISTINCT FROM B)`: Postgres has a node only for
+ * `IS DISTINCT FROM`, so it stores `a IS NOT DISTINCT FROM b` in the first spelling. Run on
+ * the RAW expression, while the brackets still show what `NOT` applies to: only `NOT`
+ * followed by one balanced bracketed operand that is exactly a single `IS DISTINCT FROM`
+ * comparison (no depth-0 `and` / `or` / `not`) is folded. `(NOT x) IS DISTINCT FROM y`
+ * has no such operand and stays as written, so the fold cannot invent an equality.
+ */
+function foldNotDistinctFrom(expr: string): string {
+  let out = "";
+  let i = 0;
+  while (i < expr.length) {
+    const ch = expr[i]!;
+    if (ch === "'" || ch === '"') {
+      const start = i++;
+      while (i < expr.length && !(expr[i] === ch && expr[i + 1] !== ch)) i += expr[i] === ch ? 2 : 1;
+      i++;
+      out += expr.slice(start, i);
+      continue;
+    }
+    const not = /^not\s*\(/i.exec(expr.slice(i));
+    if (not && !/[\w.]/.test(expr[i - 1] ?? " ")) {
+      const open = i + not[0].length - 1;
+      const close = matchingParen(expr, open);
+      if (close !== -1) {
+        const inner = expr.slice(open + 1, close);
+        const top = topLevelText(inner);
+        const operators = [...top.matchAll(/\bis\s+distinct\s+from\b/gi)];
+        if (operators.length === 1 && !/\b(?:and|or|not)\b/i.test(top)) {
+          const op = operators[0]!;
+          out += `(${inner.slice(0, op.index)}is not distinct from${inner.slice(op.index + op[0].length)})`;
+          i = close + 1;
+          continue;
+        }
+      }
+    }
+    out += ch;
+    i++;
+  }
+  return out;
+}
+
+/**
+ * Two rewrites Postgres makes while ANALYZING an expression, which no textual normalisation
+ * of its output can undo, so they are folded back to the spelling migrate emits:
+ *
+ *  - a ONE-element `x IN (v)` is stored as `x = v` (`IN` only becomes `= ANY (ARRAY[…])` from
+ *    two elements, which the `any array` fold above already handles). A literal list is told
+ *    apart by the comma after its first element, so `in 'a', 'b'` is left alone.
+ *  - `a IS NOT DISTINCT FROM b` is stored as `NOT (a IS DISTINCT FROM b)`; see
+ *    foldNotDistinctFrom, which runs earlier because it needs the brackets.
+ *
+ * The IN fold fires on text that SQLite and D1 never store and runs outside literals. It sits on
+ * the normalised string (brackets already gone), where it is a plain word pattern.
+ */
+function foldPgAnalyzerRewrites(s: string): string {
+  const parts = s.split(/('(?:[^']|'')*')/);
+  return parts
+    .map((part, i) => {
+      if (i % 2 === 1) return part;
+      let p = part;
+      // `<lhs> in 'v'` — the literal is the next part; a following comma means a list.
+      if (/(?<!\bnot) in $/.test(p) && i + 1 < parts.length && !(parts[i + 2] ?? "").trimStart().startsWith(",")) {
+        p = p.replace(/ in $/, "=");
+      }
+      return p.replace(/(?<!\bnot) in (-?\d+(?:\.\d+)?)(?![\w.]|\s*,)/g, "=$1");
+    })
+    .join("");
+}
+
 /**
  * Canonical form: drop casts/brackets/parens, fold `= ANY (ARRAY[…])` back to `IN`,
  * lower-case, collapse whitespace, canonicalize cast target types, and collapse spacing
  * around commas, casts and SYMBOLIC operators (outside single-quoted literals).
  *
- * `textColumns` (lower-cased names of text-family columns) lets a `(col)::text` cast before
- * a regex operator fold away; see isElidableCast.
+ * `textColumns` (lower-cased names of `text` / `varchar` columns) lets a `(col)::text` cast
+ * fold away; see COLUMN_TEXT_CAST and isElidableCast.
  */
 export function normalizeCheckExpr(expr: string, textColumns: ReadonlySet<string> = NO_COLUMNS): string {
-  const stripped = unwrapBacktickIdentifiers(expr)
-    .toLowerCase()
-    // Drop `::text` / `::"MyType"` type casts PG adds to literals. The lookbehind
-    // restricts the strip to a cast that immediately follows a CLOSING single
-    // quote (`'open'::text`), so a `::` appearing INSIDE a regex pattern literal
-    // (`slug ~ 'a::foo'`) is preserved — otherwise two distinct regex CHECKs would
-    // normalize equal and a pattern change would be silently missed.
-    .replace(/(?<=')::\s*"?\w+"?/g, "")
-    // Unwrap double-quoted IDENTIFIERS only (`"enumval"` → `enumval`). We emit a
-    // quoted column (`"enumVal" IN (...)`) so a mixed-case column is valid SQL;
-    // PG's introspected definition quotes mixed-case identifiers too. Unwrapping
-    // makes the generated and introspected forms compare equal, and keeps a
-    // legacy unquoted check matching its quoted re-emit. The pattern matches only
-    // a quote-pair wrapping a bare identifier (word chars), so a `"` INSIDE a
-    // single-quoted string/regex literal (`slug ~ 'a"b'`) is preserved — two
-    // distinct patterns must not normalize equal.
-    .replace(/"(\w+)"/g, "$1")
-    .replace(/[()[\]]/g, " ")     // drop parens AND square brackets (ARRAY[…])
-    .replace(/\s+/g, " ")
-    .trim();
-  // PG stores `col IN (…)` as `col = ANY (ARRAY[…])`; after the bracket strip above
-  // that reads `col = any array …`. Fold it back to the `col in …` form we emit.
-  const folded = stripped.replace(/=\s*any\s+array/g, "in").replace(/\s+/g, " ").trim();
+  // Every textual rewrite below runs OUTSIDE single-quoted literals. A literal is DATA: its
+  // case, its brackets, its `"quoted"` words and its runs of spaces all change what the
+  // constraint accepts, so two checks differing only inside a literal (an enum member's
+  // case, a regex's character class, a `(` in a pattern) must stay distinct. Reading them
+  // as equal is this file's failure in the worse direction: a real change reported clean.
+  const stripped = mapOutsideLiterals(foldNotDistinctFrom(unwrapBacktickIdentifiers(expr)), (part, index) =>
+    // Drop `::text` / `::"MyType"` type casts PG adds to literals. Only a cast that
+    // immediately follows a CLOSING single quote (`'open'::text`) is one, which is exactly
+    // the head of an outside part that follows a literal — so a `::` INSIDE a regex pattern
+    // literal (`slug ~ 'a::foo'`) is never seen here.
+    (index > 0 ? part.replace(/^::\s*"?\w+"?/, "") : part)
+      .toLowerCase()
+      // Unwrap double-quoted IDENTIFIERS only (`"enumval"` → `enumval`). We emit a
+      // quoted column (`"enumVal" IN (...)`) so a mixed-case column is valid SQL;
+      // PG's introspected definition quotes mixed-case identifiers too. Unwrapping
+      // makes the generated and introspected forms compare equal, and keeps a
+      // legacy unquoted check matching its quoted re-emit.
+      .replace(/"(\w+)"/g, "$1")
+      .replace(COLUMN_TEXT_CAST, (whole, parenthesised: string | undefined, bare: string | undefined) => {
+        const column = parenthesised ?? bare!;
+        return textColumns.has(column) ? column : whole;
+      })
+      .replace(/[()[\]]/g, " ")     // drop parens AND square brackets (ARRAY[…])
+      .replace(/\s+/g, " ")
+      // PG stores `col IN (…)` as `col = ANY (ARRAY[…])`; after the bracket strip above
+      // that reads `col = any array …`. Fold it back to the `col in …` form we emit.
+      .replace(/=\s*any\s+array/g, "in"),
+  ).trim();
+  const folded = foldPgAnalyzerRewrites(stripped);
   // Normalize spacing around COMMAS and SYMBOLIC OPERATORS, so the spelling a human
   // writes compares equal to the one Postgres hands back.
   //
@@ -199,8 +340,9 @@ function readCastTarget(s: string, i: number): { type: string; consumed: number
  */
 function isElidableCast(type: string, lhs: string, rest: string, textColumns: ReadonlySet<string>): boolean {
   // `::text` on a COLUMN directly before a regex-match operator (`~`, `~*`, `!~`, `!~*`),
-  // and only when the caller names that column as text-family (text, character varying,
-  // character). On such a column PG adds the cast itself (`"key" ~ '…'` comes back
+  // and only when the caller names that column as text-family (text, character varying;
+  // NOT char(n), whose own operator sees the blank padding the cast trims). On such a
+  // column PG adds the cast itself (`"key" ~ '…'` comes back
   // `((key)::text ~ '…'::text)`). On any other column, citext included, the cast changes
   // the operator (`col::text ~ 'A'` is case-sensitive, `col ~ 'A'` on citext is not), so it
   // survives. With no set supplied nothing is elided.

server/typescript/packages/migrate-ts/src/diff/index.ts

@@ -283,7 +283,10 @@ export async function diff(
   // Pass 3: column renames (table renames were resolved in Pass 1c).
   await detectColumnRenames(changes, args.onAmbiguous, args.renames);
   // Pass 3b: a renamed column carries the constraints and indexes named after it.
-  carryThroughColumnRenames(changes, args.dialect);
+  carryThroughColumnRenames(changes, args.dialect, (id) => {
+    const live = predecessors.get(id) ?? actualTables.get(id);
+    return live === undefined ? NO_TEXT_COLUMNS : textColumnsOf(live);
+  });
 
   // Strip the rename-detection side-channel fields before status assignment / return.
   for (const c of changes) {
@@ -369,7 +372,7 @@ function diffRenamedTables(
     const carried = withRenamedRefTables({ ...actual, name: r.to }, renamed);
     const tableChanges: Change[] = [];
     diffTablePair(expected, carried, tableChanges, dialect);
-    changes.push(...(dialect === "postgres" ? foldNameOnlyChanges(r, tableChanges) : tableChanges));
+    changes.push(...(dialect === "postgres" ? foldNameOnlyChanges(r, tableChanges, textColumnsOf(actual)) : tableChanges));
   }
   return { renamed, predecessors };
 }
@@ -422,7 +425,11 @@ function withRenamedRefTables(t: TableDescriptor, renamed: ReadonlyMap<string, s
  * sits in the always-allowed status bucket, so a fold over a pair that differed in shape
  * would silently bypass the gate.
  */
-function foldNameOnlyChanges(r: Extract<Change, { kind: "rename-table" }>, tableChanges: Change[]): Change[] {
+function foldNameOnlyChanges(
+  r: Extract<Change, { kind: "rename-table" }>,
+  tableChanges: Change[],
+  textColumns: ReadonlySet<string>,
+): Change[] {
   const constraintRenames: NameChange[] = [];
   const indexRenames: NameChange[] = [];
   const folded = new Set<Change>();
@@ -436,7 +443,7 @@ function foldNameOnlyChanges(r: Extract<Change, { kind: "rename-table" }>, table
     } else if (d.kind === "drop-check" && d.restore !== undefined) {
       const restore = d.restore;
       const a = tableChanges.find((c): c is Extract<Change, { kind: "add-check" }> =>
-        c.kind === "add-check" && !folded.has(c) && checkExprEquals(c.check.expression, restore.expression));
+        c.kind === "add-check" && !folded.has(c) && checkExprEquals(c.check.expression, restore.expression, textColumns));
       if (a) { folded.add(d).add(a); constraintRenames.push({ from: d.check, to: a.check.name }); }
     } else if (d.kind === "drop-index" && d.restore !== undefined) {
       const restore = d.restore;
@@ -474,7 +481,11 @@ function foldNameOnlyChanges(r: Extract<Change, { kind: "rename-table" }>, table
  *
  * Anything that is not EXACTLY the old rule under the new names stays a gated drop+add.
  */
-function carryThroughColumnRenames(changes: Change[], dialect: Dialect | undefined): void {
+function carryThroughColumnRenames(
+  changes: Change[],
+  dialect: Dialect | undefined,
+  textColumnsOfTable: (tableId: string) => ReadonlySet<string>,
+): void {
   const renamesByTable = new Map<string, Map<string, string>>();
   const renameChangeByTable = new Map<string, Extract<Change, { kind: "rename-column" }>>();
   for (const c of changes) {
@@ -503,8 +514,10 @@ function carryThroughColumnRenames(changes: Change[], dialect: Dialect | undefin
     const sameTable = (c: Change): boolean => !paired.has(c) && idOf(c) === table;
     if (d.kind === "drop-check" && d.restore !== undefined) {
       const want = renameExprIdentifiers(d.restore.expression, renames);
+      const lowerRenames = new Map([...renames].map(([from, to]) => [from.toLowerCase(), to.toLowerCase()]));
+      const textColumns = new Set([...textColumnsOfTable(table)].map((col) => lowerRenames.get(col) ?? col));
       const a = changes.find((c): c is Extract<Change, { kind: "add-check" }> =>
-        c.kind === "add-check" && sameTable(c) && checkExprEquals(c.check.expression, want));
+        c.kind === "add-check" && sameTable(c) && checkExprEquals(c.check.expression, want, textColumns));
       if (a) { paired.add(d).add(a); pairs.push({ drop: d, add: a, from: d.check, to: a.check.name, index: false, table }); }
     } else if (d.kind === "drop-fk" && d.restore !== undefined) {
       const want: FkDescriptor = { ...d.restore, columns: mapCols(d.restore.columns, renames) };
@@ -809,7 +822,15 @@ function respelledSpread(expected: CheckDescriptor, actual: CheckDescriptor): {
   return checkExprRespelledNullSafe(expected.expression, actual.expression) ? { respelled: true } : {};
 }
 
-const PG_TEXT_FAMILY_UDTS: ReadonlySet<string> = new Set(["text", "varchar", "bpchar"]);
+const PG_TEXT_FAMILY_UDTS: ReadonlySet<string> = new Set(["text", "varchar"]);
+
+const NO_TEXT_COLUMNS: ReadonlySet<string> = new Set();
+
+function textColumnsOf(table: TableDescriptor): Set<string> {
+  return new Set(
+    table.columns.filter((c) => c.udtName !== undefined && PG_TEXT_FAMILY_UDTS.has(c.udtName)).map((c) => c.name.toLowerCase()),
+  );
+}
 
 function diffTableChecks(
   expected: TableDescriptor,
@@ -831,9 +852,7 @@ function diffTableChecks(
   const exprFallback = dialect === "sqlite" || dialect === "d1";
   const actualByName = new Map(actual.checks.filter((c) => c.name !== "").map((c) => [c.name, c]));
   const consumed = new Set<CheckDescriptor>();
-  const textColumns = new Set(
-    actual.columns.filter((c) => c.udtName !== undefined && PG_TEXT_FAMILY_UDTS.has(c.udtName)).map((c) => c.name.toLowerCase()),
-  );
+  const textColumns = textColumnsOf(actual);
 
   for (const ec of expected.checks) {
     const ac = actualByName.get(ec.name);