Skip to content

feat(cli): upgrade pg-delta next to alpha.46 - #6300

Open
avallete wants to merge 5 commits into
developfrom
feat/upgrade-pg-delta-alpha.46
Open

feat(cli): upgrade pg-delta next to alpha.46#6300
avallete wants to merge 5 commits into
developfrom
feat/upgrade-pg-delta-alpha.46

Conversation

@avallete

Copy link
Copy Markdown
Member

Summary

  • Bump @supabase/pg-delta from 1.0.0-alpha.42 to 1.0.0-alpha.46 so db diff, db pull, and db schema declarative generate pick up supabase-profile parameter-ACL filtering, OWNED BY with the owning table, per-statement load fallback, vault_presence, and reconnect-on-stuck load assist.
  • Delete the CLI copy of platform parameter-ACL filtering; the engine profile now owns that coverage.
  • Pretty-print generated SQL by default (uppercase keywords, indent 2, aligned columns).
  • Prepare declarative shadows only when schema files recreate image defaults (pgjwt / pgcrypto / uuid-ossp). Unconditional drops were planning destructive DROP EXTENSION and writing CREATE EXTENSION that db start cannot replay.

Extracted from #6274 so the engine upgrade can land on develop without the schema-first command stack.

Linked issue

Supabase maintainer change; no public issue to close.

  • The linked issue is open and carries the open-for-contribution label (or I'm a Supabase maintainer).

Checklist

  • The PR title follows Conventional Commits (e.g. fix(cli): …).
  • Tests added or updated for the change.
  • pnpm check:all and pnpm test pass for the workspace(s) I touched.

Consume OWNED BY / load assist, drop the CLI parameter-ACL workaround,
pretty-print generated SQL, and only strip image extensions when
declarations recreate them.
@avallete
avallete requested a review from a team as a code owner August 22, 2026 17:37

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f387bcc80e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +24 to +27
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, " "),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Parse PostgreSQL lexical forms before detecting extensions

When a schema file contains PostgreSQL constructs this regex does not understand—such as nested block comments or E'...' strings with backslash-escaped quotes—it can expose a commented/string-literal CREATE EXTENSION pgcrypto to the following matcher. The prep then drops the image-default extension even though the file never recreates it, so a manifest-backed declarative sync can plan an unintended extension removal. Use a PostgreSQL-aware tokenizer, or at least correctly mask nested comments and escape strings, before deciding which defaults to drop.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d57990e. The masker now nests block comments and treats E'...' / e'...' backslash escapes as literals before CREATE EXTENSION detection.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up in ec9946d: we dropped nested-comment / E-string / dollar-quote lexing. Detection is now -- / /* */ / simple '...' plus a regex. Pathological SQL is an accepted miss.

Comment on lines +127 to +130
yield* Effect.tryPromise({
try: () => client.query(sql),
catch: (cause) => queryError(sql, cause),
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Make shadow-prep queries interruptible

When a DROP EXTENSION query blocks on a database lock, interrupting the command only interrupts the Effect: this tryPromise wrapper neither passes a cancellation signal nor installs cleanup for the in-flight pg query. Because the pool cannot release that connection until the query settles, Ctrl-C and sibling-fiber failure can leave scoped teardown hanging behind the blocked statement; adapt the query boundary so interruption cancels or closes the exact owned operation.

AGENTS.md reference: AGENTS.md:L68-L72

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d57990e. Prep checks out a pool client and release(error) on interrupt or socket error so a locked DROP cannot hang or crash the shared pool.

@github-actions

github-actions Bot commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Supabase CLI preview

npx --yes https://pkg.pr.new/supabase/cli/supabase@ec9946d8af48f8acf100bd966e814743737c0bc6

Preview package for commit ec9946d.

PG17 ships pgcrypto without pgjwt. Restoring pgjwt after every pgcrypto
recreate invented a CREATE EXTENSION the project never declared.
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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d57990ece8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

): ReadonlySet<string> => {
const declared = new Set<string>();
for (const file of files) {
for (const match of maskSqlNonCode(file.sql).matchAll(CREATE_EXTENSION_RE)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Mask double-quoted identifiers before scanning extensions

Although the final tree now handles nested comments and escape strings, the masker still copies double-quoted identifiers verbatim. A valid declaration such as CREATE TABLE "CREATE EXTENSION pgcrypto" (id int); therefore matches this regex as if it declared pgcrypto; declarative shadow prep then drops the image-default extension even though no file recreates it, allowing the resulting sync plan to propose an unintended extension removal. Mask PostgreSQL quoted identifiers, including doubled quotes, before matching.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 6af0469. CREATE EXTENSION names are scanned while skipping double-quoted identifiers (including "" escapes), so a table named "CREATE EXTENSION pgcrypto" is not treated as a declaration. Quoted names like "uuid-ossp" still count.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted the quoted-ident scanner in ec9946d. CREATE EXTENSION detection is now comments + simple strings + a regex. A table named "CREATE EXTENSION pgcrypto" is an accepted false positive — not a real schema-file case.

A table named "CREATE EXTENSION pgcrypto" was treated as a declaration
and could drop the image default. Scan keywords while skipping "..." spans.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6af0469c46

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +113 to +114
if (c === "$") {
const tagMatch = DOLLAR_TAG_RE.exec(sql.slice(i));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Require a token boundary before masking dollar quotes

PostgreSQL permits $ inside unquoted identifiers, but this branch treats every $tag$ substring as a dollar-quote opener. For example, in the valid SQL CREATE TABLE foo$tag$(id int); CREATE EXTENSION pgcrypto; SELECT $tag$body$tag$;, masking starts inside foo$tag$ and consumes through the later delimiter, hiding the real extension declaration. Fresh evidence after the earlier lexical fixes is that the final scanner still lacks a preceding-identifier boundary check here; consequently shadow prep leaves the image extension installed, causing an unguarded declaration to fail or an IF NOT EXISTS ... WITH SCHEMA/VERSION declaration to silently retain the wrong baseline state.

AGENTS.md reference: apps/cli/AGENTS.md:L52-L53

Useful? React with 👍 / 👎.

Comment on lines +359 to +361
if (declared.has("pgcrypto") && !declared.has("pgjwt")) {
const installed = yield* queryShadow(client, INSTALLED_PGJWT_SQL);
restorePgjwt = rowHasPgjwt(installed.rows);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor explicit pgjwt drops before scheduling restoration

When a PG14 declarative tree recreates pgcrypto but also explicitly runs DROP EXTENSION IF EXISTS pgjwt—for example, custom SQL added alongside an exported pgcrypto declaration—declared contains pgcrypto but never records the DROP. This condition therefore marks the installed baseline pgjwt for restoration; prep removes it, the user's guarded DROP becomes a no-op, and the synthetic tail file recreates it, so the desired shadow still contains pgjwt and sync cannot plan the requested removal. Treat an explicit pgjwt DROP as non-restorable.

AGENTS.md reference: apps/cli/AGENTS.md:L52-L53

Useful? React with 👍 / 👎.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant