Conversation
- Add --plan-host, --plan-port, --plan-db, --plan-user, --plan-password flags to apply command - Support PGSCHEMA_PLAN_* environment variables for plan database configuration - Validate plan database flags when --plan-host is provided (requires --plan-db and --plan-user) - Populate PlanConfig with plan database fields when using File Mode (--file) - Update documentation in docs/cli/apply.mdx: - Add "Plan Database Options" section with all flag descriptions - Add example showing external plan database usage - Update Overview section to explain embedded vs external database - Add test in apply_test.go to verify all plan database flags are defined with correct defaults When using File Mode (--file), apply command now supports external plan database just like the plan command, enabling schemas with PostgreSQL extensions or cross-schema foreign keys to be applied. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Updated docs/cli/plan-db.mdx to document that both plan and apply commands support external plan database: - Updated title and overview to mention both plan and apply commands - Added note that apply command only uses plan database in File Mode (--file) - Split Basic Usage into "With Plan Command" and "With Apply Command" sections - Updated extension example to show both plan and apply usage - Updated cross-schema FK example to show both plan and apply usage - Added apply command examples to all environment variable configurations - Updated .env file, Environment Variables, and Command Line Only examples This clarifies that users can use external plan database with both commands, making it easier to understand when and how to use this feature. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for using an external PostgreSQL database (instead of the default embedded PostgreSQL) in the apply command when using File Mode. This mirrors existing functionality available in the plan command and allows schemas that require PostgreSQL extensions or cross-schema references to work with apply --file.
Key changes:
- Added
--plan-*flags to theapplycommand for external database configuration - Updated documentation to reflect that both
planandapplycommands support external plan databases - Added comprehensive test coverage for the new functionality
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/cli/plan-db.mdx | Extended documentation to cover apply command usage with external plan database, including examples and clarifications about File Mode vs Plan Mode |
| docs/cli/apply.mdx | Added section documenting the new plan database options available in File Mode |
| cmd/apply/apply.go | Implemented plan database flag handling, environment variable support, and validation logic |
| cmd/apply/apply_test.go | Added unit tests to verify plan database flags are properly defined with correct defaults |
| cmd/apply/apply_integration_test.go | Added integration test validating end-to-end functionality with external plan database |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Plan database flags (--plan-host, --plan-db, etc.) are only applicable when using File Mode (--file) to generate a plan internally. When using Plan Mode (--plan) with a pre-generated plan JSON file, these flags should be ignored. Changes: - Move plan database flag validation from unconditional execution to only run inside the File Mode block (lines 303-317) - Add comprehensive test TestApplyCommand_PlanMode_IgnoresPlanDBFlags that verifies Plan Mode works even with invalid plan database env vars - This prevents unnecessary validation errors when users have plan database environment variables set but are using Plan Mode This improves UX by avoiding confusing validation errors about plan database configuration when it's not relevant to the operation.
Changed temporary schema name format from: pgschema_plan_YYYYMMDD_HHMMSS.NNNNNNNNN (period before nanoseconds) To: pgschema_plan_YYYYMMDD_HHMMSS_NNNNNNNNN (underscore before nanoseconds) The previous comment claiming 'the period is required in Go's time format' was incorrect. In Go's time.Format(), the period in '.000000000' is just a literal character that can be replaced with any separator, including underscore. This change makes the implementation consistent with the documented format shown in: - CLAUDE.md:122: pgschema_plan_20251030_154501_123456789 - docs/cli/plan-db.mdx:24: pgschema_plan_20251030_154501_123456789 Changes: - internal/postgres/external.go line 73-74: Updated format in comment and code - internal/postgres/embedded.go line 72: Updated timestamp format for runtime path
When using external plan database, temporary schema names were leaking into
generated DDL, causing it to reference non-existent schemas like:
CREATE TABLE pgschema_plan_20251030_154501_123456789.departments (...)
Instead of schema-independent DDL:
CREATE TABLE departments (...)
Root Cause:
- External database creates physical temporary schema with timestamped name
- Inspector extracts IR with Schema field set to temporary schema name
- Diff compares temp schema (pgschema_plan_*) vs target schema (public)
- qualifyEntityName() adds schema qualification because schemas don't match
- Generated DDL incorrectly includes temporary schema references
Solution:
After inspecting external database, normalize all schema names in the
desired state IR to match the target schema. This ensures qualifyEntityName()
sees matching schemas and generates unqualified (schema-independent) DDL.
Changes:
- cmd/plan/plan.go:
- Add ir package import
- Add normalizeSchemaNames() function (lines 377-423)
- Call normalization after external database inspection (lines 277-282)
- Handles all object types: Tables, Views, Functions, Procedures, Types,
Sequences, Aggregates
Fixes TestApplyCommand_WithExternalPlanDatabase test failure.
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
1 participant
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.
More to address #121 and #122