From f387bcc80ea5e5768659c16fb4a70c95ceb1760c Mon Sep 17 00:00:00 2001 From: avallete Date: Sat, 22 Aug 2026 19:36:30 +0200 Subject: [PATCH 1/6] feat(cli): upgrade pg-delta next to alpha.46 Consume OWNED BY / load assist, drop the CLI parameter-ACL workaround, pretty-print generated SQL, and only strip image extensions when declarations recreate them. --- apps/cli/docs/supabase/db/diff.md | 2 +- .../db/schema-declarative-generate.md | 2 + apps/cli/package.json | 2 +- .../db/schema/declarative/declarative.flow.ts | 24 +-- .../legacy-pgdelta-declarative-shadow-prep.ts | 132 +++++++++++++ ...delta-declarative-shadow-prep.unit.test.ts | 142 ++++++++++++++ .../legacy-pgdelta-engine.next.layer.ts | 9 +- .../shared/legacy-pgdelta-engine.service.ts | 4 +- .../db/shared/legacy-pgdelta-files.ts | 4 + .../legacy-pgdelta-next-adapter.layer.ts | 176 ++++-------------- .../legacy-pgdelta-next-adapter.unit.test.ts | 82 +++----- .../legacy-pgdelta-next-shadow.layer.ts | 34 +--- ...acy-pgdelta-next-shadow.layer.unit.test.ts | 44 ----- pnpm-lock.yaml | 10 +- pnpm-workspace.yaml | 2 +- 15 files changed, 358 insertions(+), 311 deletions(-) create mode 100644 apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts create mode 100644 apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts delete mode 100644 apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.unit.test.ts diff --git a/apps/cli/docs/supabase/db/diff.md b/apps/cli/docs/supabase/db/diff.md index fb407e4349..6ad22f7e1a 100644 --- a/apps/cli/docs/supabase/db/diff.md +++ b/apps/cli/docs/supabase/db/diff.md @@ -12,7 +12,7 @@ By default, all schemas in the target database are diffed. Use the `--schema pub Projects created by a recent `supabase init` default to the pg-delta diff engine (`[experimental.pgdelta] enabled = true` in `config.toml`). Existing projects are unaffected and keep using migra unless they opt in. To fall back to the legacy migra engine, set `enabled = false` under `[experimental.pgdelta]`, or pass `--use-migra` for a single run. -With the bundled pg-delta engine, diff SQL defaults to lowercase keywords and a maximum width of 180, matching its declarative export. When `-f` writes migrations, execution-aware transaction semantics are preserved as ordered per-unit files; non-transactional units carry a directive that the CLI apply path honors. Flattened review output retains the rendered SQL and preambles, but not the unit boundaries supplied to a migration runner. Configure overrides with `[experimental.pgdelta] format_options`, or set `format_options = "null"` to emit raw, unformatted statements. +With the bundled pg-delta engine, diff SQL defaults to uppercase keywords, indent 2, a maximum width of 180, trailing commas, and column/key alignment, matching its declarative export. When `-f` writes migrations, execution-aware transaction semantics are preserved as ordered per-unit files; non-transactional units carry a directive that the CLI apply path honors. Flattened review output retains the rendered SQL and preambles, but not the unit boundaries supplied to a migration runner. Configure overrides with `[experimental.pgdelta] format_options`, or set `format_options = "null"` to emit raw, unformatted statements. While the diff command is able to capture most schema changes, there are cases where it is known to fail. Currently, this could happen if you schema contains: diff --git a/apps/cli/docs/supabase/db/schema-declarative-generate.md b/apps/cli/docs/supabase/db/schema-declarative-generate.md index 164176d6c0..1cd416e747 100644 --- a/apps/cli/docs/supabase/db/schema-declarative-generate.md +++ b/apps/cli/docs/supabase/db/schema-declarative-generate.md @@ -6,4 +6,6 @@ Exports the schema of a live database (local, linked, or custom URL) into SQL fi The bundled pg-delta engine writes one directory per schema at the root of that directory (`supabase/schemas/public/tables/users.sql`, `supabase/schemas/public/schema.sql`), with cluster-level objects that belong to no schema under a reserved `_cluster/` directory (`supabase/schemas/_cluster/roles.sql`). A schema literally named `_cluster` or `_custom`, in any casing, has its leading underscore percent-encoded (`%5Fcluster/`) so it can never claim a directory the export owns. Hand-authored SQL that pg-delta does not model belongs in `_custom/`, which the export never writes to and never prunes. +Emitted SQL uses the same default format as `db pull` (uppercase keywords, indent 2, width 180, column-aligned). Override with `[experimental.pgdelta] format_options`, or set `format_options = "null"` for raw statements. + Requires `--experimental` flag or `[experimental.pgdelta] enabled = true` in config. diff --git a/apps/cli/package.json b/apps/cli/package.json index f7ceb4f910..73c47713e0 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -55,7 +55,7 @@ "@parcel/watcher": "^2.6.0", "@supabase/api": "workspace:*", "@supabase/config": "workspace:*", - "@supabase/pg-delta": "1.0.0-alpha.42", + "@supabase/pg-delta": "1.0.0-alpha.46", "@supabase/pg-topo": "1.0.0-alpha.5", "@supabase/process-compose": "workspace:*", "@supabase/stack": "workspace:*", diff --git a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts index 06669d82c3..28a470e511 100644 --- a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts +++ b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts @@ -1,5 +1,6 @@ import type { LegacyPgDeltaImplementation } from "../../../../shared/legacy-pgdelta-next-flag.ts"; import { legacySchemaToCsvField } from "../../../../shared/legacy-schema-flags.ts"; +import { legacyDeclaredSqlExtensions } from "../../shared/legacy-pgdelta-declarative-shadow-prep.ts"; import type { LegacyPgDeltaRemovalSummary } from "../../shared/legacy-pgdelta-engine.service.ts"; /** Extensions that legacy pg-delta treated as part of its implicit Supabase baseline. */ @@ -159,18 +160,6 @@ function matchImplicitExtension(message: string): LegacyImplicitExtensionMatch | }; } -/** - * Masks SQL comments and strings while preserving offsets. Extension declarations - * are DDL, so occurrences inside comments, quoted values, and dollar bodies must - * not suppress compatibility guidance. - */ -function maskSqlNonCode(sql: string): string { - return sql.replaceAll( - /--[^\r\n]*|\/\*[\s\S]*?\*\/|'(?:''|[^'])*'|\$(?:[a-zA-Z_][\w$]*)?\$[\s\S]*?\$(?:[a-zA-Z_][\w$]*)?\$/g, - (matched) => matched.replaceAll(/[^\r\n]/g, " "), - ); -} - function maskSqlComments(sql: string): string { return sql.replaceAll(/--[^\r\n]*|\/\*[\s\S]*?\*\//g, (matched) => matched.replaceAll(/[^\r\n]/g, " "), @@ -180,16 +169,7 @@ function maskSqlComments(sql: string): string { export function legacyDeclaredExtensions( files: readonly LegacyDeclarativeSqlFile[], ): ReadonlySet { - const declared = new Set(); - const pattern = - /\bCREATE\s+EXTENSION\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:"([^"]+)"|([a-zA-Z_][\w$-]*))/gi; - for (const file of files) { - for (const match of maskSqlNonCode(file.sql).matchAll(pattern)) { - const extension = match[1] ?? match[2]; - if (extension !== undefined) declared.add(extension.toLowerCase()); - } - } - return declared; + return legacyDeclaredSqlExtensions(files); } function declaredImplicitExtensions( diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts new file mode 100644 index 0000000000..1d272c1b2a --- /dev/null +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -0,0 +1,132 @@ +import { Effect } from "effect"; + +import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; + +export type LegacyDeclarativeShadowClient = { + readonly query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>; +}; + +/** Image-default extensions the user may still declare; omit means keep the install. */ +const IMAGE_DEFAULT_EXTENSIONS = ["pgjwt", "pgcrypto", "uuid-ossp"] as const; + +const IMAGE_DEFAULT_EXTENSION_SET = new Set(IMAGE_DEFAULT_EXTENSIONS); + +const DROP_IMAGE_DEFAULT_EXTENSION: Record<(typeof IMAGE_DEFAULT_EXTENSIONS)[number], string> = { + pgjwt: "DROP EXTENSION IF EXISTS pgjwt", + pgcrypto: "DROP EXTENSION IF EXISTS pgcrypto", + "uuid-ossp": 'DROP EXTENSION IF EXISTS "uuid-ossp"', +}; + +const CREATE_EXTENSION_RE = + /\bCREATE\s+EXTENSION\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:"([^"]+)"|([a-zA-Z_][\w$-]*))/gi; + +/** Blank comments and literals so CREATE EXTENSION in those positions is ignored. */ +const maskSqlNonCode = (sql: string): string => + sql.replaceAll( + /--[^\r\n]*|\/\*[\s\S]*?\*\/|'(?:''|[^'])*'|\$(?:[a-zA-Z_][\w$]*)?\$[\s\S]*?\$(?:[a-zA-Z_][\w$]*)?\$/g, + (matched) => matched.replaceAll(/[^\r\n]/g, " "), + ); + +export const legacyDeclaredSqlExtensions = ( + files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, +): ReadonlySet => { + const declared = new Set(); + for (const file of files) { + for (const match of maskSqlNonCode(file.sql).matchAll(CREATE_EXTENSION_RE)) { + const name = (match[1] ?? match[2] ?? "").toLowerCase(); + if (name !== "") declared.add(name); + } + } + return declared; +}; + +const declaredImageExtensions = ( + files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, +): ReadonlySet => { + const declared = new Set(); + for (const name of legacyDeclaredSqlExtensions(files)) { + if (IMAGE_DEFAULT_EXTENSION_SET.has(name)) declared.add(name); + } + return declared; +}; + +export const legacyParsePostgresMajorVersion = (serverVersion: string): number => { + const major = Number.parseInt(serverVersion, 10); + return Number.isInteger(major) ? major : 0; +}; + +export const legacyDeclarativeBaselinePrepStatements = ( + majorVersion: number, + declared: ReadonlySet, +): ReadonlyArray => { + const dropPgcrypto = declared.has("pgcrypto"); + // Image pgjwt depends on pgcrypto; drop it first so pgcrypto can drop. + const dropPgjwt = declared.has("pgjwt") || dropPgcrypto; + const dropUuidOssp = declared.has("uuid-ossp"); + const statements: string[] = []; + if (majorVersion === 14 && dropUuidOssp) { + statements.push("ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT"); + } + if (dropPgjwt) statements.push(DROP_IMAGE_DEFAULT_EXTENSION.pgjwt); + if (dropPgcrypto) statements.push(DROP_IMAGE_DEFAULT_EXTENSION.pgcrypto); + if (dropUuidOssp) statements.push(DROP_IMAGE_DEFAULT_EXTENSION["uuid-ossp"]); + return statements; +}; + +/** Recreate image pgjwt after a pgcrypto-only drop so omit still means keep. */ +export const legacyFilesForDeclarativeShadowLoad = ( + files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, +): ReadonlyArray<{ readonly name: string; readonly sql: string }> => { + const declared = declaredImageExtensions(files); + if (!declared.has("pgcrypto") || declared.has("pgjwt")) return files; + return [ + ...files, + { + name: "_cli/restore-pgjwt.sql", + sql: "CREATE EXTENSION IF NOT EXISTS pgjwt WITH SCHEMA extensions;\n", + }, + ]; +}; + +/** User cannot edit this SQL; a persistent miss is a CLI bug. */ +const DECLARATIVE_SHADOW_PREP_FAILURE_SUGGESTION = + "This statement is CLI-owned shadow prep, not a project migration or schema file. If it persists, report it with supabase issue bug."; + +const queryError = (sql: string, cause: unknown) => + new LegacyPgDeltaEngineError({ + message: `Failed to prepare the isolated declaration shadow (${sql}): ${ + cause instanceof Error ? cause.message : String(cause) + }`, + cause, + suggestion: DECLARATIVE_SHADOW_PREP_FAILURE_SUGGESTION, + }); + +const readServerVersion = (rows: ReadonlyArray): string => { + const row = rows[0]; + if (row === undefined || typeof row !== "object" || row === null) return ""; + const value = Reflect.get(row, "server_version"); + return typeof value === "string" ? value : ""; +}; + +export const legacyPrepareDeclarativeShadow = ( + client: LegacyDeclarativeShadowClient, + files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, +) => + Effect.gen(function* () { + const declared = declaredImageExtensions(files); + if (declared.size === 0) return; + const versionRows = yield* Effect.tryPromise({ + try: () => client.query("SHOW server_version"), + catch: (cause) => queryError("SHOW server_version", cause), + }); + const statements = legacyDeclarativeBaselinePrepStatements( + legacyParsePostgresMajorVersion(readServerVersion(versionRows.rows)), + declared, + ); + for (const sql of statements) { + yield* Effect.tryPromise({ + try: () => client.query(sql), + catch: (cause) => queryError(sql, cause), + }); + } + }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts new file mode 100644 index 0000000000..255d3d3991 --- /dev/null +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, it } from "@effect/vitest"; +import { Cause, Effect, Exit, Option } from "effect"; + +import { + legacyDeclarativeBaselinePrepStatements, + legacyFilesForDeclarativeShadowLoad, + legacyParsePostgresMajorVersion, + legacyPrepareDeclarativeShadow, +} from "./legacy-pgdelta-declarative-shadow-prep.ts"; +import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; + +const allImageCreates = [ + { name: "_cluster/extensions/pgjwt.sql", sql: 'CREATE EXTENSION "pgjwt";' }, + { name: "_cluster/extensions/pgcrypto.sql", sql: 'CREATE EXTENSION "pgcrypto";' }, + { name: "_cluster/extensions/uuid-ossp.sql", sql: 'CREATE EXTENSION "uuid-ossp";' }, +]; + +describe("legacyDeclarativeBaselinePrepStatements", () => { + it("emits nothing when declarations do not recreate image defaults", () => { + expect(legacyDeclarativeBaselinePrepStatements(14, new Set())).toEqual([]); + expect(legacyDeclarativeBaselinePrepStatements(17, new Set())).toEqual([]); + }); + + it("detaches PG14 storage.objects before dropping declared uuid-ossp", () => { + expect(legacyDeclarativeBaselinePrepStatements(14, new Set(["uuid-ossp"]))).toEqual([ + "ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT", + 'DROP EXTENSION IF EXISTS "uuid-ossp"', + ]); + }); + + it("drops image pgjwt before a declared pgcrypto recreate", () => { + expect(legacyDeclarativeBaselinePrepStatements(14, new Set(["pgcrypto"]))).toEqual([ + "DROP EXTENSION IF EXISTS pgjwt", + "DROP EXTENSION IF EXISTS pgcrypto", + ]); + expect(legacyDeclarativeBaselinePrepStatements(17, new Set(["pgcrypto"]))).toEqual([ + "DROP EXTENSION IF EXISTS pgjwt", + "DROP EXTENSION IF EXISTS pgcrypto", + ]); + }); + + it("drops declared image defaults on PG15+, pgjwt before pgcrypto", () => { + expect( + legacyDeclarativeBaselinePrepStatements(17, new Set(["pgjwt", "pgcrypto", "uuid-ossp"])), + ).toEqual([ + "DROP EXTENSION IF EXISTS pgjwt", + "DROP EXTENSION IF EXISTS pgcrypto", + 'DROP EXTENSION IF EXISTS "uuid-ossp"', + ]); + }); +}); + +describe("legacyFilesForDeclarativeShadowLoad", () => { + it("restores omitted pgjwt after a pgcrypto recreate", () => { + const files = [{ name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }]; + expect(legacyFilesForDeclarativeShadowLoad(files)).toEqual([ + ...files, + { + name: "_cli/restore-pgjwt.sql", + sql: "CREATE EXTENSION IF NOT EXISTS pgjwt WITH SCHEMA extensions;\n", + }, + ]); + }); + + it("does not restore pgjwt when declarations recreate it", () => { + expect(legacyFilesForDeclarativeShadowLoad(allImageCreates)).toEqual(allImageCreates); + }); +}); + +describe("legacyParsePostgresMajorVersion", () => { + it("reads the leading major from SHOW server_version", () => { + expect(legacyParsePostgresMajorVersion("17.6")).toBe(17); + expect(legacyParsePostgresMajorVersion("14.15 (Debian)")).toBe(14); + expect(legacyParsePostgresMajorVersion("")).toBe(0); + }); +}); + +describe("legacyPrepareDeclarativeShadow", () => { + it.live("skips the shadow when declarations omit image-default extensions", () => { + const queries: string[] = []; + const client = { + query: (sql: string) => { + queries.push(sql); + return Promise.resolve({ rows: [] }); + }, + }; + return Effect.gen(function* () { + yield* legacyPrepareDeclarativeShadow(client, [ + { name: "a.sql", sql: "create table a (id int);" }, + ]); + expect(queries).toEqual([]); + }); + }); + + it.live("names the failing prep statement", () => { + const client = { + query: (sql: string) => { + if (sql === "SHOW server_version") { + return Promise.resolve({ rows: [{ server_version: "15.8" }] }); + } + if (sql.includes("pgcrypto")) { + return Promise.reject(new Error("cannot drop extension pgcrypto (SQLSTATE 2BP01)")); + } + return Promise.resolve({ rows: [] }); + }, + }; + return Effect.gen(function* () { + const exit = yield* legacyPrepareDeclarativeShadow(client, [ + { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, + ]).pipe(Effect.exit); + expect(Exit.isFailure(exit)).toBe(true); + const error = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.findErrorOption(exit.cause)) + : undefined; + expect(error).toBeInstanceOf(LegacyPgDeltaEngineError); + expect(error instanceof LegacyPgDeltaEngineError ? error.message : "").toContain( + "DROP EXTENSION IF EXISTS pgcrypto", + ); + }); + }); + + it.live("runs the version-selected prep statements against the shadow", () => { + const queries: string[] = []; + const client = { + query: (sql: string) => { + queries.push(sql); + return Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], + }); + }, + }; + return Effect.gen(function* () { + yield* legacyPrepareDeclarativeShadow(client, allImageCreates); + expect(queries).toEqual([ + "SHOW server_version", + "DROP EXTENSION IF EXISTS pgjwt", + "DROP EXTENSION IF EXISTS pgcrypto", + 'DROP EXTENSION IF EXISTS "uuid-ossp"', + ]); + }); + }); +}); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts index b925e23d40..08fe04282a 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts @@ -10,6 +10,10 @@ import { import { LegacyDbConnectError } from "../../../shared/legacy-db-connection.errors.ts"; import { legacyAcquirePgPool } from "../../../shared/legacy-db-connection.sql-pg.layer.ts"; import { LegacyDebugLogger } from "../../../shared/legacy-debug-logger.service.ts"; +import { + legacyFilesForDeclarativeShadowLoad, + legacyPrepareDeclarativeShadow, +} from "./legacy-pgdelta-declarative-shadow-prep.ts"; import { LegacyPgDeltaEngine, LegacyPgDeltaEngineError, @@ -27,11 +31,11 @@ import { legacySavePgDeltaNextDebugArtifacts, type LegacyPgDeltaNextDebugArtifacts, } from "./legacy-pgdelta-next-artifacts.ts"; -import { LegacyPgDeltaNextShadow } from "./legacy-pgdelta-next-shadow.service.ts"; import { legacyPgDeltaNextDiagnosticReport, legacyReportPgDeltaNextDiagnostics, } from "./legacy-pgdelta-next-diagnostics.ts"; +import { LegacyPgDeltaNextShadow } from "./legacy-pgdelta-next-shadow.service.ts"; function legacyPgDeltaNextConnectSuggestion(cause: unknown): string | undefined { if (cause instanceof LegacyDbConnectError) return cause.suggestion; @@ -349,10 +353,11 @@ export const legacyPgDeltaNextEngineLayer = Layer.effect( ], { concurrency: 2 }, ); + yield* legacyPrepareDeclarativeShadow(declarativePool, input.files); const result = yield* adapter.planDeclarativeSchema({ targetPool: migrationsPool, shadowPool: declarativePool, - files: input.files, + files: legacyFilesForDeclarativeShadowLoad(input.files), allowDrops: true, debug: input.debug, schema: input.schema, diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.service.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.service.ts index 7f71f6814a..837a057614 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.service.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.service.ts @@ -43,6 +43,7 @@ export interface LegacyPgDeltaExportManifest { readonly baselineDigest?: string; readonly defaultOwner?: string | null; readonly files?: ReadonlyArray; + readonly loadOrder?: ReadonlyArray; } export interface LegacyPgDeltaRenderedFile { @@ -75,7 +76,8 @@ export type LegacyPgDeltaHazardKind = | "access_exclusive_lock" | "unmodeled_kind" | "unmodeled_drift" - | "unresolved_security_label"; + | "unresolved_security_label" + | "vault_presence"; interface LegacyPgDeltaActionHazard { readonly actionIndex: number; diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-files.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-files.ts index e73738e446..91c98882d0 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-files.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-files.ts @@ -76,6 +76,7 @@ export const LegacyReadPgDeltaExportManifest = Effect.fnUntraced(function* ( const baselineDigest = readManifestValue(decoded, "baselineDigest"); const defaultOwner = readManifestValue(decoded, "defaultOwner"); const files = readManifestValue(decoded, "files"); + const loadOrder = readManifestValue(decoded, "loadOrder"); return { redactSecrets, scope, @@ -83,6 +84,9 @@ export const LegacyReadPgDeltaExportManifest = Effect.fnUntraced(function* ( ...(typeof baselineDigest === "string" ? { baselineDigest } : {}), ...(typeof defaultOwner === "string" || defaultOwner === null ? { defaultOwner } : {}), ...(Array.isArray(files) && files.every((file) => typeof file === "string") ? { files } : {}), + ...(Array.isArray(loadOrder) && loadOrder.every((file) => typeof file === "string") + ? { loadOrder } + : {}), } satisfies LegacyPgDeltaExportManifest; }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts index dec237f75c..7b490890d7 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts @@ -273,99 +273,6 @@ function legacySkippedStatementDiagnostics( })); } -function legacyIsPgDeltaNextParameterAclDiagnostic( - diagnostic: LegacyPgDeltaNextLibraryDiagnostic, -): boolean { - return diagnostic.code === "unmodeled_kind" && diagnostic.context?.["kind"] === "parameter ACL"; -} - -/** - * The parameter-ACL catalog is cluster-wide, so a co-located declarative shadow - * observes Supabase platform grants too. Keep strict coverage for every ACL - * other than the exact platform bootstrap grant while removing the aggregate - * diagnostic when that bootstrap grant is the only observed parameter ACL. - */ -export function legacyFilterPgDeltaNextPlatformParameterAclDiagnostics( - diagnostics: readonly LegacyPgDeltaNextLibraryDiagnostic[], - userOwnedParameterAcls: readonly string[], -): LegacyPgDeltaNextLibraryDiagnostic[] { - const names = [...new Set(userOwnedParameterAcls)].sort(); - const filtered: LegacyPgDeltaNextLibraryDiagnostic[] = []; - for (const diagnostic of diagnostics) { - if (!legacyIsPgDeltaNextParameterAclDiagnostic(diagnostic)) { - filtered.push(diagnostic); - continue; - } - if (names.length === 0) continue; - const samples = names.slice(0, 5); - const more = names.length > samples.length ? ", …" : ""; - filtered.push({ - ...diagnostic, - message: - `${names.length} unmodeled "parameter ACL" object${names.length === 1 ? "" : "s"} ` + - `not managed by this engine (e.g. ${samples.join(", ")}${more}) — ` + - "v1 detects but does not model this kind", - context: { kind: "parameter ACL", count: names.length, samples }, - }); - } - return filtered; -} - -interface LegacyPgDeltaNextParameterAclGrant { - readonly name: string; - readonly grantee: string; - readonly privilege: string; -} - -// Supabase's platform bootstrap grants these so privileged platform roles can -// manage the setting and the Realtime owner can replay routines whose proconfig -// contains `SET log_min_messages ...`. Parameter ACLs have cluster scope, so -// the grants are also visible from sibling shadow DBs. -const legacyPgDeltaNextPlatformParameterAcls = new Set([ - "log_min_messages\u0000supabase_admin\u0000ALTER SYSTEM", - "log_min_messages\u0000supabase_admin\u0000SET", - "log_min_messages\u0000supabase_realtime_admin\u0000SET", -]); - -function legacyPgDeltaNextParameterAclKey(grant: LegacyPgDeltaNextParameterAclGrant): string { - return `${grant.name}\u0000${grant.grantee}\u0000${grant.privilege}`; -} - -export function legacyPgDeltaNextUserOwnedParameterAcls( - grants: readonly LegacyPgDeltaNextParameterAclGrant[], -): string[] { - return [ - ...new Set( - grants - .filter( - (grant) => - !legacyPgDeltaNextPlatformParameterAcls.has(legacyPgDeltaNextParameterAclKey(grant)), - ) - .map((grant) => grant.name), - ), - ].sort(); -} - -async function legacyFilterPgDeltaNextPlatformDiagnostics( - pool: Pool, - diagnostics: readonly LegacyPgDeltaNextLibraryDiagnostic[], -): Promise[]> { - if (!diagnostics.some(legacyIsPgDeltaNextParameterAclDiagnostic)) return [...diagnostics]; - const result = await pool.query( - `SELECT DISTINCT pa.parname AS name, - COALESCE(grantee.rolname, 'PUBLIC') AS grantee, - acl.privilege_type AS privilege - FROM pg_parameter_acl pa - CROSS JOIN LATERAL aclexplode(pa.paracl) acl - LEFT JOIN pg_roles grantee ON grantee.oid = acl.grantee - ORDER BY pa.parname, grantee, privilege`, - ); - return legacyFilterPgDeltaNextPlatformParameterAclDiagnostics( - diagnostics, - legacyPgDeltaNextUserOwnedParameterAcls(result.rows), - ); -} - function legacyNormalizePgDeltaNextRenderedFiles( files: readonly LegacyPgDeltaNextLibraryRenderedFile[], ): LegacyPgDeltaNextRenderedFile[] { @@ -412,17 +319,22 @@ export function legacyPgDeltaNextProfile( return { ...supabaseProfile, policy }; } -const legacyPgDeltaNextHumanFormatOptions: SqlFormatOptions = { - keywordCase: "lower", +/** Human-readable SQL when `[experimental.pgdelta] format_options` is omitted. */ +export const legacyPgDeltaNextDefaultFormatOptions = { + keywordCase: "upper", + indent: 2, maxWidth: 180, -}; + commaStyle: "trailing", + alignColumns: true, + alignKeyValues: true, +} satisfies SqlFormatOptions; function legacyPgDeltaNextFormatOptions(raw: string | undefined): SqlFormatOptions | undefined { - if (raw === undefined || raw.trim().length === 0) return legacyPgDeltaNextHumanFormatOptions; + if (raw === undefined || raw.trim().length === 0) return legacyPgDeltaNextDefaultFormatOptions; const parsed: unknown = JSON.parse(raw); if (parsed === null) return undefined; if (typeof parsed !== "object" || Array.isArray(parsed)) { - return legacyPgDeltaNextHumanFormatOptions; + return legacyPgDeltaNextDefaultFormatOptions; } const value = (key: string): unknown => Reflect.get(parsed, key); const keywordCase = value("keywordCase"); @@ -435,7 +347,7 @@ function legacyPgDeltaNextFormatOptions(raw: string | undefined): SqlFormatOptio const preserveViewBodies = value("preserveViewBodies"); const preserveRuleBodies = value("preserveRuleBodies"); return { - ...legacyPgDeltaNextHumanFormatOptions, + ...legacyPgDeltaNextDefaultFormatOptions, ...(keywordCase === "upper" || keywordCase === "lower" || keywordCase === "preserve" ? { keywordCase } : {}), @@ -455,6 +367,14 @@ function legacyTerminatePgDeltaNextStatement(sql: string): string { return trimmed.endsWith(";") ? trimmed : `${trimmed};`; } +export function legacyFormatPgDeltaNextSql( + sql: string, + format: SqlFormatOptions | undefined, +): string { + if (format === undefined) return sql; + return `${formatSqlStatements([sql], format).map(legacyTerminatePgDeltaNextStatement).join("\n\n")}\n`; +} + function legacyFormatPgDeltaNextRenderedFiles( files: readonly LegacyPgDeltaNextLibraryRenderedFile[], format: SqlFormatOptions | undefined, @@ -462,9 +382,7 @@ function legacyFormatPgDeltaNextRenderedFiles( if (format === undefined) return files; return files.map((file) => ({ ...file, - contents: `${formatSqlStatements([file.contents], format) - .map(legacyTerminatePgDeltaNextStatement) - .join("\n\n")}\n`, + contents: legacyFormatPgDeltaNextSql(file.contents, format), })); } @@ -480,9 +398,14 @@ function legacyPgDeltaNextExportOptions(input: LegacyPgDeltaNextDeclarativeExpor function legacyPgDeltaNextPlanOptions(input: LegacyPgDeltaNextDeclarativePlanInput) { let manifest; if (input.manifest !== undefined) { - const { files, ...metadata } = input.manifest; - manifest = { ...metadata, ...(files !== undefined ? { files: [...files] } : {}) }; + const { files, loadOrder, ...metadata } = input.manifest; + manifest = { + ...metadata, + ...(files !== undefined ? { files: [...files] } : {}), + ...(loadOrder !== undefined ? { loadOrder: [...loadOrder] } : {}), + }; } + // Isolated load only — do not pin scope/redactSecrets; the sidecar owns those. return { profile: legacyPgDeltaNextProfile(input.schema), ...(manifest !== undefined ? { manifest } : {}), @@ -491,6 +414,7 @@ function legacyPgDeltaNextPlanOptions(input: LegacyPgDeltaNextDeclarativePlanInp seedAssumedSchemas: false, strictDataStatements: true, reorder: true, + connectionReuse: "reconnect-on-stuck" as const, }; } @@ -652,56 +576,26 @@ function legacyMakePgDeltaNextAdapter[2], schema?: readonly string[], - ) => { - const resolved = await resolveProfile(pool, legacyPgDeltaNextProfile(schema), options); - return { - ...resolved, - extract: async ( - extractPool: Pool, - extractOptions?: Parameters[1], - ) => { - const result = await resolved.extract(extractPool, extractOptions); - return { - ...result, - diagnostics: await legacyFilterPgDeltaNextPlatformDiagnostics( - extractPool, - result.diagnostics, - ), - }; - }, - }; - }, + ) => resolveProfile(pool, legacyPgDeltaNextProfile(schema), options), plan, renderPlanFiles, - buildSchemaExport: async (pool: Pool, input: LegacyPgDeltaNextLibraryExportOptions) => { - const result = await buildSchemaExport(pool, input); - return { - ...result, - diagnostics: await legacyFilterPgDeltaNextPlatformDiagnostics(pool, result.diagnostics), - }; - }, - planSchemaFiles: async ( + buildSchemaExport, + planSchemaFiles: ( targetPool: Pool, shadowPool: Pool, files: readonly LegacyPgDeltaNextSqlFile[], input: LegacyPgDeltaNextLibraryPlanOptions, - ) => { - const result = await planSchemaFiles( + ) => + planSchemaFiles( targetPool, shadowPool, files.map((file) => ({ name: file.name, sql: file.sql })), input, - ); - const [loadDiagnostics, targetDiagnostics] = await Promise.all([ - legacyFilterPgDeltaNextPlatformDiagnostics(shadowPool, result.loadDiagnostics), - legacyFilterPgDeltaNextPlatformDiagnostics(targetPool, result.targetDiagnostics), - ]); - return { ...result, loadDiagnostics, targetDiagnostics }; - }, + ), serializeSnapshot, serializePlan, summarizeRemovals: legacySummarizePgDeltaNextRemovals, diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.unit.test.ts index c4c47df6fe..183213e981 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.unit.test.ts @@ -7,10 +7,10 @@ import { Pool } from "pg"; import { describe, expect } from "vitest"; import { + legacyFormatPgDeltaNextSql, legacyPgDeltaNextAdapterLayerFromLibraries, - legacyFilterPgDeltaNextPlatformParameterAclDiagnostics, + legacyPgDeltaNextDefaultFormatOptions, legacyPgDeltaNextProfile, - legacyPgDeltaNextUserOwnedParameterAcls, legacySummarizePgDeltaNextHazards, legacySummarizePgDeltaNextRemovals, type LegacyPgDeltaNextLibraries, @@ -300,64 +300,17 @@ describe("LegacyPgDeltaNextAdapter", () => { }); }); - it("filters platform parameter ACL coverage without hiding user-owned ACLs", () => { - const diagnostics = [ - { - origin: "declarativeLoad" as const, - code: "unmodeled_kind", - severity: "warning" as const, - message: "2 unmodeled parameter ACLs", - context: { - kind: "parameter ACL", - count: 2, - samples: ["log_min_messages", "work_mem"], - }, - }, - { - origin: "declarativeLoad" as const, - code: "unsupported_extension", - severity: "warning" as const, - message: "extension is externally managed", - }, - ]; - - expect(legacyFilterPgDeltaNextPlatformParameterAclDiagnostics(diagnostics, [])).toEqual([ - diagnostics[1], - ]); + it("pretty-prints SQL with the CLI default options", () => { expect( - legacyFilterPgDeltaNextPlatformParameterAclDiagnostics(diagnostics, ["work_mem"]), - ).toEqual([ - { - ...diagnostics[0], - message: - '1 unmodeled "parameter ACL" object not managed by this engine (e.g. work_mem) — v1 detects but does not model this kind', - context: { kind: "parameter ACL", count: 1, samples: ["work_mem"] }, - }, - diagnostics[1], - ]); - }); - - it("recognizes only the exact Supabase platform parameter grant tuples", () => { - expect( - legacyPgDeltaNextUserOwnedParameterAcls([ - { name: "log_min_messages", grantee: "supabase_admin", privilege: "SET" }, - { name: "log_min_messages", grantee: "app_user", privilege: "SET" }, - { name: "work_mem", grantee: "supabase_realtime_admin", privilege: "SET" }, - { name: "work_mem", grantee: "app_user", privilege: "SET" }, - ]), - ).toEqual(["log_min_messages", "work_mem"]); - expect( - legacyPgDeltaNextUserOwnedParameterAcls([ - { name: "log_min_messages", grantee: "supabase_admin", privilege: "ALTER SYSTEM" }, - { name: "log_min_messages", grantee: "supabase_admin", privilege: "SET" }, - { name: "log_min_messages", grantee: "supabase_realtime_admin", privilege: "SET" }, - ]), - ).toEqual([]); - expect( - legacyPgDeltaNextUserOwnedParameterAcls([ - { name: "log_min_messages", grantee: "supabase_realtime_admin", privilege: "ALTER SYSTEM" }, - ]), - ).toEqual(["log_min_messages"]); + legacyFormatPgDeltaNextSql( + "create table public.widgets (id integer, display_name text);", + legacyPgDeltaNextDefaultFormatOptions, + ), + ).toBe(`CREATE TABLE public.widgets ( + id integer, + display_name text +); +`); }); it("renders selected-schema state without leaking other user or platform objects", () => { @@ -543,7 +496,7 @@ describe("LegacyPgDeltaNextAdapter", () => { pool: targetPool, }); expect(state.exportInputs[1]).toMatchObject({ - format: { keywordCase: "lower", maxWidth: 180 }, + format: legacyPgDeltaNextDefaultFormatOptions, }); const planned = yield* adapter.planDeclarativeSchema({ @@ -554,15 +507,24 @@ describe("LegacyPgDeltaNextAdapter", () => { allowSameDatabaseIdentity: true, debug: true, formatOptions: "null", + manifest: { + redactSecrets: true, + scope: "database", + loadOrder: ["public/tables/items.sql"], + }, }); expect(state.declarativeInputs).toHaveLength(1); expect(state.declarativeInputs[0]).toMatchObject({ reorder: true, + connectionReuse: "reconnect-on-stuck", isolatedShadow: true, allowSameDatabaseIdentity: true, seedAssumedSchemas: false, strictDataStatements: true, + manifest: { loadOrder: ["public/tables/items.sql"] }, }); + expect(state.declarativeInputs[0]).not.toHaveProperty("scope"); + expect(state.declarativeInputs[0]).not.toHaveProperty("redactSecrets"); expect(planned.diagnostics.map((diagnostic) => diagnostic.origin)).toEqual([ "declarativeLoad", "declarativeTarget", diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.ts index 8319962425..8ff3ae3882 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.ts @@ -9,10 +9,7 @@ import { } from "../../../../shared/legacy/global-flags.ts"; import { Output } from "../../../../shared/output/output.service.ts"; import { RuntimeInfo } from "../../../../shared/runtime/runtime-info.service.ts"; -import { - LegacyDbConnection, - type LegacyDbSession, -} from "../../../shared/legacy-db-connection.service.ts"; +import { LegacyDbConnection } from "../../../shared/legacy-db-connection.service.ts"; import { LegacyDockerRun } from "../../../shared/legacy-docker-run.service.ts"; import { legacyToPostgresURL } from "../../../shared/legacy-postgres-url.ts"; import { @@ -21,7 +18,6 @@ import { } from "../../../shared/db-bootstrap/local-container-inputs.ts"; import { legacyWaitForHealthyServices } from "../../../shared/db-bootstrap/health-check.ts"; import { - legacyConnectShadowDatabase, legacyCreateShadowDatabase, legacyMigrateNextShadowDatabase, legacyRemoveShadowDatabase, @@ -79,25 +75,6 @@ interface NativeShadowBase { readonly image: string; } -/** - * Removes extensions that the legacy PG14 platform baseline installs implicitly - * so the declarative shadow reflects only extension declarations in schema files. - * `pgjwt` has a hard extension dependency on `pgcrypto`, and `storage.objects.id` - * depends on `uuid-ossp`, so both dependencies must be detached before the - * user-manageable extensions can be dropped with the default RESTRICT behavior. - */ -export const legacyPreparePgDeltaNextDeclarativeBaseline = Effect.fnUntraced(function* ( - session: Pick, - majorVersion: number, -) { - if (majorVersion === 14) { - yield* session.exec("ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT"); - yield* session.exec("DROP EXTENSION IF EXISTS pgjwt"); - } - yield* session.exec("DROP EXTENSION IF EXISTS pgcrypto"); - yield* session.exec('DROP EXTENSION IF EXISTS "uuid-ossp"'); -}); - const setupRunInput = (input: NativeShadowInput, handle: LegacyShadowDatabaseHandle) => ({ fs: input.base.fs, path: input.base.path, @@ -226,15 +203,6 @@ export const legacyPgDeltaNextShadowLayer = Layer.effect( }); const setup = setupRunInput(input, handle); yield* legacySetupShadowDatabase(input.spawner, setup, { webhooks: "disabled" }); - yield* Effect.scoped( - Effect.gen(function* () { - const session = yield* legacyConnectShadowDatabase(setup.connConfig); - yield* legacyPreparePgDeltaNextDeclarativeBaseline( - session, - input.base.setup.majorVersion, - ); - }), - ); return legacyToPostgresURL(setup.connConfig); }).pipe(Effect.provide(runtime), Effect.mapError(nextShadowError)); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.unit.test.ts deleted file mode 100644 index aa40d07057..0000000000 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-shadow.layer.unit.test.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { it } from "@effect/vitest"; -import { Effect } from "effect"; -import { describe, expect } from "vitest"; - -import { legacyPreparePgDeltaNextDeclarativeBaseline } from "./legacy-pgdelta-next-shadow.layer.ts"; - -function recordingSession() { - const statements: string[] = []; - return { - statements, - session: { - exec: (sql: string) => - Effect.sync(() => { - statements.push(sql); - }), - }, - }; -} - -describe("legacyPreparePgDeltaNextDeclarativeBaseline", () => { - it.effect("detaches the PG14 platform dependencies before dropping extensions", () => { - const { session, statements } = recordingSession(); - return Effect.gen(function* () { - yield* legacyPreparePgDeltaNextDeclarativeBaseline(session, 14); - expect(statements).toEqual([ - "ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT", - "DROP EXTENSION IF EXISTS pgjwt", - "DROP EXTENSION IF EXISTS pgcrypto", - 'DROP EXTENSION IF EXISTS "uuid-ossp"', - ]); - }); - }); - - it.effect("does not modify PG15+ platform objects before dropping extensions", () => { - const { session, statements } = recordingSession(); - return Effect.gen(function* () { - yield* legacyPreparePgDeltaNextDeclarativeBaseline(session, 17); - expect(statements).toEqual([ - "DROP EXTENSION IF EXISTS pgcrypto", - 'DROP EXTENSION IF EXISTS "uuid-ossp"', - ]); - }); - }); -}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 1c4969ce34..c094bd8ffe 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -139,8 +139,8 @@ importers: specifier: workspace:* version: link:../../packages/config '@supabase/pg-delta': - specifier: 1.0.0-alpha.42 - version: 1.0.0-alpha.42(@supabase/pg-topo@1.0.0-alpha.5) + specifier: 1.0.0-alpha.46 + version: 1.0.0-alpha.46(@supabase/pg-topo@1.0.0-alpha.5) '@supabase/pg-topo': specifier: 1.0.0-alpha.5 version: 1.0.0-alpha.5 @@ -2763,8 +2763,8 @@ packages: resolution: {integrity: sha512-gfv481mTOVWtZIJgXupxZpni2V2UWPf6jeF/jOK7HdMHdH+mt6sU0sHHwf0POsPip8ltlulu9OUHgwVzl5ddRw==} engines: {node: '>=22.0.0'} - '@supabase/pg-delta@1.0.0-alpha.42': - resolution: {integrity: sha512-E1t30VEBu4ZZF6fK90iVfBT3AJTXM70XOfeMnJQ0vh9kRSuVOnVoYXjWh9/Nf/faJ2+kOCRNGKiUDQaTAbzTzQ==} + '@supabase/pg-delta@1.0.0-alpha.46': + resolution: {integrity: sha512-PaziTZjZk+zMw+wL2iBR0kJB1rOMGPCanYYYjb2pxwINCAr+XMNjnbWxTDMVZjNhPdOYLe34ocpO2lHLv+LK1A==} engines: {node: '>=20.0.0'} hasBin: true peerDependencies: @@ -8817,7 +8817,7 @@ snapshots: dependencies: tslib: 2.8.1 - '@supabase/pg-delta@1.0.0-alpha.42(@supabase/pg-topo@1.0.0-alpha.5)': + '@supabase/pg-delta@1.0.0-alpha.46(@supabase/pg-topo@1.0.0-alpha.5)': dependencies: debug: 4.4.3(supports-color@7.2.0) pg: 8.23.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 8d24ddba32..37a7b6eb5f 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -45,7 +45,7 @@ minimumReleaseAgeExclude: - "@effect/platform-node-shared@4.0.0-rc.111" - "@effect/sql-pg@4.0.0-rc.111" - "@effect/vitest@4.0.0-rc.111" - - "@supabase/pg-delta@1.0.0-alpha.42" + - "@supabase/pg-delta@1.0.0-alpha.46" - "@supabase/pg-topo@1.0.0-alpha.5" - "@types/bun@1.4.0" - "bun-types@1.4.0" From 59285c6014c0388aac40a9e01f0a96bdff18256c Mon Sep 17 00:00:00 2001 From: avallete Date: Sat, 22 Aug 2026 19:52:44 +0200 Subject: [PATCH 2/6] fix(cli): restore image pgjwt only when it was installed PG17 ships pgcrypto without pgjwt. Restoring pgjwt after every pgcrypto recreate invented a CREATE EXTENSION the project never declared. --- .../legacy-pgdelta-declarative-shadow-prep.ts | 98 +++++++++++++++++-- ...delta-declarative-shadow-prep.unit.test.ts | 71 ++++++++++++-- .../legacy-pgdelta-engine.next.layer.ts | 4 +- .../legacy-pgdelta-next-adapter.layer.ts | 38 ++++++- .../legacy-pgdelta-next-adapter.service.ts | 1 + .../legacy-pgdelta-next-adapter.unit.test.ts | 52 ++++++++++ 6 files changed, 244 insertions(+), 20 deletions(-) diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts index 1d272c1b2a..b7dc87fa22 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -6,6 +6,11 @@ export type LegacyDeclarativeShadowClient = { readonly query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>; }; +export interface LegacyDeclarativeShadowPrepResult { + /** True only when prep dropped an installed image pgjwt to recreate pgcrypto. */ + readonly restorePgjwt: boolean; +} + /** Image-default extensions the user may still declare; omit means keep the install. */ const IMAGE_DEFAULT_EXTENSIONS = ["pgjwt", "pgcrypto", "uuid-ossp"] as const; @@ -20,12 +25,68 @@ const DROP_IMAGE_DEFAULT_EXTENSION: Record<(typeof IMAGE_DEFAULT_EXTENSIONS)[num const CREATE_EXTENSION_RE = /\bCREATE\s+EXTENSION\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:"([^"]+)"|([a-zA-Z_][\w$-]*))/gi; -/** Blank comments and literals so CREATE EXTENSION in those positions is ignored. */ -const maskSqlNonCode = (sql: string): string => - sql.replaceAll( - /--[^\r\n]*|\/\*[\s\S]*?\*\/|'(?:''|[^'])*'|\$(?:[a-zA-Z_][\w$]*)?\$[\s\S]*?\$(?:[a-zA-Z_][\w$]*)?\$/g, - (matched) => matched.replaceAll(/[^\r\n]/g, " "), - ); +const DOLLAR_TAG_RE = /^\$[A-Za-z_]?[A-Za-z0-9_]*\$/; + +const blankRange = (sql: string, start: number, end: number): string => { + let blanked = ""; + for (let i = start; i < end; i++) { + const ch = sql[i]; + blanked += ch === "\n" || ch === "\r" ? ch : " "; + } + return blanked; +}; + +/** Blank comments and literals; dollar quotes close on the matching tag only. */ +const maskSqlNonCode = (sql: string): string => { + const out: string[] = []; + let i = 0; + const n = sql.length; + while (i < n) { + const c = sql[i] ?? ""; + const next = sql[i + 1]; + if (c === "-" && next === "-") { + const start = i; + while (i < n && sql[i] !== "\n") i++; + out.push(blankRange(sql, start, i)); + continue; + } + if (c === "/" && next === "*") { + const start = i; + i += 2; + while (i < n && !(sql[i] === "*" && sql[i + 1] === "/")) i++; + if (i < n) i += 2; + out.push(blankRange(sql, start, i)); + continue; + } + if (c === "'") { + const start = i; + i += 1; + while (i < n) { + if (sql[i] === "'" && sql[i + 1] === "'") i += 2; + else if (sql[i] === "'") { + i += 1; + break; + } else i += 1; + } + out.push(blankRange(sql, start, i)); + continue; + } + if (c === "$") { + const tagMatch = DOLLAR_TAG_RE.exec(sql.slice(i)); + if (tagMatch !== null) { + const tag = tagMatch[0]; + const end = sql.indexOf(tag, i + tag.length); + const stop = end === -1 ? n : end + tag.length; + out.push(blankRange(sql, i, stop)); + i = stop; + continue; + } + } + out.push(c); + i += 1; + } + return out.join(""); +}; export const legacyDeclaredSqlExtensions = ( files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, @@ -76,9 +137,9 @@ export const legacyDeclarativeBaselinePrepStatements = ( /** Recreate image pgjwt after a pgcrypto-only drop so omit still means keep. */ export const legacyFilesForDeclarativeShadowLoad = ( files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, + restorePgjwt: boolean, ): ReadonlyArray<{ readonly name: string; readonly sql: string }> => { - const declared = declaredImageExtensions(files); - if (!declared.has("pgcrypto") || declared.has("pgjwt")) return files; + if (!restorePgjwt) return files; return [ ...files, { @@ -108,13 +169,31 @@ const readServerVersion = (rows: ReadonlyArray): string => { return typeof value === "string" ? value : ""; }; +const rowHasPgjwt = (rows: ReadonlyArray): boolean => + rows.some((row) => { + if (typeof row !== "object" || row === null) return false; + const name = Reflect.get(row, "extname"); + return name === "pgjwt"; + }); + +const INSTALLED_PGJWT_SQL = "SELECT extname FROM pg_extension WHERE extname = 'pgjwt'"; + export const legacyPrepareDeclarativeShadow = ( client: LegacyDeclarativeShadowClient, files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, ) => Effect.gen(function* () { const declared = declaredImageExtensions(files); - if (declared.size === 0) return; + if (declared.size === 0) + return { restorePgjwt: false } satisfies LegacyDeclarativeShadowPrepResult; + let restorePgjwt = false; + if (declared.has("pgcrypto") && !declared.has("pgjwt")) { + const installed = yield* Effect.tryPromise({ + try: () => client.query(INSTALLED_PGJWT_SQL), + catch: (cause) => queryError(INSTALLED_PGJWT_SQL, cause), + }); + restorePgjwt = rowHasPgjwt(installed.rows); + } const versionRows = yield* Effect.tryPromise({ try: () => client.query("SHOW server_version"), catch: (cause) => queryError("SHOW server_version", cause), @@ -129,4 +208,5 @@ export const legacyPrepareDeclarativeShadow = ( catch: (cause) => queryError(sql, cause), }); } + return { restorePgjwt } satisfies LegacyDeclarativeShadowPrepResult; }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts index 255d3d3991..8c1fcbe5b4 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -3,6 +3,7 @@ import { Cause, Effect, Exit, Option } from "effect"; import { legacyDeclarativeBaselinePrepStatements, + legacyDeclaredSqlExtensions, legacyFilesForDeclarativeShadowLoad, legacyParsePostgresMajorVersion, legacyPrepareDeclarativeShadow, @@ -50,10 +51,29 @@ describe("legacyDeclarativeBaselinePrepStatements", () => { }); }); +describe("legacyDeclaredSqlExtensions", () => { + it("ignores CREATE EXTENSION inside mismatched nested dollar quotes", () => { + expect( + legacyDeclaredSqlExtensions([ + { + name: "fn.sql", + sql: `CREATE FUNCTION public.demo() RETURNS void +LANGUAGE plpgsql AS $function$ +BEGIN + PERFORM $sql$CREATE EXTENSION pgcrypto$sql$; +END; +$function$;`, + }, + ]), + ).toEqual(new Set()); + }); +}); + describe("legacyFilesForDeclarativeShadowLoad", () => { - it("restores omitted pgjwt after a pgcrypto recreate", () => { + it("restores omitted pgjwt only when prep dropped an installed image copy", () => { const files = [{ name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }]; - expect(legacyFilesForDeclarativeShadowLoad(files)).toEqual([ + expect(legacyFilesForDeclarativeShadowLoad(files, false)).toEqual(files); + expect(legacyFilesForDeclarativeShadowLoad(files, true)).toEqual([ ...files, { name: "_cli/restore-pgjwt.sql", @@ -61,10 +81,6 @@ describe("legacyFilesForDeclarativeShadowLoad", () => { }, ]); }); - - it("does not restore pgjwt when declarations recreate it", () => { - expect(legacyFilesForDeclarativeShadowLoad(allImageCreates)).toEqual(allImageCreates); - }); }); describe("legacyParsePostgresMajorVersion", () => { @@ -85,9 +101,10 @@ describe("legacyPrepareDeclarativeShadow", () => { }, }; return Effect.gen(function* () { - yield* legacyPrepareDeclarativeShadow(client, [ + const prep = yield* legacyPrepareDeclarativeShadow(client, [ { name: "a.sql", sql: "create table a (id int);" }, ]); + expect(prep.restorePgjwt).toBe(false); expect(queries).toEqual([]); }); }); @@ -130,7 +147,8 @@ describe("legacyPrepareDeclarativeShadow", () => { }, }; return Effect.gen(function* () { - yield* legacyPrepareDeclarativeShadow(client, allImageCreates); + const prep = yield* legacyPrepareDeclarativeShadow(client, allImageCreates); + expect(prep.restorePgjwt).toBe(false); expect(queries).toEqual([ "SHOW server_version", "DROP EXTENSION IF EXISTS pgjwt", @@ -139,4 +157,41 @@ describe("legacyPrepareDeclarativeShadow", () => { ]); }); }); + + it.live("restores pgjwt only when the image had it installed", () => { + const queries: string[] = []; + const client = { + query: (sql: string) => { + queries.push(sql); + if (sql.startsWith("SELECT extname")) { + return Promise.resolve({ rows: [{ extname: "pgjwt" }] }); + } + return Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "14.15" }] : [], + }); + }, + }; + return Effect.gen(function* () { + const prep = yield* legacyPrepareDeclarativeShadow(client, [ + { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, + ]); + expect(prep.restorePgjwt).toBe(true); + expect(queries[0]).toContain("pg_extension"); + }); + }); + + it.live("does not restore pgjwt on images that never installed it", () => { + const client = { + query: (sql: string) => + Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], + }), + }; + return Effect.gen(function* () { + const prep = yield* legacyPrepareDeclarativeShadow(client, [ + { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, + ]); + expect(prep.restorePgjwt).toBe(false); + }); + }); }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts index 08fe04282a..f1cdb92d45 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts @@ -353,11 +353,11 @@ export const legacyPgDeltaNextEngineLayer = Layer.effect( ], { concurrency: 2 }, ); - yield* legacyPrepareDeclarativeShadow(declarativePool, input.files); + const prep = yield* legacyPrepareDeclarativeShadow(declarativePool, input.files); const result = yield* adapter.planDeclarativeSchema({ targetPool: migrationsPool, shadowPool: declarativePool, - files: legacyFilesForDeclarativeShadowLoad(input.files), + files: legacyFilesForDeclarativeShadowLoad(input.files, prep.restorePgjwt), allowDrops: true, debug: input.debug, schema: input.schema, diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts index 7b490890d7..7b66c0f06c 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-next-adapter.layer.ts @@ -234,6 +234,27 @@ function legacyTryPgDeltaNext( }); } +function legacyIsLibraryDiagnostic( + value: unknown, +): value is LegacyPgDeltaNextLibraryDiagnostic { + if (typeof value !== "object" || value === null) return false; + const severity = Reflect.get(value, "severity"); + return ( + typeof Reflect.get(value, "code") === "string" && + typeof Reflect.get(value, "message") === "string" && + (severity === "error" || severity === "warning" || severity === "info") + ); +} + +function legacyReadPlanDiagnostics( + plan: unknown, +): readonly LegacyPgDeltaNextLibraryDiagnostic[] { + if (typeof plan !== "object" || plan === null) return []; + const diagnostics = Reflect.get(plan, "diagnostics"); + if (!Array.isArray(diagnostics)) return []; + return diagnostics.filter((diagnostic) => legacyIsLibraryDiagnostic(diagnostic)); +} + function legacyNormalizePgDeltaNextDiagnostics( diagnostics: readonly LegacyPgDeltaNextLibraryDiagnostic[], origin: LegacyPgDeltaNextDiagnosticOrigin, @@ -405,7 +426,8 @@ function legacyPgDeltaNextPlanOptions(input: LegacyPgDeltaNextDeclarativePlanInp ...(loadOrder !== undefined ? { loadOrder: [...loadOrder] } : {}), }; } - // Isolated load only — do not pin scope/redactSecrets; the sidecar owns those. + // Isolated load only. pg-delta's preflight derives scope/redactSecrets from + // the manifest and files — do not pin those here. return { profile: legacyPgDeltaNextProfile(input.schema), ...(manifest !== undefined ? { manifest } : {}), @@ -442,6 +464,7 @@ function legacyMakePgDeltaNextAdapter(generatedPlan); const diagnostics = [ ...legacyNormalizePgDeltaNextDiagnostics( source.diagnostics, @@ -453,6 +476,11 @@ function legacyMakePgDeltaNextAdapter(result.plan); const libraryDiagnostics = [ ...result.loadDiagnostics, ...result.targetDiagnostics, ...result.driftDiagnostics, + ...planDiagnostics, ]; return { changes: rendered.changes, @@ -539,6 +570,11 @@ function legacyMakePgDeltaNextAdapter { }, ); + it.effect("forwards plan-time vault_presence into the diagnostic report", () => { + const sourcePool = new Pool(); + const desiredPool = new Pool(); + const layer = legacyPgDeltaNextAdapterLayerFromLibraries({ + ...unusedLibraries, + resolveProfile: async () => ({ + id: "supabase", + planOptions: {}, + extract: async () => ({ + factBase: "facts", + pgVersion: "17.6", + diagnostics: [], + }), + }), + plan: () => ({ + source: "s", + desired: "d", + diagnostics: [fakeDiagnostic("vault_presence", "vault")], + }), + renderPlanFiles: () => ({ changes: false, files: [] }), + encodeSubject: (subject) => + typeof subject === "object" && subject !== null && "id" in subject + ? `subject:${String(Reflect.get(subject, "id"))}` + : String(subject), + summarizeHazards: () => ({ + actions: [], + dataLoss: [], + coverage: ["vault_presence"], + kinds: ["vault_presence"], + }), + }); + + return Effect.gen(function* () { + const adapter = yield* LegacyPgDeltaNextAdapter; + const result = yield* adapter.diff({ + sourcePool, + desiredPool, + allowDrops: false, + debug: false, + }); + expect(result.diagnostics).toEqual([ + expect.objectContaining({ + origin: "plan", + code: "vault_presence", + subject: "subject:vault", + }), + ]); + expect(result.hazards.kinds).toEqual(["vault_presence"]); + yield* Effect.promise(() => Promise.all([sourcePool.end(), desiredPool.end()])); + }).pipe(Effect.provide(layer)); + }); + it.effect("preserves shadow-load diagnostics in the actionable error", () => { const targetPool = new Pool(); const shadowPool = new Pool(); From d57990ece81a5c400956d92f5a3cf2bee0b97af6 Mon Sep 17 00:00:00 2001 From: avallete Date: Sat, 22 Aug 2026 20:23:05 +0200 Subject: [PATCH 3/6] fix(cli): harden declarative shadow-prep SQL and cancel Nested comments and E-strings were still leaking CREATE EXTENSION into prep drops. Checkout the shadow client so interrupt and socket errors cannot hang or crash the shared pool. --- .../legacy-pgdelta-declarative-shadow-prep.ts | 110 ++++++++-- ...delta-declarative-shadow-prep.unit.test.ts | 195 ++++++++++++++---- .../legacy-pgdelta-engine.next.layer.ts | 5 +- 3 files changed, 250 insertions(+), 60 deletions(-) diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts index b7dc87fa22..8be0e660e9 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -2,8 +2,15 @@ import { Effect } from "effect"; import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; -export type LegacyDeclarativeShadowClient = { +export type LegacyDeclarativeShadowConnection = { readonly query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>; + readonly release: (error?: Error | boolean) => void; + readonly on?: (event: "error", listener: (error: Error) => void) => void; + readonly removeListener?: (event: "error", listener: (error: Error) => void) => void; +}; + +export type LegacyDeclarativeShadowClient = { + readonly connect: () => Promise; }; export interface LegacyDeclarativeShadowPrepResult { @@ -27,6 +34,22 @@ const CREATE_EXTENSION_RE = const DOLLAR_TAG_RE = /^\$[A-Za-z_]?[A-Za-z0-9_]*\$/; +const isIdentChar = (ch: string): boolean => + (ch >= "A" && ch <= "Z") || + (ch >= "a" && ch <= "z") || + (ch >= "0" && ch <= "9") || + ch === "_" || + ch === "$"; + +/** `E'...'` / `e'...'` only when E is not part of a preceding identifier. */ +const isEscapeString = (sql: string, quoteIndex: number): boolean => { + if (quoteIndex < 1) return false; + const prefix = sql[quoteIndex - 1]; + if (prefix !== "E" && prefix !== "e") return false; + if (quoteIndex === 1) return true; + return !isIdentChar(sql[quoteIndex - 2] ?? ""); +}; + const blankRange = (sql: string, start: number, end: number): string => { let blanked = ""; for (let i = start; i < end; i++) { @@ -36,7 +59,7 @@ const blankRange = (sql: string, start: number, end: number): string => { return blanked; }; -/** Blank comments and literals; dollar quotes close on the matching tag only. */ +/** Blank comments and literals, including nested block comments and E-string escapes. */ const maskSqlNonCode = (sql: string): string => { const out: string[] = []; let i = 0; @@ -52,16 +75,29 @@ const maskSqlNonCode = (sql: string): string => { } if (c === "/" && next === "*") { const start = i; + let depth = 1; i += 2; - while (i < n && !(sql[i] === "*" && sql[i + 1] === "/")) i++; - if (i < n) i += 2; + while (i < n && depth > 0) { + if (sql[i] === "/" && sql[i + 1] === "*") { + depth += 1; + i += 2; + } else if (sql[i] === "*" && sql[i + 1] === "/") { + depth -= 1; + i += 2; + } else i += 1; + } out.push(blankRange(sql, start, i)); continue; } if (c === "'") { const start = i; + const escape = isEscapeString(sql, i); i += 1; while (i < n) { + if (escape && sql[i] === "\\") { + i += 2; + continue; + } if (sql[i] === "'" && sql[i + 1] === "'") i += 2; else if (sql[i] === "'") { i += 1; @@ -177,6 +213,57 @@ const rowHasPgjwt = (rows: ReadonlyArray): boolean => }); const INSTALLED_PGJWT_SQL = "SELECT extname FROM pg_extension WHERE extname = 'pgjwt'"; +const SHADOW_PREP_INTERRUPTED = "shadow prep interrupted"; + +/** Checkout so interrupt can discard a locked DROP instead of hanging the shared pool. */ +const queryShadow = (client: LegacyDeclarativeShadowClient, sql: string) => + Effect.callback<{ readonly rows: ReadonlyArray }, LegacyPgDeltaEngineError>((resume) => { + let settled = false; + let released = false; + let conn: LegacyDeclarativeShadowConnection | undefined; + const onError = (error: Error) => { + finish(Effect.fail(queryError(sql, error)), error); + }; + const releaseConn = (error?: Error) => { + if (released || conn === undefined) return; + released = true; + conn.removeListener?.("error", onError); + conn.release(error); + }; + const finish = ( + effect: Effect.Effect<{ readonly rows: ReadonlyArray }, LegacyPgDeltaEngineError>, + releaseError?: Error, + ) => { + if (settled) return; + settled = true; + releaseConn(releaseError); + resume(effect); + }; + void client.connect().then( + (acquired) => { + conn = acquired; + acquired.on?.("error", onError); + if (settled) { + releaseConn(new Error(SHADOW_PREP_INTERRUPTED)); + return; + } + void acquired.query(sql).then( + (result) => finish(Effect.succeed(result)), + (cause) => + finish( + Effect.fail(queryError(sql, cause)), + cause instanceof Error ? cause : new Error(String(cause)), + ), + ); + }, + (cause) => finish(Effect.fail(queryError(sql, cause))), + ); + return Effect.sync(() => { + if (settled) return; + settled = true; + releaseConn(new Error(SHADOW_PREP_INTERRUPTED)); + }); + }); export const legacyPrepareDeclarativeShadow = ( client: LegacyDeclarativeShadowClient, @@ -188,25 +275,16 @@ export const legacyPrepareDeclarativeShadow = ( return { restorePgjwt: false } satisfies LegacyDeclarativeShadowPrepResult; let restorePgjwt = false; if (declared.has("pgcrypto") && !declared.has("pgjwt")) { - const installed = yield* Effect.tryPromise({ - try: () => client.query(INSTALLED_PGJWT_SQL), - catch: (cause) => queryError(INSTALLED_PGJWT_SQL, cause), - }); + const installed = yield* queryShadow(client, INSTALLED_PGJWT_SQL); restorePgjwt = rowHasPgjwt(installed.rows); } - const versionRows = yield* Effect.tryPromise({ - try: () => client.query("SHOW server_version"), - catch: (cause) => queryError("SHOW server_version", cause), - }); + const versionRows = yield* queryShadow(client, "SHOW server_version"); const statements = legacyDeclarativeBaselinePrepStatements( legacyParsePostgresMajorVersion(readServerVersion(versionRows.rows)), declared, ); for (const sql of statements) { - yield* Effect.tryPromise({ - try: () => client.query(sql), - catch: (cause) => queryError(sql, cause), - }); + yield* queryShadow(client, sql); } return { restorePgjwt } satisfies LegacyDeclarativeShadowPrepResult; }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts index 8c1fcbe5b4..e60e709510 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -1,5 +1,6 @@ +import { EventEmitter } from "node:events"; import { describe, expect, it } from "@effect/vitest"; -import { Cause, Effect, Exit, Option } from "effect"; +import { Cause, Deferred, Effect, Exit, Fiber, Option } from "effect"; import { legacyDeclarativeBaselinePrepStatements, @@ -7,9 +8,23 @@ import { legacyFilesForDeclarativeShadowLoad, legacyParsePostgresMajorVersion, legacyPrepareDeclarativeShadow, + type LegacyDeclarativeShadowClient, } from "./legacy-pgdelta-declarative-shadow-prep.ts"; import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; +const fakeShadowClient = ( + query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>, + onRelease?: (error?: Error | boolean) => void, +): LegacyDeclarativeShadowClient => ({ + connect: () => + Promise.resolve({ + query, + release: (error?: Error | boolean) => { + onRelease?.(error); + }, + }), +}); + const allImageCreates = [ { name: "_cluster/extensions/pgjwt.sql", sql: 'CREATE EXTENSION "pgjwt";' }, { name: "_cluster/extensions/pgcrypto.sql", sql: 'CREATE EXTENSION "pgcrypto";' }, @@ -67,6 +82,44 @@ $function$;`, ]), ).toEqual(new Set()); }); + + it("ignores CREATE EXTENSION hidden by nested block comments", () => { + expect( + legacyDeclaredSqlExtensions([ + { + name: "nested.sql", + sql: "/* outer /* inner */ CREATE EXTENSION pgcrypto */", + }, + ]), + ).toEqual(new Set()); + expect( + legacyDeclaredSqlExtensions([ + { + name: "after-nested.sql", + sql: "/* /* inner */ */ CREATE EXTENSION pgcrypto;", + }, + ]), + ).toEqual(new Set(["pgcrypto"])); + }); + + it("ignores CREATE EXTENSION inside E-string escape quotes", () => { + expect( + legacyDeclaredSqlExtensions([ + { + name: "escape.sql", + sql: "SELECT E'it\\'s CREATE EXTENSION pgcrypto';", + }, + ]), + ).toEqual(new Set()); + expect( + legacyDeclaredSqlExtensions([ + { + name: "after-escape.sql", + sql: "SELECT E'it\\'s fine'; CREATE EXTENSION pgcrypto;", + }, + ]), + ).toEqual(new Set(["pgcrypto"])); + }); }); describe("legacyFilesForDeclarativeShadowLoad", () => { @@ -94,12 +147,10 @@ describe("legacyParsePostgresMajorVersion", () => { describe("legacyPrepareDeclarativeShadow", () => { it.live("skips the shadow when declarations omit image-default extensions", () => { const queries: string[] = []; - const client = { - query: (sql: string) => { - queries.push(sql); - return Promise.resolve({ rows: [] }); - }, - }; + const client = fakeShadowClient((sql) => { + queries.push(sql); + return Promise.resolve({ rows: [] }); + }); return Effect.gen(function* () { const prep = yield* legacyPrepareDeclarativeShadow(client, [ { name: "a.sql", sql: "create table a (id int);" }, @@ -110,17 +161,15 @@ describe("legacyPrepareDeclarativeShadow", () => { }); it.live("names the failing prep statement", () => { - const client = { - query: (sql: string) => { - if (sql === "SHOW server_version") { - return Promise.resolve({ rows: [{ server_version: "15.8" }] }); - } - if (sql.includes("pgcrypto")) { - return Promise.reject(new Error("cannot drop extension pgcrypto (SQLSTATE 2BP01)")); - } - return Promise.resolve({ rows: [] }); - }, - }; + const client = fakeShadowClient((sql) => { + if (sql === "SHOW server_version") { + return Promise.resolve({ rows: [{ server_version: "15.8" }] }); + } + if (sql.includes("pgcrypto")) { + return Promise.reject(new Error("cannot drop extension pgcrypto (SQLSTATE 2BP01)")); + } + return Promise.resolve({ rows: [] }); + }); return Effect.gen(function* () { const exit = yield* legacyPrepareDeclarativeShadow(client, [ { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, @@ -138,14 +187,12 @@ describe("legacyPrepareDeclarativeShadow", () => { it.live("runs the version-selected prep statements against the shadow", () => { const queries: string[] = []; - const client = { - query: (sql: string) => { - queries.push(sql); - return Promise.resolve({ - rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], - }); - }, - }; + const client = fakeShadowClient((sql) => { + queries.push(sql); + return Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], + }); + }); return Effect.gen(function* () { const prep = yield* legacyPrepareDeclarativeShadow(client, allImageCreates); expect(prep.restorePgjwt).toBe(false); @@ -160,17 +207,15 @@ describe("legacyPrepareDeclarativeShadow", () => { it.live("restores pgjwt only when the image had it installed", () => { const queries: string[] = []; - const client = { - query: (sql: string) => { - queries.push(sql); - if (sql.startsWith("SELECT extname")) { - return Promise.resolve({ rows: [{ extname: "pgjwt" }] }); - } - return Promise.resolve({ - rows: sql === "SHOW server_version" ? [{ server_version: "14.15" }] : [], - }); - }, - }; + const client = fakeShadowClient((sql) => { + queries.push(sql); + if (sql.startsWith("SELECT extname")) { + return Promise.resolve({ rows: [{ extname: "pgjwt" }] }); + } + return Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "14.15" }] : [], + }); + }); return Effect.gen(function* () { const prep = yield* legacyPrepareDeclarativeShadow(client, [ { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, @@ -181,12 +226,11 @@ describe("legacyPrepareDeclarativeShadow", () => { }); it.live("does not restore pgjwt on images that never installed it", () => { - const client = { - query: (sql: string) => - Promise.resolve({ - rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], - }), - }; + const client = fakeShadowClient((sql) => + Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], + }), + ); return Effect.gen(function* () { const prep = yield* legacyPrepareDeclarativeShadow(client, [ { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, @@ -194,4 +238,69 @@ describe("legacyPrepareDeclarativeShadow", () => { expect(prep.restorePgjwt).toBe(false); }); }); + + it.live("discards the in-flight shadow connection when prep is interrupted", () => { + return Effect.gen(function* () { + const started = Deferred.makeUnsafe(); + const released = Deferred.makeUnsafe(); + const client: LegacyDeclarativeShadowClient = { + connect: () => + Promise.resolve({ + query: () => { + Deferred.doneUnsafe(started, Effect.void); + return new Promise<{ readonly rows: ReadonlyArray }>(() => {}); + }, + release: (error?: Error | boolean) => { + Deferred.doneUnsafe( + released, + Effect.succeed(error instanceof Error ? error : undefined), + ); + }, + }), + }; + const fiber = yield* legacyPrepareDeclarativeShadow(client, [ + { name: "x.sql", sql: "CREATE EXTENSION pgcrypto;" }, + ]).pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(started); + yield* Fiber.interrupt(fiber); + const error = yield* Deferred.await(released); + expect(error?.message).toBe("shadow prep interrupted"); + }); + }); + + it.live("fails prep when the checked-out connection emits error", () => { + return Effect.gen(function* () { + const emitter = new EventEmitter(); + const started = Deferred.makeUnsafe(); + const client: LegacyDeclarativeShadowClient = { + connect: () => + Promise.resolve({ + query: () => { + Deferred.doneUnsafe(started, Effect.void); + return new Promise<{ readonly rows: ReadonlyArray }>(() => {}); + }, + release: () => {}, + on: (event, listener) => { + emitter.on(event, listener); + }, + removeListener: (event, listener) => { + emitter.removeListener(event, listener); + }, + }), + }; + const fiber = yield* legacyPrepareDeclarativeShadow(client, [ + { name: "x.sql", sql: "CREATE EXTENSION pgcrypto;" }, + ]).pipe(Effect.forkChild({ startImmediately: true })); + yield* Deferred.await(started); + emitter.emit("error", new Error("socket hang up")); + const exit = yield* Fiber.await(fiber); + expect(Exit.isFailure(exit)).toBe(true); + const error = Exit.isFailure(exit) + ? Option.getOrUndefined(Cause.findErrorOption(exit.cause)) + : undefined; + expect(error instanceof LegacyPgDeltaEngineError ? error.message : "").toContain( + "socket hang up", + ); + }); + }); }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts index f1cdb92d45..bc98be88ef 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts @@ -353,7 +353,10 @@ export const legacyPgDeltaNextEngineLayer = Layer.effect( ], { concurrency: 2 }, ); - const prep = yield* legacyPrepareDeclarativeShadow(declarativePool, input.files); + const prep = yield* legacyPrepareDeclarativeShadow( + { connect: () => declarativePool.connect() }, + input.files, + ); const result = yield* adapter.planDeclarativeSchema({ targetPool: migrationsPool, shadowPool: declarativePool, From 6af0469c46bc05922cc5ecb01832c07571fae2e0 Mon Sep 17 00:00:00 2001 From: avallete Date: Sat, 22 Aug 2026 21:49:34 +0200 Subject: [PATCH 4/6] fix(cli): skip quoted idents when scanning CREATE EXTENSION A table named "CREATE EXTENSION pgcrypto" was treated as a declaration and could drop the image default. Scan keywords while skipping "..." spans. --- .../legacy-pgdelta-declarative-shadow-prep.ts | 94 +++++++++++++++++-- ...delta-declarative-shadow-prep.unit.test.ts | 35 +++++++ 2 files changed, 123 insertions(+), 6 deletions(-) diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts index 8be0e660e9..c0f8a50492 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -29,9 +29,6 @@ const DROP_IMAGE_DEFAULT_EXTENSION: Record<(typeof IMAGE_DEFAULT_EXTENSIONS)[num "uuid-ossp": 'DROP EXTENSION IF EXISTS "uuid-ossp"', }; -const CREATE_EXTENSION_RE = - /\bCREATE\s+EXTENSION\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:"([^"]+)"|([a-zA-Z_][\w$-]*))/gi; - const DOLLAR_TAG_RE = /^\$[A-Za-z_]?[A-Za-z0-9_]*\$/; const isIdentChar = (ch: string): boolean => @@ -41,6 +38,12 @@ const isIdentChar = (ch: string): boolean => ch === "_" || ch === "$"; +const isIdentStart = (ch: string): boolean => + (ch >= "A" && ch <= "Z") || (ch >= "a" && ch <= "z") || ch === "_"; + +const isWs = (ch: string | undefined): boolean => + ch === " " || ch === "\t" || ch === "\n" || ch === "\r"; + /** `E'...'` / `e'...'` only when E is not part of a preceding identifier. */ const isEscapeString = (sql: string, quoteIndex: number): boolean => { if (quoteIndex < 1) return false; @@ -124,14 +127,93 @@ const maskSqlNonCode = (sql: string): string => { return out.join(""); }; +const matchKeyword = (sql: string, i: number, word: string): number | undefined => { + if (i > 0 && isIdentChar(sql[i - 1] ?? "")) return undefined; + const end = i + word.length; + if (sql.slice(i, end).toLowerCase() !== word) return undefined; + const next = sql[end]; + if (next !== undefined && isIdentChar(next)) return undefined; + return end; +}; + +const skipWs = (sql: string, i: number): number => { + while (isWs(sql[i])) i += 1; + return i; +}; + +const readQuotedIdent = (sql: string, start: number): { value: string; end: number } => { + let i = start + 1; + let value = ""; + while (i < sql.length) { + if (sql[i] === '"' && sql[i + 1] === '"') { + value += '"'; + i += 2; + } else if (sql[i] === '"') { + return { value, end: i + 1 }; + } else { + value += sql[i] ?? ""; + i += 1; + } + } + return { value, end: i }; +}; + +/** CREATE EXTENSION names only; skip `"CREATE EXTENSION pgcrypto"` identifiers. */ +const scanCreateExtensionNames = (sql: string): ReadonlyArray => { + const names: string[] = []; + let i = 0; + const n = sql.length; + while (i < n) { + if (sql[i] === '"') { + i = readQuotedIdent(sql, i).end; + continue; + } + const afterCreate = matchKeyword(sql, i, "create"); + if (afterCreate === undefined) { + i += 1; + continue; + } + let j = skipWs(sql, afterCreate); + const afterExt = matchKeyword(sql, j, "extension"); + if (afterExt === undefined) { + i += 1; + continue; + } + j = skipWs(sql, afterExt); + const afterIf = matchKeyword(sql, j, "if"); + if (afterIf !== undefined) { + const afterNot = matchKeyword(sql, skipWs(sql, afterIf), "not"); + if (afterNot !== undefined) { + const afterExists = matchKeyword(sql, skipWs(sql, afterNot), "exists"); + if (afterExists !== undefined) j = skipWs(sql, afterExists); + } + } + if (sql[j] === '"') { + const ident = readQuotedIdent(sql, j); + if (ident.value !== "") names.push(ident.value.toLowerCase()); + i = ident.end; + continue; + } + if (sql[j] !== undefined && isIdentStart(sql[j] ?? "")) { + const start = j; + j += 1; + while (j < n && (isIdentChar(sql[j] ?? "") || sql[j] === "-")) j += 1; + names.push(sql.slice(start, j).toLowerCase()); + i = j; + continue; + } + i = afterExt; + } + return names; +}; + export const legacyDeclaredSqlExtensions = ( files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, ): ReadonlySet => { const declared = new Set(); for (const file of files) { - for (const match of maskSqlNonCode(file.sql).matchAll(CREATE_EXTENSION_RE)) { - const name = (match[1] ?? match[2] ?? "").toLowerCase(); - if (name !== "") declared.add(name); + for (const name of scanCreateExtensionNames(maskSqlNonCode(file.sql))) { + declared.add(name); } } return declared; diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts index e60e709510..2887b92cfc 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -102,6 +102,41 @@ $function$;`, ).toEqual(new Set(["pgcrypto"])); }); + it("ignores CREATE EXTENSION inside double-quoted identifiers", () => { + expect( + legacyDeclaredSqlExtensions([ + { + name: "quoted-table.sql", + sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto" (id int);', + }, + ]), + ).toEqual(new Set()); + expect( + legacyDeclaredSqlExtensions([ + { + name: "quoted-then-real.sql", + sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto" (id int); CREATE EXTENSION pgjwt;', + }, + ]), + ).toEqual(new Set(["pgjwt"])); + expect( + legacyDeclaredSqlExtensions([ + { + name: "quoted-ext.sql", + sql: 'CREATE EXTENSION IF NOT EXISTS "uuid-ossp";', + }, + ]), + ).toEqual(new Set(["uuid-ossp"])); + expect( + legacyDeclaredSqlExtensions([ + { + name: "doubled-quote.sql", + sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto""x" (id int);', + }, + ]), + ).toEqual(new Set()); + }); + it("ignores CREATE EXTENSION inside E-string escape quotes", () => { expect( legacyDeclaredSqlExtensions([ From ec9946d8af48f8acf100bd966e814743737c0bc6 Mon Sep 17 00:00:00 2001 From: avallete Date: Sat, 22 Aug 2026 22:05:40 +0200 Subject: [PATCH 5/6] refactor(cli): drop the CREATE EXTENSION lexer Shadow prep only needs comments and simple strings stripped before a regex. Nested comments, E-strings, and quoted-ident scanning were not worth the complexity. --- .../db/schema/declarative/declarative.flow.ts | 13 +- .../legacy-pgdelta-declarative-shadow-prep.ts | 188 +----------------- ...delta-declarative-shadow-prep.unit.test.ts | 80 +------- 3 files changed, 20 insertions(+), 261 deletions(-) diff --git a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts index 28a470e511..bdf22189e6 100644 --- a/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts +++ b/apps/cli/src/legacy/commands/db/schema/declarative/declarative.flow.ts @@ -1,6 +1,9 @@ import type { LegacyPgDeltaImplementation } from "../../../../shared/legacy-pgdelta-next-flag.ts"; import { legacySchemaToCsvField } from "../../../../shared/legacy-schema-flags.ts"; -import { legacyDeclaredSqlExtensions } from "../../shared/legacy-pgdelta-declarative-shadow-prep.ts"; +import { + legacyDeclaredSqlExtensions, + legacyMaskSqlComments, +} from "../../shared/legacy-pgdelta-declarative-shadow-prep.ts"; import type { LegacyPgDeltaRemovalSummary } from "../../shared/legacy-pgdelta-engine.service.ts"; /** Extensions that legacy pg-delta treated as part of its implicit Supabase baseline. */ @@ -160,12 +163,6 @@ function matchImplicitExtension(message: string): LegacyImplicitExtensionMatch | }; } -function maskSqlComments(sql: string): string { - return sql.replaceAll(/--[^\r\n]*|\/\*[\s\S]*?\*\//g, (matched) => - matched.replaceAll(/[^\r\n]/g, " "), - ); -} - export function legacyDeclaredExtensions( files: readonly LegacyDeclarativeSqlFile[], ): ReadonlySet { @@ -190,7 +187,7 @@ function locateSignature( const diagnosticFile = files.find((file) => diagnosticMessage.startsWith(`${file.name}:`)); const candidates = diagnosticFile === undefined ? files : [diagnosticFile]; for (const file of candidates) { - const match = pattern.exec(maskSqlComments(file.sql)); + const match = pattern.exec(legacyMaskSqlComments(file.sql)); if (match?.index === undefined) continue; return { file: file.name, diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts index c0f8a50492..12edd56c93 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -29,191 +29,23 @@ const DROP_IMAGE_DEFAULT_EXTENSION: Record<(typeof IMAGE_DEFAULT_EXTENSIONS)[num "uuid-ossp": 'DROP EXTENSION IF EXISTS "uuid-ossp"', }; -const DOLLAR_TAG_RE = /^\$[A-Za-z_]?[A-Za-z0-9_]*\$/; +const CREATE_EXTENSION_RE = + /\bCREATE\s+EXTENSION\s+(?:IF\s+NOT\s+EXISTS\s+)?(?:"([^"]+)"|([a-zA-Z_][\w$-]*))/gi; -const isIdentChar = (ch: string): boolean => - (ch >= "A" && ch <= "Z") || - (ch >= "a" && ch <= "z") || - (ch >= "0" && ch <= "9") || - ch === "_" || - ch === "$"; - -const isIdentStart = (ch: string): boolean => - (ch >= "A" && ch <= "Z") || (ch >= "a" && ch <= "z") || ch === "_"; - -const isWs = (ch: string | undefined): boolean => - ch === " " || ch === "\t" || ch === "\n" || ch === "\r"; - -/** `E'...'` / `e'...'` only when E is not part of a preceding identifier. */ -const isEscapeString = (sql: string, quoteIndex: number): boolean => { - if (quoteIndex < 1) return false; - const prefix = sql[quoteIndex - 1]; - if (prefix !== "E" && prefix !== "e") return false; - if (quoteIndex === 1) return true; - return !isIdentChar(sql[quoteIndex - 2] ?? ""); -}; - -const blankRange = (sql: string, start: number, end: number): string => { - let blanked = ""; - for (let i = start; i < end; i++) { - const ch = sql[i]; - blanked += ch === "\n" || ch === "\r" ? ch : " "; - } - return blanked; -}; - -/** Blank comments and literals, including nested block comments and E-string escapes. */ -const maskSqlNonCode = (sql: string): string => { - const out: string[] = []; - let i = 0; - const n = sql.length; - while (i < n) { - const c = sql[i] ?? ""; - const next = sql[i + 1]; - if (c === "-" && next === "-") { - const start = i; - while (i < n && sql[i] !== "\n") i++; - out.push(blankRange(sql, start, i)); - continue; - } - if (c === "/" && next === "*") { - const start = i; - let depth = 1; - i += 2; - while (i < n && depth > 0) { - if (sql[i] === "/" && sql[i + 1] === "*") { - depth += 1; - i += 2; - } else if (sql[i] === "*" && sql[i + 1] === "/") { - depth -= 1; - i += 2; - } else i += 1; - } - out.push(blankRange(sql, start, i)); - continue; - } - if (c === "'") { - const start = i; - const escape = isEscapeString(sql, i); - i += 1; - while (i < n) { - if (escape && sql[i] === "\\") { - i += 2; - continue; - } - if (sql[i] === "'" && sql[i + 1] === "'") i += 2; - else if (sql[i] === "'") { - i += 1; - break; - } else i += 1; - } - out.push(blankRange(sql, start, i)); - continue; - } - if (c === "$") { - const tagMatch = DOLLAR_TAG_RE.exec(sql.slice(i)); - if (tagMatch !== null) { - const tag = tagMatch[0]; - const end = sql.indexOf(tag, i + tag.length); - const stop = end === -1 ? n : end + tag.length; - out.push(blankRange(sql, i, stop)); - i = stop; - continue; - } - } - out.push(c); - i += 1; - } - return out.join(""); -}; - -const matchKeyword = (sql: string, i: number, word: string): number | undefined => { - if (i > 0 && isIdentChar(sql[i - 1] ?? "")) return undefined; - const end = i + word.length; - if (sql.slice(i, end).toLowerCase() !== word) return undefined; - const next = sql[end]; - if (next !== undefined && isIdentChar(next)) return undefined; - return end; -}; - -const skipWs = (sql: string, i: number): number => { - while (isWs(sql[i])) i += 1; - return i; -}; - -const readQuotedIdent = (sql: string, start: number): { value: string; end: number } => { - let i = start + 1; - let value = ""; - while (i < sql.length) { - if (sql[i] === '"' && sql[i + 1] === '"') { - value += '"'; - i += 2; - } else if (sql[i] === '"') { - return { value, end: i + 1 }; - } else { - value += sql[i] ?? ""; - i += 1; - } - } - return { value, end: i }; -}; - -/** CREATE EXTENSION names only; skip `"CREATE EXTENSION pgcrypto"` identifiers. */ -const scanCreateExtensionNames = (sql: string): ReadonlyArray => { - const names: string[] = []; - let i = 0; - const n = sql.length; - while (i < n) { - if (sql[i] === '"') { - i = readQuotedIdent(sql, i).end; - continue; - } - const afterCreate = matchKeyword(sql, i, "create"); - if (afterCreate === undefined) { - i += 1; - continue; - } - let j = skipWs(sql, afterCreate); - const afterExt = matchKeyword(sql, j, "extension"); - if (afterExt === undefined) { - i += 1; - continue; - } - j = skipWs(sql, afterExt); - const afterIf = matchKeyword(sql, j, "if"); - if (afterIf !== undefined) { - const afterNot = matchKeyword(sql, skipWs(sql, afterIf), "not"); - if (afterNot !== undefined) { - const afterExists = matchKeyword(sql, skipWs(sql, afterNot), "exists"); - if (afterExists !== undefined) j = skipWs(sql, afterExists); - } - } - if (sql[j] === '"') { - const ident = readQuotedIdent(sql, j); - if (ident.value !== "") names.push(ident.value.toLowerCase()); - i = ident.end; - continue; - } - if (sql[j] !== undefined && isIdentStart(sql[j] ?? "")) { - const start = j; - j += 1; - while (j < n && (isIdentChar(sql[j] ?? "") || sql[j] === "-")) j += 1; - names.push(sql.slice(start, j).toLowerCase()); - i = j; - continue; - } - i = afterExt; - } - return names; -}; +/** Blank comments and simple strings; keep offsets for locateSignature line mapping. */ +export const legacyMaskSqlComments = (sql: string): string => + sql.replaceAll(/--[^\r\n]*|\/\*[\s\S]*?\*\/|'(?:[^']|'')*'/g, (matched) => + matched.replaceAll(/[^\r\n]/g, " "), + ); export const legacyDeclaredSqlExtensions = ( files: ReadonlyArray<{ readonly name: string; readonly sql: string }>, ): ReadonlySet => { const declared = new Set(); for (const file of files) { - for (const name of scanCreateExtensionNames(maskSqlNonCode(file.sql))) { - declared.add(name); + for (const match of legacyMaskSqlComments(file.sql).matchAll(CREATE_EXTENSION_RE)) { + const name = (match[1] ?? match[2] ?? "").toLowerCase(); + if (name !== "") declared.add(name); } } return declared; diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts index 2887b92cfc..728f8dfc0c 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -67,93 +67,23 @@ describe("legacyDeclarativeBaselinePrepStatements", () => { }); describe("legacyDeclaredSqlExtensions", () => { - it("ignores CREATE EXTENSION inside mismatched nested dollar quotes", () => { + it("ignores CREATE EXTENSION in comments and simple strings", () => { expect( legacyDeclaredSqlExtensions([ { - name: "fn.sql", - sql: `CREATE FUNCTION public.demo() RETURNS void -LANGUAGE plpgsql AS $function$ -BEGIN - PERFORM $sql$CREATE EXTENSION pgcrypto$sql$; -END; -$function$;`, - }, - ]), - ).toEqual(new Set()); - }); - - it("ignores CREATE EXTENSION hidden by nested block comments", () => { - expect( - legacyDeclaredSqlExtensions([ - { - name: "nested.sql", - sql: "/* outer /* inner */ CREATE EXTENSION pgcrypto */", + name: "commented.sql", + sql: "-- CREATE EXTENSION pgcrypto;\n/* CREATE EXTENSION pgjwt */\nselect 'create extension uuid-ossp';", }, ]), ).toEqual(new Set()); expect( legacyDeclaredSqlExtensions([ { - name: "after-nested.sql", - sql: "/* /* inner */ */ CREATE EXTENSION pgcrypto;", - }, - ]), - ).toEqual(new Set(["pgcrypto"])); - }); - - it("ignores CREATE EXTENSION inside double-quoted identifiers", () => { - expect( - legacyDeclaredSqlExtensions([ - { - name: "quoted-table.sql", - sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto" (id int);', - }, - ]), - ).toEqual(new Set()); - expect( - legacyDeclaredSqlExtensions([ - { - name: "quoted-then-real.sql", - sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto" (id int); CREATE EXTENSION pgjwt;', - }, - ]), - ).toEqual(new Set(["pgjwt"])); - expect( - legacyDeclaredSqlExtensions([ - { - name: "quoted-ext.sql", - sql: 'CREATE EXTENSION IF NOT EXISTS "uuid-ossp";', + name: "real.sql", + sql: '-- skip me\nCREATE EXTENSION IF NOT EXISTS "uuid-ossp";', }, ]), ).toEqual(new Set(["uuid-ossp"])); - expect( - legacyDeclaredSqlExtensions([ - { - name: "doubled-quote.sql", - sql: 'CREATE TABLE "CREATE EXTENSION pgcrypto""x" (id int);', - }, - ]), - ).toEqual(new Set()); - }); - - it("ignores CREATE EXTENSION inside E-string escape quotes", () => { - expect( - legacyDeclaredSqlExtensions([ - { - name: "escape.sql", - sql: "SELECT E'it\\'s CREATE EXTENSION pgcrypto';", - }, - ]), - ).toEqual(new Set()); - expect( - legacyDeclaredSqlExtensions([ - { - name: "after-escape.sql", - sql: "SELECT E'it\\'s fine'; CREATE EXTENSION pgcrypto;", - }, - ]), - ).toEqual(new Set(["pgcrypto"])); }); }); From 1387b702443e4b010302218921d201e5dca34d60 Mon Sep 17 00:00:00 2001 From: avallete Date: Sun, 23 Aug 2026 16:43:21 +0200 Subject: [PATCH 6/6] refactor(cli): drop the declarative shadow-prep checkout adapter Use pool.query again so a locked DROP waits like other sites, and keep scenario tests for the drop/restore contract. --- .../legacy-pgdelta-declarative-shadow-prep.ts | 64 +------ ...delta-declarative-shadow-prep.unit.test.ts | 165 ++++-------------- .../legacy-pgdelta-engine.next.layer.ts | 5 +- 3 files changed, 37 insertions(+), 197 deletions(-) diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts index 12edd56c93..93b399af25 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.ts @@ -2,15 +2,8 @@ import { Effect } from "effect"; import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; -export type LegacyDeclarativeShadowConnection = { - readonly query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>; - readonly release: (error?: Error | boolean) => void; - readonly on?: (event: "error", listener: (error: Error) => void) => void; - readonly removeListener?: (event: "error", listener: (error: Error) => void) => void; -}; - export type LegacyDeclarativeShadowClient = { - readonly connect: () => Promise; + readonly query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>; }; export interface LegacyDeclarativeShadowPrepResult { @@ -61,12 +54,12 @@ const declaredImageExtensions = ( return declared; }; -export const legacyParsePostgresMajorVersion = (serverVersion: string): number => { +const legacyParsePostgresMajorVersion = (serverVersion: string): number => { const major = Number.parseInt(serverVersion, 10); return Number.isInteger(major) ? major : 0; }; -export const legacyDeclarativeBaselinePrepStatements = ( +const legacyDeclarativeBaselinePrepStatements = ( majorVersion: number, declared: ReadonlySet, ): ReadonlyArray => { @@ -127,56 +120,11 @@ const rowHasPgjwt = (rows: ReadonlyArray): boolean => }); const INSTALLED_PGJWT_SQL = "SELECT extname FROM pg_extension WHERE extname = 'pgjwt'"; -const SHADOW_PREP_INTERRUPTED = "shadow prep interrupted"; -/** Checkout so interrupt can discard a locked DROP instead of hanging the shared pool. */ const queryShadow = (client: LegacyDeclarativeShadowClient, sql: string) => - Effect.callback<{ readonly rows: ReadonlyArray }, LegacyPgDeltaEngineError>((resume) => { - let settled = false; - let released = false; - let conn: LegacyDeclarativeShadowConnection | undefined; - const onError = (error: Error) => { - finish(Effect.fail(queryError(sql, error)), error); - }; - const releaseConn = (error?: Error) => { - if (released || conn === undefined) return; - released = true; - conn.removeListener?.("error", onError); - conn.release(error); - }; - const finish = ( - effect: Effect.Effect<{ readonly rows: ReadonlyArray }, LegacyPgDeltaEngineError>, - releaseError?: Error, - ) => { - if (settled) return; - settled = true; - releaseConn(releaseError); - resume(effect); - }; - void client.connect().then( - (acquired) => { - conn = acquired; - acquired.on?.("error", onError); - if (settled) { - releaseConn(new Error(SHADOW_PREP_INTERRUPTED)); - return; - } - void acquired.query(sql).then( - (result) => finish(Effect.succeed(result)), - (cause) => - finish( - Effect.fail(queryError(sql, cause)), - cause instanceof Error ? cause : new Error(String(cause)), - ), - ); - }, - (cause) => finish(Effect.fail(queryError(sql, cause))), - ); - return Effect.sync(() => { - if (settled) return; - settled = true; - releaseConn(new Error(SHADOW_PREP_INTERRUPTED)); - }); + Effect.tryPromise({ + try: () => client.query(sql), + catch: (cause) => queryError(sql, cause), }); export const legacyPrepareDeclarativeShadow = ( diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts index 728f8dfc0c..1ed272209d 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-declarative-shadow-prep.unit.test.ts @@ -1,12 +1,9 @@ -import { EventEmitter } from "node:events"; import { describe, expect, it } from "@effect/vitest"; -import { Cause, Deferred, Effect, Exit, Fiber, Option } from "effect"; +import { Cause, Effect, Exit, Option } from "effect"; import { - legacyDeclarativeBaselinePrepStatements, legacyDeclaredSqlExtensions, legacyFilesForDeclarativeShadowLoad, - legacyParsePostgresMajorVersion, legacyPrepareDeclarativeShadow, type LegacyDeclarativeShadowClient, } from "./legacy-pgdelta-declarative-shadow-prep.ts"; @@ -14,16 +11,7 @@ import { LegacyPgDeltaEngineError } from "./legacy-pgdelta-engine.service.ts"; const fakeShadowClient = ( query: (sql: string) => Promise<{ readonly rows: ReadonlyArray }>, - onRelease?: (error?: Error | boolean) => void, -): LegacyDeclarativeShadowClient => ({ - connect: () => - Promise.resolve({ - query, - release: (error?: Error | boolean) => { - onRelease?.(error); - }, - }), -}); +): LegacyDeclarativeShadowClient => ({ query }); const allImageCreates = [ { name: "_cluster/extensions/pgjwt.sql", sql: 'CREATE EXTENSION "pgjwt";' }, @@ -31,41 +19,6 @@ const allImageCreates = [ { name: "_cluster/extensions/uuid-ossp.sql", sql: 'CREATE EXTENSION "uuid-ossp";' }, ]; -describe("legacyDeclarativeBaselinePrepStatements", () => { - it("emits nothing when declarations do not recreate image defaults", () => { - expect(legacyDeclarativeBaselinePrepStatements(14, new Set())).toEqual([]); - expect(legacyDeclarativeBaselinePrepStatements(17, new Set())).toEqual([]); - }); - - it("detaches PG14 storage.objects before dropping declared uuid-ossp", () => { - expect(legacyDeclarativeBaselinePrepStatements(14, new Set(["uuid-ossp"]))).toEqual([ - "ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT", - 'DROP EXTENSION IF EXISTS "uuid-ossp"', - ]); - }); - - it("drops image pgjwt before a declared pgcrypto recreate", () => { - expect(legacyDeclarativeBaselinePrepStatements(14, new Set(["pgcrypto"]))).toEqual([ - "DROP EXTENSION IF EXISTS pgjwt", - "DROP EXTENSION IF EXISTS pgcrypto", - ]); - expect(legacyDeclarativeBaselinePrepStatements(17, new Set(["pgcrypto"]))).toEqual([ - "DROP EXTENSION IF EXISTS pgjwt", - "DROP EXTENSION IF EXISTS pgcrypto", - ]); - }); - - it("drops declared image defaults on PG15+, pgjwt before pgcrypto", () => { - expect( - legacyDeclarativeBaselinePrepStatements(17, new Set(["pgjwt", "pgcrypto", "uuid-ossp"])), - ).toEqual([ - "DROP EXTENSION IF EXISTS pgjwt", - "DROP EXTENSION IF EXISTS pgcrypto", - 'DROP EXTENSION IF EXISTS "uuid-ossp"', - ]); - }); -}); - describe("legacyDeclaredSqlExtensions", () => { it("ignores CREATE EXTENSION in comments and simple strings", () => { expect( @@ -101,14 +54,6 @@ describe("legacyFilesForDeclarativeShadowLoad", () => { }); }); -describe("legacyParsePostgresMajorVersion", () => { - it("reads the leading major from SHOW server_version", () => { - expect(legacyParsePostgresMajorVersion("17.6")).toBe(17); - expect(legacyParsePostgresMajorVersion("14.15 (Debian)")).toBe(14); - expect(legacyParsePostgresMajorVersion("")).toBe(0); - }); -}); - describe("legacyPrepareDeclarativeShadow", () => { it.live("skips the shadow when declarations omit image-default extensions", () => { const queries: string[] = []; @@ -170,102 +115,52 @@ describe("legacyPrepareDeclarativeShadow", () => { }); }); - it.live("restores pgjwt only when the image had it installed", () => { + it.live("detaches PG14 storage.objects before dropping declared uuid-ossp", () => { const queries: string[] = []; const client = fakeShadowClient((sql) => { queries.push(sql); - if (sql.startsWith("SELECT extname")) { - return Promise.resolve({ rows: [{ extname: "pgjwt" }] }); - } return Promise.resolve({ rows: sql === "SHOW server_version" ? [{ server_version: "14.15" }] : [], }); }); return Effect.gen(function* () { - const prep = yield* legacyPrepareDeclarativeShadow(client, [ - { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, + yield* legacyPrepareDeclarativeShadow(client, [ + { name: "uuid.sql", sql: 'CREATE EXTENSION "uuid-ossp";' }, + ]); + expect(queries).toEqual([ + "SHOW server_version", + "ALTER TABLE storage.objects ALTER COLUMN id DROP DEFAULT", + 'DROP EXTENSION IF EXISTS "uuid-ossp"', ]); - expect(prep.restorePgjwt).toBe(true); - expect(queries[0]).toContain("pg_extension"); }); }); - it.live("does not restore pgjwt on images that never installed it", () => { - const client = fakeShadowClient((sql) => + it.live("restores pgjwt only when the image had it installed", () => { + const queries: string[] = []; + const withPgjwt = fakeShadowClient((sql) => { + queries.push(sql); + if (sql.startsWith("SELECT extname")) { + return Promise.resolve({ rows: [{ extname: "pgjwt" }] }); + } + return Promise.resolve({ + rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], + }); + }); + const withoutPgjwt = fakeShadowClient((sql) => Promise.resolve({ rows: sql === "SHOW server_version" ? [{ server_version: "17.6" }] : [], }), ); + const files = [{ name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }]; return Effect.gen(function* () { - const prep = yield* legacyPrepareDeclarativeShadow(client, [ - { name: "public/01.sql", sql: "CREATE EXTENSION pgcrypto;" }, + expect((yield* legacyPrepareDeclarativeShadow(withPgjwt, files)).restorePgjwt).toBe(true); + expect(queries).toEqual([ + "SELECT extname FROM pg_extension WHERE extname = 'pgjwt'", + "SHOW server_version", + "DROP EXTENSION IF EXISTS pgjwt", + "DROP EXTENSION IF EXISTS pgcrypto", ]); - expect(prep.restorePgjwt).toBe(false); - }); - }); - - it.live("discards the in-flight shadow connection when prep is interrupted", () => { - return Effect.gen(function* () { - const started = Deferred.makeUnsafe(); - const released = Deferred.makeUnsafe(); - const client: LegacyDeclarativeShadowClient = { - connect: () => - Promise.resolve({ - query: () => { - Deferred.doneUnsafe(started, Effect.void); - return new Promise<{ readonly rows: ReadonlyArray }>(() => {}); - }, - release: (error?: Error | boolean) => { - Deferred.doneUnsafe( - released, - Effect.succeed(error instanceof Error ? error : undefined), - ); - }, - }), - }; - const fiber = yield* legacyPrepareDeclarativeShadow(client, [ - { name: "x.sql", sql: "CREATE EXTENSION pgcrypto;" }, - ]).pipe(Effect.forkChild({ startImmediately: true })); - yield* Deferred.await(started); - yield* Fiber.interrupt(fiber); - const error = yield* Deferred.await(released); - expect(error?.message).toBe("shadow prep interrupted"); - }); - }); - - it.live("fails prep when the checked-out connection emits error", () => { - return Effect.gen(function* () { - const emitter = new EventEmitter(); - const started = Deferred.makeUnsafe(); - const client: LegacyDeclarativeShadowClient = { - connect: () => - Promise.resolve({ - query: () => { - Deferred.doneUnsafe(started, Effect.void); - return new Promise<{ readonly rows: ReadonlyArray }>(() => {}); - }, - release: () => {}, - on: (event, listener) => { - emitter.on(event, listener); - }, - removeListener: (event, listener) => { - emitter.removeListener(event, listener); - }, - }), - }; - const fiber = yield* legacyPrepareDeclarativeShadow(client, [ - { name: "x.sql", sql: "CREATE EXTENSION pgcrypto;" }, - ]).pipe(Effect.forkChild({ startImmediately: true })); - yield* Deferred.await(started); - emitter.emit("error", new Error("socket hang up")); - const exit = yield* Fiber.await(fiber); - expect(Exit.isFailure(exit)).toBe(true); - const error = Exit.isFailure(exit) - ? Option.getOrUndefined(Cause.findErrorOption(exit.cause)) - : undefined; - expect(error instanceof LegacyPgDeltaEngineError ? error.message : "").toContain( - "socket hang up", - ); + expect((yield* legacyPrepareDeclarativeShadow(withoutPgjwt, files)).restorePgjwt).toBe(false); }); }); }); diff --git a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts index bc98be88ef..f1cdb92d45 100644 --- a/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts +++ b/apps/cli/src/legacy/commands/db/shared/legacy-pgdelta-engine.next.layer.ts @@ -353,10 +353,7 @@ export const legacyPgDeltaNextEngineLayer = Layer.effect( ], { concurrency: 2 }, ); - const prep = yield* legacyPrepareDeclarativeShadow( - { connect: () => declarativePool.connect() }, - input.files, - ); + const prep = yield* legacyPrepareDeclarativeShadow(declarativePool, input.files); const result = yield* adapter.planDeclarativeSchema({ targetPool: migrationsPool, shadowPool: declarativePool,