fix(cli): rewrite pooler to direct for initial pg-delta db pull#5833
fix(cli): rewrite pooler to direct for initial pg-delta db pull#5833mittal-parth wants to merge 1 commit into
Conversation
Initial db pull with pg-delta skips pg_dump and introspects the remote catalog via the diff TARGET. When link resolved to the pooler, pg-delta often saw an empty catalog and reported "No schema changes found". Rewrite the pg-delta TARGET to db.<ref>.<host> on initial linked pulls, probe the remote catalog when the diff is empty, and gate IPv6 pooler fallback on the container target host in the legacy handler.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6243efe74f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| direct := config | ||
| direct.Host = GetSupabaseDbHost(projectRef) | ||
| direct.Port = 5432 | ||
| direct.User = "postgres" |
There was a problem hiding this comment.
When NewDbConfigWithPassword chooses the pooler because the direct host is unreachable and no SUPABASE_DB_PASSWORD was provided, initPoolerLogin stores a temporary login role/password in this config. Overwriting User with postgres while keeping that temp-role password makes the rewritten direct pg-delta target fail authentication, so the initial pull path this change is meant to fix still breaks for no-password linked users on IPv4-only host networks; preserve the resolved login role (removing only pooler routing/suffix as needed) unless the password is known to be the postgres DB password.
Useful? React with 👍 / 👎.
| ...rest, | ||
| host: `db.${projectRef}.${projectHost}`, | ||
| port: 5432, | ||
| user: "postgres", |
There was a problem hiding this comment.
In the linked fallback path without SUPABASE_DB_PASSWORD, the resolver mints a temporary login role and returns a pooler connection whose password belongs to that role, not to postgres. This rewrite changes only the target URL to direct but also forces user: "postgres", pairing the temp-role password with the wrong user and causing initial pg-delta pulls to fail authentication in the IPv4/pooler fallback scenario; strip the pooler tenant routing/suffix while keeping the resolved role, or only force postgres when the password is actually the database password.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| func diffRemoteSchema(ctx context.Context, schema []string, path string, config pgconn.Config, usePgDeltaDiff bool, differ diff.DiffFunc, fsys afero.Fs, options ...func(*pgx.ConnConfig)) error { | ||
| initialPull := len(schema) == 0 |
There was a problem hiding this comment.
A normal non-initial db pull --diff-engine pg-delta with no --schema also calls this function with schema == [], so this now treats that common path as an initial pull. If the diff is empty and the remote catalog has objects, handleInitialPgDeltaEmptyPull reports the new pg-delta introspection failure instead of the correct “No schema changes found”; pass the sync/initial state explicitly rather than deriving it from the schema filter.
Useful? React with 👍 / 👎.
| // IsPoolerDbHost reports whether host is a Supabase connection pooler endpoint. | ||
| func IsPoolerDbHost(host string) bool { | ||
| host = strings.ToLower(host) | ||
| return host == "pooler.supabase.com" || strings.HasSuffix(host, ".pooler.supabase.com") |
There was a problem hiding this comment.
This only recognizes production *.pooler.supabase.com hosts, but the repo supports other built-in profiles whose validated pooler domains are not supabase.com (for example supabase-staging uses supabase.green, and custom profiles can set their own pooler_host). In those profiles a linked connection can legitimately resolve to a pooler host under the profile domain, but this predicate returns false, so the initial pg-delta pull skips the direct-host rewrite and retains the pooler behavior this change is trying to avoid.
Useful? React with 👍 / 👎.
| /** Mirrors Go's `IsPoolerDbHost` (`internal/utils/connect.go`). */ | ||
| export function legacyIsPoolerDbHost(host: string): boolean { | ||
| const normalized = host.toLowerCase(); | ||
| return normalized === "pooler.supabase.com" || normalized.endsWith(".pooler.supabase.com"); |
There was a problem hiding this comment.
This only recognizes production *.pooler.supabase.com hosts, while the legacy resolver already validates pooler URLs against the active profile's poolerHost (including built-ins like supabase.green and custom profile files). For those profiles, a linked initial pg-delta pull that resolved to the profile's pooler will not be rewritten to the direct db.<ref>.<projectHost> target, leaving the pooler introspection issue unfixed outside the default production profile.
Useful? React with 👍 / 👎.
| return nil | ||
| } | ||
| summary := diff.SummarizeCatalogJSON(targetCatalog) | ||
| if summary.TotalObjects == 0 { |
There was a problem hiding this comment.
For an initial pg-delta pull against a project with no user-defined schema, exportTargetCatalog still returns the default/managed catalog entries (for example schema nodes), and SummarizeCatalogJSON counts any node with a schema field. That means TotalObjects can be nonzero even though there is truly nothing to pull, so this path reports the new “could not introspect” error instead of the intended generic “No schema changes found”; base the probe on pullable user objects or the same Supabase filter used by the diff.
Useful? React with 👍 / 👎.
| targetRef: pgDeltaTargetUrl, | ||
| role: "postgres", | ||
| }).pipe(Effect.catch(() => Effect.succeed(""))); | ||
| if (legacySummarizeCatalogJson(targetCatalog).totalObjects > 0) { |
There was a problem hiding this comment.
For an initial pg-delta pull against a project with no user-defined schema, the catalog export can still contain default/managed schema entries, and legacySummarizeCatalogJson counts any node with a schema field as an object. This makes totalObjects > 0 true even when there is nothing pullable, so the handler emits the introspection-failure error instead of the intended generic “No schema changes found”; filter the catalog summary down to objects the pg-delta diff would actually emit.
Useful? React with 👍 / 👎.
1 participant
What kind of change does this PR introduce?
Bug fix
What is the current behavior?
Initial
db pullwith pg-delta skips pg_dump and introspects the remote catalog via the diff TARGET (remote db). When link resolved to the pooler, pg-delta often saw an empty catalog and reported "No schema changes found".What is the new behavior?
On initial linked pg-delta pulls, the diff TARGET is rewritten from the pooler to the direct database host (
db.<project-ref>.supabase.co) before pg-delta runs. The CLI Postgres session is unchanged.Furthermore, if pg-delta still returns empty SQL on an initial pull:
Ref
Closes #5826