fix(migrate-ts): regex CHECK on a varchar/text column no longer reads as drift on Postgres

CHANGELOG.md

@@ -22,6 +22,16 @@ here.**
 
 ### Fixed
 
+- **`meta verify --db` no longer reports false drift on a regex CHECK over a varchar/text
+  column.** Postgres stores `"key" ~ '^[a-z]+$'` as `((key)::text ~ '^[a-z]+$'::text)`; the
+  literal's `::text` was already folded away but the column's was not, so a database `migrate`
+  had just built showed the same constraint as both added and dropped. The comparison now
+  drops a `(col)::text` cast before a regex-match operator (`~`, `~*`, `!~`, `!~*`) only when
+  the database column is `text`, `varchar` or `bpchar`; on any other column, `citext` included,
+  the cast changes the operator and still reads as drift, as does a changed pattern or any
+  other cast. Schema comparison is TypeScript-owned, so no other port
+  carries this comparison.
+
 - **The api reference now follows a routes generator's `filter` as well as its `expose`.**
   `routesFile({ filter })` and `routesFileHono({ filter })` mount nothing for an entity the filter
   rejects, but the API pages still listed all five verbs for it. The pages now document the

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

@@ -41,12 +41,17 @@ function unwrapBacktickIdentifiers(expr: string): string {
     .join("");
 }
 
+const NO_COLUMNS: ReadonlySet<string> = new Set();
+
 /**
  * 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.
  */
-export function normalizeCheckExpr(expr: string): string {
+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
@@ -95,7 +100,7 @@ export function normalizeCheckExpr(expr: string): string {
   // meaningful, so two checks differing only there must stay distinct — else a real
   // pattern change reads as clean drift, which is this file's failure in the other
   // direction and the worse of the two.
-  return collapseSeparatorSpacingOutsideQuotes(folded);
+  return collapseSeparatorSpacingOutsideQuotes(folded, textColumns);
 }
 
 /**
@@ -192,7 +197,17 @@ function readCastTarget(s: string, i: number): { type: string; consumed: number
  * cast on anything that is not a literal or a text-returning operator is a REAL cast and
  * survives, because erasing it would let a genuine change read as clean drift.
  */
-function isElidableCast(type: string, lhs: string): boolean {
+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
+  // `((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.
+  if (type === "text" && /^\s*!?~\*?(?![-+*\/<>=~!@#%^&|?])/.test(rest)) {
+    const column = /(?:^|[^\w.])([a-z_]\w*)$/.exec(lhs)?.[1];
+    if (column !== undefined && textColumns.has(column)) return true;
+  }
   // `->>` / `#>>` yield text; a following `::text` cannot change anything.
   if (type === "text" && /(?:->>|#>>)\s*(?:'(?:[^']|'')*'|[\w."]+)$/.test(lhs)) return true;
   // A numeric literal cast to a numeric type.
@@ -210,7 +225,7 @@ function isElidableCast(type: string, lhs: string): boolean {
  * distinct regex CHECKs would compare equal — a real change reading as clean drift, which
  * is this file's failure in the worse direction.
  */
-function collapseSeparatorSpacingOutsideQuotes(s: string): string {
+function collapseSeparatorSpacingOutsideQuotes(s: string, textColumns: ReadonlySet<string>): string {
   let out = "";
   let i = 0;
   const n = s.length;
@@ -246,7 +261,7 @@ function collapseSeparatorSpacingOutsideQuotes(s: string): string {
       const target = readCastTarget(s, i);
       i += target.consumed;
       const lhs = out.replace(/\s+$/, "");
-      token = isElidableCast(target.type, lhs) ? "" : `::${target.type}`;
+      token = isElidableCast(target.type, lhs, s.slice(i), textColumns) ? "" : `::${target.type}`;
       out = lhs;
       // Whitespace AFTER the target type is load-bearing — `x::integer is not null` must
       // not become `x::integeris not null` — so the cast, unlike a comma or an operator,
@@ -272,9 +287,13 @@ function collapseSeparatorSpacingOutsideQuotes(s: string): string {
 }
 
 /** True when two CHECK expressions are equivalent after normalization. */
-export function checkExprEquals(a: string | undefined, b: string | undefined): boolean {
+export function checkExprEquals(
+  a: string | undefined,
+  b: string | undefined,
+  textColumns?: ReadonlySet<string>,
+): boolean {
   if (a === undefined || b === undefined) return false;
-  return normalizeCheckExpr(a) === normalizeCheckExpr(b);
+  return normalizeCheckExpr(a, textColumns) === normalizeCheckExpr(b, textColumns);
 }
 
 /**

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

@@ -809,6 +809,8 @@ 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"]);
+
 function diffTableChecks(
   expected: TableDescriptor,
   actual: TableDescriptor,
@@ -829,12 +831,15 @@ 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()),
+  );
 
   for (const ec of expected.checks) {
     const ac = actualByName.get(ec.name);
     if (ac) {
       consumed.add(ac);
-      if (!checkExprEquals(ec.expression, ac.expression)) {
+      if (!checkExprEquals(ec.expression, ac.expression, textColumns)) {
         const respelled = respelledSpread(ec, ac);
         changes.push({ kind: "drop-check", table: expected.name, ...sx, check: ec.name, restore: ac, ...respelled, status: ALLOWED });
         changes.push({ kind: "add-check", table: expected.name, ...sx, check: ec, ...respelled, status: ALLOWED });

server/typescript/packages/migrate-ts/src/introspect/postgres.ts

@@ -475,6 +475,7 @@ async function readColumns(k: Kysely<any>, schema: string, tableName: string): P
       name: r.column_name,
       sqlType,
       nullable: r.is_nullable === "YES",
+      udtName: r.udt_name.toLowerCase(),
     };
 
     const def = parsePgDefault(r.column_default);

server/typescript/packages/migrate-ts/src/types.ts

@@ -46,6 +46,12 @@ export interface ColumnDescriptor {
   name: string;
   sqlType: SqlType;
   nullable: boolean;
+  /**
+   * Postgres `udt_name` as introspected (`text`, `varchar`, `bpchar`, `citext`, …). Set by
+   * the Postgres introspector only; absent on the expected side and on SQLite. `sqlType`
+   * folds every unrecognised type into `text`, so this is the only place citext survives.
+   */
+  udtName?: string;
   default?: ColumnDefault;
   identity?: "increment" | "uuid";
   /**

server/typescript/packages/migrate-ts/test/check-evolution/diff-offline.test.ts

@@ -50,3 +50,23 @@ describe("diffTableChecks — postgres existing-table evolution", () => {
     expect(r.changes.some((c) => c.kind.endsWith("-check"))).toBe(false);
   });
 });
+
+describe("diffTableChecks — regex check on a text-family column", () => {
+  const generated = CHK("spaces_key_regex_chk", `"key" ~ '^[a-z0-9][a-z0-9-]*$'`);
+  const stored = CHK("spaces_key_regex_chk", `(((key)::text ~ '^[a-z0-9][a-z0-9-]*$'::text))`);
+  const withKey = (udtName: string | undefined, checks: CheckDescriptor[]): SchemaSnapshot => ({
+    tables: [{ name: "spaces", columns: [{ name: "key", sqlType: { kind: "text" }, nullable: false, ...(udtName ? { udtName } : {}) }],
+      indexes: [], foreignKeys: [], checks, primaryKey: [] }],
+    views: [],
+  });
+  const checkChanges = async (udtName: string | undefined) => {
+    const r = await diff({ expected: withKey(undefined, [generated]), actual: withKey(udtName, [stored]), dialect: "postgres" });
+    return r.changes.filter((c) => c.kind.endsWith("-check"));
+  };
+  test.each(["text", "varchar", "bpchar"])("%s column: Postgres's rewrite is no drift", async (udt) => {
+    expect(await checkChanges(udt)).toEqual([]);
+  });
+  test("citext column: cast vs uncast is drift", async () => {
+    expect((await checkChanges("citext")).map((c) => c.kind).sort()).toEqual(["add-check", "drop-check"]);
+  });
+});

server/typescript/packages/migrate-ts/test/check-evolution/normalize.test.ts

@@ -204,3 +204,25 @@ describe("stripCheckWrapper", () => {
     expect(stripCheckWrapper("qty >= 1")).toBe("qty >= 1");
   });
 });
+
+describe("regex check on a varchar/text column", () => {
+  const generated = `"key" ~ '^[a-z0-9][a-z0-9-]*$'`;
+  const introspected = `(((key)::text ~ '^[a-z0-9][a-z0-9-]*$'::text))`;
+  const textColumns = new Set(["key"]);
+  test("Postgres's added column cast does not read as drift on a text-family column", () => {
+    expect(checkExprEquals(generated, introspected, textColumns)).toBe(true);
+    expect(checkExprEquals(generated, `(((key)::text !~* '^[a-z0-9][a-z0-9-]*$'::text))`, textColumns)).toBe(false);
+  });
+  test("without a text-family column set the cast is kept", () => {
+    expect(checkExprEquals(generated, introspected)).toBe(false);
+  });
+  test("a cast on a column outside the set (citext) reads as drift", () => {
+    expect(checkExprEquals(generated, introspected, new Set(["other"]))).toBe(false);
+  });
+  test("a changed regex pattern still reads as drift", () => {
+    expect(checkExprEquals(generated, `(((key)::text ~ '^[a-z][a-z0-9-]*$'::text))`, textColumns)).toBe(false);
+  });
+  test("a cast that is not before a regex operator survives", () => {
+    expect(checkExprEquals("qty > 0", "((qty)::text > '0'::text)", new Set(["qty"]))).toBe(false);
+  });
+});

server/typescript/packages/migrate-ts/test/integration/pg-regex-check-cast.test.ts

@@ -0,0 +1,98 @@
+/**
+ * A regex CHECK on a text-family column must not read as drift on Postgres.
+ *
+ * Postgres stores `"key" ~ '^[a-z0-9][a-z0-9-]*$'` as `(((key)::text ~ '…'::text))`. The
+ * oracle is the live engine: create the table from the DDL migrate emits for the authored
+ * check, read the constraint back through the real introspector, and diff the authored
+ * metadata against it. On a citext column the cast changes the operator (case-sensitive vs
+ * citext's case-insensitive `~`), so the same pair must still read as drift.
+ *
+ * Skips gracefully when MIGRATE_TS_PG_URL is not set.
+ */
+
+import { test, expect, beforeAll, afterAll, describe } from "bun:test";
+import { Pool } from "pg";
+import { Kysely, PostgresDialect, sql } from "kysely";
+import { MetaDataLoader, InMemoryStringSource } from "@metaobjectsdev/metadata";
+import { buildExpectedSchema } from "../../src/expected-schema.js";
+import { introspectPostgres } from "../../src/introspect/postgres.js";
+import { diff } from "../../src/diff/index.js";
+
+const PG_URL = process.env["MIGRATE_TS_PG_URL"];
+const PATTERN = "^[a-z0-9][a-z0-9-]*$";
+
+const meta = JSON.stringify({
+  "metadata.root": {
+    package: "acme",
+    children: [
+      {
+        "object.entity": {
+          name: "Space",
+          children: [
+            { "source.rdb": { "@table": "spaces" } },
+            { "field.long": { name: "id" } },
+            { "field.string": { name: "key", "@column": "key", children: [{ "validator.regex": { name: "re", "@pattern": PATTERN } }] } },
+            { "identity.primary": { name: "pk", "@fields": ["id"], "@generation": "increment" } },
+          ],
+        },
+      },
+    ],
+  },
+});
+
+describe("regex CHECK vs Postgres's stored rewrite (live PG)", () => {
+  if (!PG_URL) {
+    test.skip("skipped — MIGRATE_TS_PG_URL not set", () => {});
+    return;
+  }
+  let pool: Pool;
+  let kysely: Kysely<Record<string, unknown>>;
+
+  beforeAll(async () => {
+    pool = new Pool({ connectionString: PG_URL });
+    kysely = new Kysely<Record<string, unknown>>({ dialect: new PostgresDialect({ pool }) });
+    await sql.raw("CREATE EXTENSION IF NOT EXISTS citext").execute(kysely);
+  });
+  afterAll(async () => {
+    await sql.raw("DROP TABLE IF EXISTS spaces CASCADE").execute(kysely);
+    await pool.end();
+  });
+
+  const checkChanges = async (colType: string, authoredCheck = `"key" ~ '${PATTERN}'`) => {
+    await sql.raw("DROP TABLE IF EXISTS spaces CASCADE").execute(kysely);
+    // The constraint is created from the AUTHORED spelling; PG stores its own rendering.
+    await sql
+      .raw(`CREATE TABLE spaces (id bigserial PRIMARY KEY, "key" ${colType} NOT NULL,
+            CONSTRAINT spaces_key_regex_chk CHECK (${authoredCheck}))`)
+      .execute(kysely);
+    const actual = await introspectPostgres(kysely);
+    const stored = actual.tables.find((t) => t.name === "spaces")?.checks.find((c) => c.name === "spaces_key_regex_chk");
+    const loaded = await new MetaDataLoader().load([new InMemoryStringSource(meta)]);
+    expect(loaded.errors).toEqual([]);
+    const d = await diff({ expected: buildExpectedSchema(loaded.root, { dialect: "postgres" }), actual, dialect: "postgres" });
+    return { stored: stored?.expression, kinds: d.changes.filter((c) => c.kind.endsWith("-check")).map((c) => c.kind).sort() };
+  };
+
+  test.each(["varchar(64)", "text", "char(8)"])("%s column: PG's rewrite is no drift", async (colType) => {
+    const r = await checkChanges(colType);
+    // The gate is not vacuous: PG really did rewrite the authored text.
+    expect(r.stored).toContain("::text");
+    expect(r.kinds).toEqual([]);
+  });
+
+  test("citext column, uncast check: PG keeps citext's own operator and it is clean", async () => {
+    const r = await checkChanges("citext");
+    expect(r.stored).toContain("::citext");
+    expect(r.kinds).toEqual([]);
+  });
+
+  test("citext column: a check stored with an explicit ::text cast is a different (case-sensitive) check, so drift", async () => {
+    const r = await checkChanges("citext", `("key")::text ~ '${PATTERN}'`);
+    expect(r.kinds).toEqual(["add-check", "drop-check"]);
+  });
+
+  test("varchar column: a changed pattern still reads as drift", async () => {
+    const r = await checkChanges("varchar(64)", `"key" ~ '^[a-z]+$'`);
+    expect(r.kinds).toEqual(["add-check", "drop-check"]);
+  });
+});