fix: add SET search_path to dump output for non-public schemas (#296) by tianzhou · Pull Request #298 · pgplex/pgschema

tianzhou · 2026-02-19T05:52:50Z

Summary

When dumping non-public schemas (e.g., --schema vehicle), the output now includes SET search_path TO <schema>, public; in the header, making dump files self-contained and ensuring objects are created in the correct schema when applied directly (e.g., via psql)
Strips SET search_path statements from SQL in ApplySchema() to prevent dump file headers from overriding the temp schema's search_path during plan generation
Follows pg_dump conventions for schema-specific output

Fixes #296

Test plan

New test TestDumpCommand_Issue296NonPublicSchemaSearchPath verifies:
- SET search_path is present in dump output for non-public schemas
- SET search_path is NOT present for public schema dumps
Existing tenant schema test passes (normalizes SET search_path for cross-schema comparison)
Full test suite passes (go test -v ./...)

Run: go test -v ./cmd/dump -run TestDumpCommand_Issue296

🤖 Generated with Claude Code

When dumping non-public schemas, the output now includes SET search_path TO <schema>, public; in the header. This makes dump files self-contained and ensures objects are created in the correct schema when applied directly via psql. Also strips SET search_path from SQL in ApplySchema() to prevent it from overriding the temp schema's search_path during plan generation. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

greptile-apps · 2026-02-19T05:55:04Z

Greptile Summary

This PR fixes issue #296 by making dump files for non-public schemas self-contained and executable. When dumping non-public schemas (e.g., vehicle), the output now includes SET search_path TO <schema>, public; in the header, following pg_dump conventions. This ensures objects are created in the correct schema when the dump is applied directly via psql.

To prevent this new header from interfering with the plan command's temporary schema workflow, the PR adds a stripSetSearchPath() function that removes SET search_path statements before applying SQL to temporary schemas during plan generation. This ensures objects are created in the intended temporary schema rather than being redirected by the dump header.

Key changes:

internal/dump/formatter.go: Added conditional SET search_path header for non-public schemas (lines 169-175)
internal/postgres/desired_state.go: New stripSetSearchPath() function with regex to remove SET search_path statements (lines 60-69)
internal/postgres/embedded.go & external.go: Integrated stripSetSearchPath() call before applying SQL to temporary schemas
cmd/dump/dump_integration_test.go: Comprehensive test verifying presence/absence of SET search_path for non-public/public schemas
Test normalization updated to skip SET search_path lines for cross-schema comparisons

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The implementation is clean, well-tested, and follows pg_dump conventions. The regex pattern correctly handles the SET search_path statement format, and the changes are isolated to the dump formatting and plan generation workflows without affecting other functionality. The new test provides good coverage for both public and non-public schema scenarios.
No files require special attention

Important Files Changed

Filename	Overview
cmd/dump/dump_integration_test.go	Added comprehensive test for issue #296 verifying SET search_path behavior for public and non-public schemas
internal/dump/formatter.go	Added SET search_path header for non-public schemas in dump output to make dumps self-contained
internal/postgres/desired_state.go	Added stripSetSearchPath function to remove SET search_path statements during plan generation
internal/postgres/embedded.go	Integrated stripSetSearchPath call in ApplySchema to prevent search_path override in temp schema
internal/postgres/external.go	Integrated stripSetSearchPath call in ApplySchema to prevent search_path override in temp schema

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Dump non-public schema] --> B{Schema == public?}
    B -->|No| C[Add SET search_path TO schema, public]
    B -->|Yes| D[Skip SET search_path]
    C --> E[Write to dump file]
    D --> E
    
    E --> F[Plan command loads dump file]
    F --> G[Create temp schema pgschema_tmp_*]
    G --> H[Set search_path to temp schema]
    H --> I[stripSetSearchPath removes SET search_path from SQL]
    I --> J[stripSchemaQualifications removes schema prefixes]
    J --> K[Apply SQL to temp schema]
    K --> L[Objects created in correct temp schema]
    
    style C fill:#90EE90
    style I fill:#FFB6C1
    style L fill:#87CEEB

_{Last reviewed commit: 6262440}

Copilot

Pull request overview

This PR fixes issue #296 where dumping and applying non-public schemas resulted in objects being created in the wrong schema. The fix adds SET search_path TO <schema>, public; to dump headers for non-public schemas, making dumps self-contained and compatible with direct application via psql, while also ensuring that pgschema's internal plan generation correctly strips these statements to avoid interfering with temporary schema operations.

Changes:

Added SET search_path statement to dump output headers for non-public schemas to make dumps self-contained
Implemented stripSetSearchPath function to remove SET search_path statements from SQL before applying to temporary schemas during plan generation
Updated test normalization to skip SET search_path lines when comparing dumps from different schemas

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
internal/dump/formatter.go	Added SET search_path statement to dump headers for non-public schemas using QuoteIdentifier for proper escaping
internal/postgres/desired_state.go	Implemented stripSetSearchPath function with regex to remove SET search_path statements from SQL
internal/postgres/embedded.go	Integrated stripSetSearchPath into ApplySchema to clean SQL before applying to temporary schemas
internal/postgres/external.go	Integrated stripSetSearchPath into ApplySchema to clean SQL before applying to temporary schemas
cmd/dump/dump_integration_test.go	Added test verifying SET search_path presence in non-public schema dumps and absence in public schema dumps; updated normalization function to skip SET search_path lines

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T05:58:10Z

internal/postgres/desired_state.go

+func stripSetSearchPath(sql string) string {
+	// Match SET search_path TO ... ; with optional whitespace and newlines
+	re := regexp.MustCompile(`(?im)^\s*SET\s+search_path\s+TO\s+[^;]+;\s*\n?`)
+	return re.ReplaceAllString(sql, "")


The regex pattern is compiled on every function call to stripSetSearchPath. For better performance, consider compiling the regex once at the package level as a global variable, similar to how functionCallRegex is defined in internal/diff/diff.go. While this function isn't called in a hot path, this would follow the performance best practice pattern used elsewhere in the codebase.

Suggested change

func stripSetSearchPath(sql string) string {

// Match SET search_path TO ... ; with optional whitespace and newlines

re := regexp.MustCompile(`(?im)^\s*SET\s+search_path\s+TO\s+[^;]+;\s*\n?`)

return re.ReplaceAllString(sql, "")

// setSearchPathRegex matches "SET search_path TO ...;" with optional whitespace and newlines.

var setSearchPathRegex = regexp.MustCompile(`(?im)^\s*SET\s+search_path\s+TO\s+[^;]+;\s*\n?`)

func stripSetSearchPath(sql string) string {

return setSearchPathRegex.ReplaceAllString(sql, "")

Copilot · 2026-02-19T05:58:10Z

cmd/dump/dump_integration_test.go

+func TestDumpCommand_Issue296NonPublicSchemaSearchPath(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	// Setup PostgreSQL
+	embeddedPG := testutil.SetupPostgres(t)
+	defer embeddedPG.Stop()
+
+	// Connect to database
+	conn, host, port, dbname, user, password := testutil.ConnectToPostgres(t, embeddedPG)
+	defer conn.Close()
+
+	// Create a non-public schema with a simple table
+	schemaName := "vehicle"
+	_, err := conn.Exec(fmt.Sprintf("CREATE SCHEMA %s", schemaName))
+	if err != nil {
+		t.Fatalf("Failed to create schema %s: %v", schemaName, err)
+	}
+
+	_, err = conn.Exec(fmt.Sprintf("SET search_path TO %s", schemaName))
+	if err != nil {
+		t.Fatalf("Failed to set search_path: %v", err)
+	}
+
+	_, err = conn.Exec(`
+		CREATE TABLE vehicle_config (
+			id serial PRIMARY KEY,
+			enabled boolean DEFAULT false NOT NULL
+		)
+	`)
+	if err != nil {
+		t.Fatalf("Failed to create table: %v", err)
+	}
+
+	// Dump the non-public schema
+	config := &DumpConfig{
+		Host:     host,
+		Port:     port,
+		DB:       dbname,
+		User:     user,
+		Password: password,
+		Schema:   schemaName,
+	}
+
+	output, err := ExecuteDump(config)
+	if err != nil {
+		t.Fatalf("Dump command failed: %v", err)
+	}
+
+	// Verify SET search_path is present for non-public schema
+	expectedSearchPath := fmt.Sprintf("SET search_path TO %s, public;", ir.QuoteIdentifier(schemaName))
+	if !strings.Contains(output, expectedSearchPath) {
+		t.Errorf("Dump output for non-public schema %q should contain %q\nActual output:\n%s",
+			schemaName, expectedSearchPath, output)
+	}
+
+	// Dump the public schema (reset search_path first)
+	_, err = conn.Exec("SET search_path TO public")
+	if err != nil {
+		t.Fatalf("Failed to reset search_path: %v", err)
+	}
+
+	publicConfig := &DumpConfig{
+		Host:     host,
+		Port:     port,
+		DB:       dbname,
+		User:     user,
+		Password: password,
+		Schema:   "public",
+	}
+
+	publicOutput, err := ExecuteDump(publicConfig)
+	if err != nil {
+		t.Fatalf("Dump command failed for public schema: %v", err)
+	}
+
+	// Verify SET search_path is NOT present for public schema
+	if strings.Contains(publicOutput, "SET search_path") {
+		t.Errorf("Dump output for public schema should NOT contain SET search_path\nActual output:\n%s",
+			publicOutput)
+	}
+}


Consider adding an integration test that verifies the complete workflow: dump a non-public schema (which should include SET search_path), then use the plan command to apply that dump to verify that stripSetSearchPath correctly prevents the dump header from overriding the temporary schema's search_path. This would provide more comprehensive test coverage for the fix beyond just verifying the dump output format.

Copilot AI review requested due to automatic review settings February 19, 2026 05:52

Copilot started reviewing on behalf of tianzhou February 19, 2026 05:53 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add SET search_path to dump output for non-public schemas (#296)#298

fix: add SET search_path to dump output for non-public schemas (#296)#298
tianzhou wants to merge 1 commit intomainfrom
fix/issue-296-non-public-schema-dump

tianzhou commented Feb 19, 2026

Uh oh!

greptile-apps bot commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

Conversation

tianzhou commented Feb 19, 2026

Summary

Test plan

Uh oh!

greptile-apps bot commented Feb 19, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments