Pd/feat/plan multi schema#431
Conversation
- Updated the `README.md` to include instructions for using multiple schemas with the `pgschema plan` command, detailing the behavior and caveats. - Modified the `plan.go` command to accept a comma-separated list of schemas, allowing for comparison across multiple PostgreSQL namespaces. - Enhanced the `GeneratePlan` function to handle multiple schemas, ensuring correct introspection and DDL generation. - Added new utility functions for parsing schema lists and computing fingerprints for multiple schemas. - Updated related documentation and tests to reflect these changes. This update improves the flexibility of schema management in migration plans, accommodating more complex database structures.
- Implemented the GetObjectName method for the Schema type, allowing it to return the schema's name. - This addition enhances the consistency of object name retrieval across different types in the IR package.
Greptile SummaryThis PR adds multi-schema planning support for PostgreSQL schemas. The main changes are:
Confidence Score: 2/5This should be fixed before merging.
cmd/plan/plan.go, internal/postgres/external.go, internal/postgres/embedded.go, internal/diff/schema.go, internal/diff/diff.go, testutil/postgres.go. Important Files Changed
|
There was a problem hiding this comment.
Pull request overview
Adds multi-schema support to pgschema plan: --schema now accepts a comma-separated list, with the first entry treated as the primary (strip/normalize target) and the rest introspected on both target and plan databases. The diff engine gains a DiffTypeSchema and emits CREATE SCHEMA IF NOT EXISTS / DROP SCHEMA … CASCADE statements. Foreign keys produced during CREATE and ALTER TABLE flows are now uniformly deferred and flushed (topologically sorted) after the modify phase, so cross-table/cross-schema FK dependencies resolve correctly.
Changes:
- New
BuildIRFromSchemas/ComputeFingerprintForSchemasandParseSchemaList, wired throughcmd/planto build a merged multi-schema IR for both sides of the diff. - New
DiffTypeSchemawithgenerateCreateSchemasSQL/generateDropSchemasSQL, plus formatter/grouping support. - All FK constraint emissions now go through
collector.queueDeferredForeignKey+flushDeferredForeignKeys, with a newsortDeferredForeignKeystopological pass.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testutil/postgres.go | Hard-codes PG version to 15 and comments out os import (debug leftover). |
| README.md | Documents multi-schema plan usage. |
| docs/cli/plan.mdx | Expanded --schema docs and multi-schema example. |
| cmd/plan/README.md | Rewritten with single- vs multi-schema behavior and caveats. |
| cmd/plan/plan.go | Parses schema list, picks primary, builds inspect spec, normalizes temp→primary. |
| cmd/util/connection.go | Adds ParseSchemaList, switches to BuildIRFromSchemas. |
| ir/inspector.go | Splits BuildIR into single/multi paths; adds buildSchemaContent and BuildIRFromSchemas. |
| ir/ir.go | Adds Schema.GetObjectName. |
| internal/fingerprint/fingerprint.go | Adds ComputeFingerprintForSchemas over a schema subset. |
| internal/diff/diff.go | New DiffTypeSchema, emits schema create/drop, defers FK flush after modify phase. |
| internal/diff/schema.go (+test) | New file generating CREATE/DROP SCHEMA DDL. |
| internal/diff/collector.go | Pending FK queue and flush API on the collector. |
| internal/diff/table.go | Routes FKs through deferred queue; refactors generateDeferredConstraintsSQL. |
| internal/diff/topological.go (+test) | New sortDeferredForeignKeys topo sort for deferred FKs. |
| internal/dump/formatter.go | Adds schema object directory and grouping for DiffTypeSchema. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -153,21 +153,7 @@ func ConnectToPostgres(t testing.TB, embeddedPG *postgres.EmbeddedPostgres) (con | |||
| // It reads from the PGSCHEMA_POSTGRES_VERSION environment variable, | |||
| // defaulting to "18" if not set. | |||
| func getPostgresVersion() postgres.PostgresVersion { | |||
| versionStr := os.Getenv("PGSCHEMA_POSTGRES_VERSION") | |||
| switch versionStr { | |||
| case "14": | |||
| return embeddedpostgres.V14 | |||
| case "15": | |||
| return embeddedpostgres.V15 | |||
| case "16": | |||
| return embeddedpostgres.V16 | |||
| case "17": | |||
| return embeddedpostgres.V17 | |||
| case "18", "": | |||
| return embeddedpostgres.V18 | |||
| default: | |||
| return embeddedpostgres.V18 | |||
| } | |||
| return embeddedpostgres.PostgresVersion("15") | |||
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -319,7 +328,7 @@ func GeneratePlan(config *PlanConfig, provider postgres.DesiredStateProvider) (* | |||
| providerSSLMode = "prefer" | |||
| } | |||
| } | |||
| desiredStateIR, err := util.GetIRFromDatabase(providerHost, providerPort, providerDB, providerUsername, providerPassword, providerSSLMode, schemaToInspect, config.ApplicationName, ignoreConfig) | |||
| desiredStateIR, err := util.GetIRFromDatabase(providerHost, providerPort, providerDB, providerUsername, providerPassword, providerSSLMode, inspectSpec, config.ApplicationName, ignoreConfig) | |||
There was a problem hiding this comment.
The desired side now inspects the provider's temporary schema plus the remaining schema names from --schema, but ApplySchema only rewrites the primary schema. With --schema public,app and desired SQL such as CREATE TABLE app.widgets (...), embedded planning fails because only the temp schema was created. With an external plan database, app is read from the real validation database, so stale or unrelated objects in that schema can be included in the desired IR.
| func generateDropSchemasSQL(schemas []*ir.Schema, collector *diffCollector) { | ||
| if len(schemas) == 0 { | ||
| return | ||
| } | ||
| sorted := append([]*ir.Schema(nil), schemas...) | ||
| sort.Slice(sorted, func(i, j int) bool { | ||
| return sorted[i].Name > sorted[j].Name | ||
| }) | ||
| for _, s := range sorted { | ||
| if s == nil { | ||
| continue | ||
| } | ||
| sql := fmt.Sprintf("DROP SCHEMA IF EXISTS %s CASCADE;", ir.QuoteIdentifier(s.Name)) | ||
| ctx := &diffContext{ |
There was a problem hiding this comment.
Dropped schemas are always emitted with CASCADE. If a planned schema contains ignored objects or object types pgschema does not model, the generated migration can remove them even though they were not visible as individual drops in the plan. A multi-schema plan that omits app from the desired state can therefore delete extra objects in app, not just the objects represented in the IR.
No description provided.