fix: auto-detect schema from dump file header for plan/apply (#296)#312
fix: auto-detect schema from dump file header for plan/apply (#296)#312
Conversation
When dumping a non-public schema (e.g., vehicle), the dump output now includes a "-- Dumped from schema: <name>" metadata line. The plan and apply commands read this header and auto-detect the target schema when --schema is not explicitly set, preventing accidental cross-schema operations that could drop tables from the wrong schema. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Greptile SummaryAdds schema auto-detection to prevent accidental cross-schema operations by embedding schema metadata in dump headers and parsing it during plan/apply commands.
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[dump command] -->|writes| B[SQL file with schema header]
B -->|"-- Dumped from schema: X"| C{plan/apply command}
C -->|--schema flag set?| D[Use explicit schema]
C -->|--schema NOT set| E[DetectSchemaFromFile]
E -->|header found| F[Use detected schema X]
E -->|header NOT found| G[Default to 'public']
F --> H[Execute plan/apply with schema X]
D --> H
G --> H
Last reviewed commit: 8cb2e74 |
There was a problem hiding this comment.
Pull request overview
This PR adds schema metadata to pgschema dump output and uses it to auto-select the correct target schema during plan/apply when --schema isn’t explicitly provided, preventing accidental operations against the default public schema.
Changes:
- Add
-- Dumped from schema: <name>to the dump header. - Introduce a schema-detection utility that parses the schema from a dump file header.
- Update
planandapplyto use the detected schema when--schemais not set, and adjust dump integration test normalization accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/dump/formatter.go | Emits schema metadata in the dump header for downstream consumers. |
| cmd/util/schema_detect.go | Adds a small helper to detect schema from the dump header. |
| cmd/util/schema_detect_test.go | Adds unit tests for schema header parsing. |
| cmd/plan/plan.go | Uses detected schema (when available) as the effective schema for planning. |
| cmd/apply/apply.go | Uses detected schema (when available) as the effective schema for apply. |
| internal/diff/header.go | Removes the old dump header generator (now unused). |
| cmd/dump/dump_integration_test.go | Updates normalization to ignore the new schema header line in comparisons. |
Comments suppressed due to low confidence (1)
internal/diff/header.go:2
- This file is now empty (only a package declaration). Since GenerateDumpHeader was removed, consider deleting the file entirely to avoid a confusing empty source file in the diff package.
package diff
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: "quoted schema name", | ||
| content: `-- | ||
| -- pgschema database dump | ||
| -- | ||
|
|
||
| -- Dumped from database version PostgreSQL 17.7 | ||
| -- Dumped by pgschema version 1.7.1 | ||
| -- Dumped from schema: my_schema | ||
|
|
||
| `, | ||
| expected: "my_schema", | ||
| }, |
There was a problem hiding this comment.
This test case is labeled "quoted schema name" but the content uses an unquoted identifier (my_schema). Rename the case to reflect what it actually covers, or update the fixture to include a truly quoted schema name (e.g., "MySchema") and assert the intended behavior.
| func detectSchemaFromReader(r io.Reader) (string, error) { | ||
| scanner := bufio.NewScanner(r) | ||
| for i := 0; i < 20 && scanner.Scan(); i++ { | ||
| line := scanner.Text() | ||
| if strings.HasPrefix(line, schemaHeaderPrefix) { | ||
| return strings.TrimSpace(line[len(schemaHeaderPrefix):]), nil | ||
| } | ||
| } | ||
| return "", scanner.Err() |
There was a problem hiding this comment.
detectSchemaFromReader uses bufio.Scanner without increasing the buffer size. Scanner will return ErrTooLong if it encounters a line >64K, which can happen in SQL files (e.g., large function bodies or comments on a single line), causing schema detection to error. Consider increasing the scanner buffer (scanner.Buffer) or using a bufio.Reader to read lines so header parsing is robust.
| // Auto-detect schema from dump file if --schema was not explicitly set | ||
| effectiveSchema := planSchema | ||
| if !cmd.Flags().Changed("schema") && planFile != "" { | ||
| if detected, err := util.DetectSchemaFromFile(planFile); err == nil && detected != "" { | ||
| effectiveSchema = detected | ||
| fmt.Fprintf(os.Stderr, "Auto-detected schema '%s' from dump file\n", detected) | ||
| } | ||
| } |
There was a problem hiding this comment.
Schema auto-detection ignores errors from DetectSchemaFromFile. If header parsing fails (e.g., due to a scanning error), this silently falls back to the default schema, which undermines the safety goal of preventing cross-schema operations. Consider surfacing a warning to stderr when err != nil, or failing fast with a message instructing the user to pass --schema explicitly.
| if detected, err := util.DetectSchemaFromFile(applyFile); err == nil && detected != "" { | ||
| effectiveSchema = detected | ||
| fmt.Fprintf(os.Stderr, "Auto-detected schema '%s' from dump file\n", detected) | ||
| } |
There was a problem hiding this comment.
Schema auto-detection ignores errors from DetectSchemaFromFile. If header parsing fails, apply will silently continue using the default schema, which can reintroduce the cross-schema risk this change is meant to prevent. Consider warning on detection errors or requiring --schema when detection fails.
| if detected, err := util.DetectSchemaFromFile(applyFile); err == nil && detected != "" { | |
| effectiveSchema = detected | |
| fmt.Fprintf(os.Stderr, "Auto-detected schema '%s' from dump file\n", detected) | |
| } | |
| detected, err := util.DetectSchemaFromFile(applyFile) | |
| if err != nil { | |
| return fmt.Errorf("failed to auto-detect schema from dump file %q: %w (please specify --schema explicitly)", applyFile, err) | |
| } | |
| if detected == "" { | |
| return fmt.Errorf("schema could not be auto-detected from dump file %q; please specify --schema explicitly", applyFile) | |
| } | |
| effectiveSchema = detected | |
| fmt.Fprintf(os.Stderr, "Auto-detected schema '%s' from dump file\n", detected) |
|
Closing this PR — after further analysis, the reported behavior is working as designed. pgschema produces schema-agnostic dumps by design. The same dump file can be applied to any schema ( The issue was a missing |
Summary
-- Dumped from schema: <name>metadata in the header--schemais not explicitly setvehicleschema then applying without--schema vehiclewould previously default topublicand drop public tables)Fixes #296
Changes
internal/dump/formatter.go: Added-- Dumped from schema: <name>line to dump headercmd/util/schema_detect.go: New utility to parse schema name from dump file headercmd/plan/plan.go: Auto-detect schema from file when--schemais not explicitly setcmd/apply/apply.go: Same auto-detection for apply commandinternal/diff/header.go: Removed deadGenerateDumpHeaderfunction (duplicate of formatter's method)cmd/dump/dump_integration_test.go: Updated test normalization to handle new header lineBehavior
--schemais explicitly set, it is always used (no change)--schemais not set and the dump file has-- Dumped from schema: X, usesX--schemais not set and the dump file has no schema header (old format), defaults topublic(backward compatible)Auto-detected schema 'vehicle' from dump fileTest plan
cmd/util/schema_detect_test.go)TestDiffFromFiles)TestPlanAndApply— create_table, migrate, dependency categories)🤖 Generated with Claude Code