fix(cli): skip pg_dump in db pull when using pg-delta diff engine#5255
Open
avallete wants to merge 19 commits into
Open
fix(cli): skip pg_dump in db pull when using pg-delta diff engine#5255avallete wants to merge 19 commits into
avallete wants to merge 19 commits into
Conversation
Initial pulls under --diff-engine pg-delta were dumping the remote schema via pg_dump before running pg-delta. The dump-then-restore round-trip strips ownership information for objects the local postgres role cannot assume, so platform-managed objects (FDWs, wasm wrappers, system-owned ACLs) leak into the migration file and break \`supabase db reset\` (see CLI-1469, CLI-1470). pg-delta speaks pg_catalog directly via extractCatalog and the supabase integration filters platform objects by owner. Diffing against an empty shadow on initial pull yields a clean initial migration on its own, so dumpRemoteSchema is unnecessary on this path.
Coverage Report for CI Build 27024156346Warning No base build found for commit Coverage: 64.279%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
Added documentation for testing local pg-delta builds in CONTRIBUTING.md, detailing the steps to publish local changes and configure the CLI to use a local npm registry. Updated the RunEdgeRuntimeScript function to accept additional options for handling npm registry configurations, allowing for scoped .npmrc files and environment variable forwarding. Introduced PgDeltaNpmRegistryOption to manage npm registry settings and added tests to ensure correct behavior. Refactored related functions to integrate these enhancements, improving the overall local development experience for pg-delta.
Resolve edgeruntime conflicts by combining EdgeRuntimeOption support with dynamic port allocation from develop (#5407).
- Renamed test cases for clarity and updated assertions to reflect new behavior regarding the use of the pg-delta flag and experimental config. - Modified `shouldUseDeclarativePgDeltaPull` function to remove reliance on the `usePgDelta` variable. - Enhanced debug capture logic in the diffing process to ensure proper handling of empty diffs. - Updated TypeScript template to use the latest pg-delta version (1.0.0-alpha.25). - Adjusted comments and formatting in various files for consistency and clarity.
- Updated the golangci-lint configuration to correct the exclusion format for the ST1003 rule. - Removed the unused `diffWithStream` function from `internal/db/diff/diff.go` to clean up the codebase.
Supabase CLI previewnpx --yes https://pkg.pr.new/supabase@5255Preview package for commit |
Review follow-ups for PR #5255: - Give SOURCE and TARGET distinct on-disk CA bundle filenames (pgdelta-source-ca.crt vs pgdelta-target-ca.crt) so a diff between two remote databases with different CAs cannot share one file. caBundleFilename derives the name from the env var. - Replace strings.Contains(host, "pooler.supabase.com") with an exact match plus a ".pooler.supabase.com" suffix check so look-alike hostnames (e.g. pooler.supabase.com.example.org) do not match. - Unexport the test-only EnsurePgDeltaVerifyCA helper; tests now exercise ensurePgDeltaSSL directly with an empty sslrootcert. - Rename pull.exportCatalogPgDelta to pull.exportTargetCatalog to distinguish it from the diff package's identically named test seam and to reflect that it captures the remote (target) catalog for the empty-pull debug bundle.
The build:go-sidecar step runs first in the build chain, before any bun --compile step has created dist/, so `cp ../cli-go/supabase-go dist/supabase-go` failed with "No such file or directory" and broke nx run supabase:build (and every e2e shard that depends on it). Prefix the copy with `mkdir -p dist` so the destination exists regardless of step order; it is idempotent when dist/ already exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Switches
supabase db pull --diff-engine pg-deltato drive its initial pull from pg-delta's catalog diff instead ofpg_dump, and lands the supporting plumbing (remote TLS handling, debug bundles, and a local pg-delta dev workflow) needed to make that path reliable against Cloud.Closes CLI-1472.
Why
The legacy initial-pull pipeline is
pg_dump→ restore into a local shadow → diff. That round-trip silently destroys ownership for any object the localpostgresrole can't assume:pg_dumpemitsALTER ... OWNER TO supabase_admin(etc.), the restore runs as a non-superuser so the owner is dropped, and every restored object then reportsowner = postgres. pg-delta's supabase integration filter drops platform-managed objects byowner ∈ SUPABASE_SYSTEM_ROLES, so once the owner signal is gone those objects leak into the user's migration file and breaksupabase db reset(CLI-1469 FDW ACLs, CLI-1470 Wasm wrappers).pg-delta already speaks
pg_catalogdirectly viaextractCatalog, preserving ownership, so diffing the remote catalog against an empty shadow produces a clean initial migration withoutpg_dumpin the loop.What changed
Initial-pull routing
db pullno longer runsdumpRemoteSchemaon the initial pull when the pg-delta diff engine is selected; the shadow diff is the sole producer of the migration file (internal/db/pull/pull.go,cmd/db.go).shouldUseDeclarativePgDeltaPullmakes explicit--diff-engine pg-deltakeep the migration-file workflow even when[experimental.pgdelta] enabled = truewould otherwise default to the declarative export path.swallowInitialInSync/ensureMigrationWrittenhelpers distinguish "pg-delta produced no statements" from "pg_dump already wrote content", so an empty pg-delta result still surfacesNo schema changes foundcorrectly.Diff plumbing
DiffDatabasenow returns aDatabaseDiffstruct (SQLplus an optional debug capture) rather than a bare string; callers indiff.goandpull.goupdated accordingly.diffWithStreamand its golangci exclusion.templates/pgdelta.ts) setsskipDefaultPrivilegeSubtraction: trueand emits structured diagnostics underPGDELTA_DEBUG.Remote TLS handling for pg-delta
internal/gen/types/pgdelta_conn.go:PreparePgDeltaPostgresRefwrites the CA bundle into the workspace, forcessslmode=verify-ca+sslrootcert, and always uses the embedded bundle for Supabase-hosted hosts even when the SSL probe is skipped (e.g.--debug). Source/target now use distinctPGDELTA_SOURCE_SSLROOTCERT/PGDELTA_TARGET_SSLROOTCERTenv vars.pgdelta.goandpgcache/cache.gorefactored onto this helper.Debug bundles for empty pulls
PGDELTA_DEBUGflag (separate from--debug; keeps SSL on). When a pg-delta pull returns zero statements, the CLI writes a bundle undersupabase/.temp/pgdelta/debug/<timestamp>/:source-catalog.json,target-catalog.json,pgdelta-stderr.txt,connection.txt(redacted), anderror.txt, plus a per-schema catalog object summary to stderr (internal/db/diff/pgdelta_debug.go,internal/db/pull/pgdelta_pull_debug.go).DiffPgDeltaRefDetailedsurfaces edge-runtime stderr;declarative.DebugBundlegains inline-catalog, stderr, and connection-info fields.Local pg-delta dev workflow
PGDELTA_NPM_REGISTRY(pkg/config/pgdelta_local.go,internal/utils/pgdelta_local.go) routes Deno'snpm:@supabase/pg-deltaresolution through a local Verdaccio registry. The edge-runtime helper gained composableWithExtraFile/WithExtraEnvoptions to drop a scoped.npmrcand forwardNPM_CONFIG_REGISTRY; all pg-delta edge-runtime calls now passPgDeltaNpmRegistryOption(). Documented inCONTRIBUTING.md.Version, build, and docs
DefaultPgDeltaNpmVersionbumped to1.0.0-alpha.27.apps/cli/package.jsonadds abuild:go-sidecarstep that copies the Go binary intodist/.docs/supabase/db/pull.mddocuments the new initial-pull behavior, the experimental declarative default, the direct-connection recommendation for--db-url, and thePGDELTA_DEBUGworkflow.Reviewer notes
CREATE FOREIGN DATA WRAPPERsuppression rules in@supabase/pg-deltabecome defense-in-depth on this path rather than load-bearing.supabase db dumpand the declarative export path still usepg_dump/ the existing flows.https://claude.ai/code/session_016o7U7hiut6dkkhfbSJgKv4