fix: mirror target extensions into the embedded plan database (#584) - #585
Conversation
pgschema does not manage extensions, so dump never emits CREATE EXTENSION. The embedded plan database therefore had none of the target's extensions, and any desired-state SQL referencing an extension type (citext, hstore, pg_trgm, ...) failed with "type does not exist" — even though every contrib extension is bundled with the embedded binary. plan already queries the target's installed extensions (with their schemas) but only used them for the external-plan-db consistency check. Now the embedded provider receives that map and, on ApplySchema, runs CREATE EXTENSION IF NOT EXISTS ... WITH SCHEMA for each one before the desired state, pinned to the target's schema so type qualification matches on both sides of the diff. An extension living in the managed schema maps to the temporary schema. Extensions the binary does not bundle (postgis, pgvector) are skipped with a warning; the existing hint still points at --plan-host for those. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Greptile SummaryThis PR passes target extension metadata into embedded plan databases, installs available extensions before applying desired-state SQL, and documents when an external plan database remains necessary.
Confidence Score: 4/5The PR is not yet safe to merge because bundled extensions installed in The main propagation and schema-mapping paths are sound, but the broad Files Needing Attention: internal/postgres/embedded.go Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
T[(Target database)] --> D[Detect extension names and schemas]
D --> C[EmbeddedPostgres configuration]
C --> A[ApplySchema]
A --> M{Target extension schema}
M -->|Managed schema| TS[Install into temporary schema]
M -->|Other user schema| US[Create schema and install extension]
M -->|pg_catalog| SK[Currently skipped]
TS --> SQL[Apply desired-state SQL]
US --> SQL
SK --> SQL
SQL --> I[Inspect desired schema and generate plan]
Reviews (1): Last reviewed commit: "fix: mirror target extensions into the e..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for _, name := range names { | ||
| schema := ep.extensions[name] | ||
| switch { | ||
| case name == "plpgsql", schema == "pg_catalog": |
There was a problem hiding this comment.
If the target has a separately installed bundled extension in pg_catalog, such as adminpack, this branch assumes it is already present in the fresh embedded database and skips installation. Desired SQL that uses the extension's functions or types then fails during planning instead of having the target extension mirrored. Only extensions known to be preinstalled, such as plpgsql, should be skipped.
Knowledge Base Used: Plan command
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Good catch — fixed in 51b9b54. The skip now applies only to plpgsql. Extensions in pg_catalog (or any pg_* schema) are mirrored; the only special-casing left is that we don't attempt CREATE SCHEMA for them, since the pg_ prefix is reserved and errors even with IF NOT EXISTS. Added seg installed in pg_catalog to the integration test.
There was a problem hiding this comment.
🟡 Changes recommended
The embedded extension mirroring logic currently skips all extensions installed in pg_catalog, which can prevent valid target extensions from being mirrored and reintroduce plan-time failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR improves pgschema plan (and apply --file) reliability for schemas that reference extension-owned types by ensuring the embedded plan database mirrors the target database’s installed extensions (including their installation schemas), eliminating the need to manually add CREATE EXTENSION statements to dumped schema files.
Changes:
- Thread target extension schema info into the embedded plan DB path and install those extensions before applying desired-state SQL.
- Improve the extension-related failure hint text to clarify contrib vs third-party extension behavior and when
--plan-hostis required. - Add an integration test covering extensions installed in
public, a side schema, and the managed schema; updateplan-dbdocumentation accordingly.
File summaries
| File | Description |
|---|---|
| internal/postgres/embedded.go | Installs target extensions into the embedded plan DB (schema-pinned) prior to applying desired SQL; updates extension-related hint text. |
| cmd/plan/plan.go | Passes detected target extensions into embedded plan DB configuration. |
| cmd/plan/extension_integration_test.go | Adds an integration test ensuring embedded plan DB installs target extensions so type references resolve without CREATE EXTENSION. |
| docs/cli/plan-db.mdx | Updates guidance to reflect automatic mirroring for bundled (contrib) extensions and external DB requirement for third-party extensions. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The previous skip treated every extension whose schema is pg_catalog as preinstalled, which is wrong for adminpack or any relocatable extension the user pinned there. The skip existed only because CREATE SCHEMA rejects the reserved pg_ prefix even with IF NOT EXISTS. Now only plpgsql is skipped; for pg_* schemas the CREATE SCHEMA step is bypassed and the extension is installed there directly. Test covers seg in pg_catalog. Docs: make the external plan database section self-consistent and state that it is not mirrored automatically. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
Alphabetical extension installation can skip bundled extensions whose prerequisites sort later.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
… ordering Alphabetical order is not a dependency order: hstore_plperl requires plperl, which sorts later, so the single pass failed it and the warn-and- skip never retried. Installing with CASCADE would put prerequisites in the wrong schema. Instead, retry failures after each pass until a pass installs nothing more; whatever remains is genuinely unavailable and is warned about once. Extracted as installUntilFixpoint with a unit test, since the order-sensitive contrib pairs all need a PL runtime the embedded binary cannot load on every platform. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The implementation addresses the reported failure, preserves target schema placement, handles extension dependencies, and includes focused coverage.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Balanced
Summary
pgschema dumpnever emitsCREATE EXTENSION(extensions are database-level and unmanaged), so the embedded plan database started with none of the target's extensions. Any schema referencing an extension type —citext,hstore,pg_trgm, … — failed at plan time withtype "x" does not exist, forcing users to hand-editCREATE EXTENSIONinto their dump. Yet every contrib extension is bundled with the embedded binary; the gap was purely that nothing installed them.planalready reads the target's installed extensions and their schemas (for the--plan-hostconsistency check from #518) and then discarded that map on the embedded path. This PR threads it intoEmbeddedPostgresand installs each extension inApplySchema, before the desired state:WITH SCHEMAto the target's schema, so type qualification matches on both sides of the diff (Extension-owned type schema mismatch causes false-positive diffs (pgvector, etc.) #518 stays correct)--plan-hostAlso corrects the hint text and
plan-db.mdx, which claimed the embedded database has no extensions at all — only third-party ones need an external plan database.Fixes #584. The roles half of that issue is a duplicate of #450 and is tracked there.
Test plan
New
TestEmbeddedPlanDB_InstallsTargetExtensions(cmd/plan/extension_integration_test.go) sets up a target withcitextinpublic,hstorein a side schema, andltreeinside a managed schemaapp, then plans a desired state that references those types with noCREATE EXTENSION. It fails before this change with the reporter's exact error and passes after, asserting an empty plan for both thepublicandappmanaged-schema cases.go test ./cmd/plan -run TestEmbeddedPlanDB_InstallsTargetExtensions -vAlso ran locally: all
cmd/planintegration tests (TestPlanCommand_*,TestExternalDatabase_*,TestPlanConfigDataConsistency) andcmd/applyfile-mode tests, all passing.🤖 Generated with Claude Code