From 62624405391be2db19bd523904fdc9bc3233206a Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Wed, 18 Feb 2026 21:52:33 -0800 Subject: [PATCH] fix: add SET search_path to dump output for non-public schemas (#296) When dumping non-public schemas, the output now includes SET search_path TO , 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 --- cmd/dump/dump_integration_test.go | 92 +++++++++++++++++++++++++++++- internal/dump/formatter.go | 9 +++ internal/postgres/desired_state.go | 11 ++++ internal/postgres/embedded.go | 6 +- internal/postgres/external.go | 6 +- 5 files changed, 120 insertions(+), 4 deletions(-) diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index cb21bad3..846351c0 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -109,6 +109,90 @@ func TestDumpCommand_Issue252FunctionSchemaQualifier(t *testing.T) { runExactMatchTest(t, "issue_252_function_schema_qualifier") } +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) + } +} + func runExactMatchTest(t *testing.T, testDataDir string) { runExactMatchTestWithContext(t, context.Background(), testDataDir) } @@ -270,8 +354,8 @@ func runTenantSchemaTest(t *testing.T, testDataDir string) { } } -// normalizeSchemaOutput removes version-specific lines for comparison. -// This allows comparing dumps across different PostgreSQL versions. +// normalizeSchemaOutput removes version-specific and schema-specific header lines for comparison. +// This allows comparing dumps across different PostgreSQL versions and different schemas. func normalizeSchemaOutput(output string) string { lines := strings.Split(output, "\n") var normalizedLines []string @@ -282,6 +366,10 @@ func normalizeSchemaOutput(output string) string { strings.Contains(line, "-- Dumped from database version") { continue } + // Skip SET search_path lines (added for non-public schemas) + if strings.HasPrefix(line, "SET search_path TO ") { + continue + } normalizedLines = append(normalizedLines, line) } diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index 9dd262db..7038d8d6 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -165,6 +165,15 @@ func (f *DumpFormatter) generateDumpHeader() string { header.WriteString(fmt.Sprintf("-- Dumped from database version %s\n", f.dbVersion)) header.WriteString(fmt.Sprintf("-- Dumped by pgschema version %s\n", version.App())) header.WriteString("\n") + + // For non-public schemas, add SET search_path so the dump is self-contained. + // This ensures objects are created in the correct schema when the SQL is applied + // directly (e.g., via psql), matching pg_dump conventions. + if f.targetSchema != "" && f.targetSchema != "public" { + quotedSchema := ir.QuoteIdentifier(f.targetSchema) + header.WriteString(fmt.Sprintf("SET search_path TO %s, public;\n", quotedSchema)) + } + header.WriteString("\n") return header.String() } diff --git a/internal/postgres/desired_state.go b/internal/postgres/desired_state.go index 40b36de7..d2d20453 100644 --- a/internal/postgres/desired_state.go +++ b/internal/postgres/desired_state.go @@ -57,6 +57,17 @@ func GenerateTempSchemaName() string { return fmt.Sprintf("pgschema_tmp_%s_%s", timestamp, randomSuffix) } +// stripSetSearchPath removes SET search_path statements from SQL. +// This is needed because pgschema dump includes SET search_path for non-public schemas +// to make the dump self-contained. When applying desired state SQL to a temporary schema +// during plan generation, the SET search_path would override the temp schema's search_path, +// causing objects to be created in the wrong schema. +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, "") +} + // stripSchemaQualifications removes schema qualifications from SQL statements for the specified target schema. // // Purpose: diff --git a/internal/postgres/embedded.go b/internal/postgres/embedded.go index 27020c1f..c85d0bba 100644 --- a/internal/postgres/embedded.go +++ b/internal/postgres/embedded.go @@ -206,10 +206,14 @@ func (ep *EmbeddedPostgres) ApplySchema(ctx context.Context, schema string, sql return fmt.Errorf("failed to set search_path: %w", err) } + // Strip SET search_path statements from SQL to prevent overriding the temp schema's search_path. + // pgschema dump includes SET search_path for non-public schemas to make dumps self-contained. + cleanedSQL := stripSetSearchPath(sql) + // Strip schema qualifications from SQL before applying to temporary schema // This ensures that objects are created in the temporary schema via search_path // rather than being explicitly qualified with the original schema name - schemaAgnosticSQL := stripSchemaQualifications(sql, schema) + schemaAgnosticSQL := stripSchemaQualifications(cleanedSQL, schema) // Replace schema names in ALTER DEFAULT PRIVILEGES statements // These use "IN SCHEMA " syntax which isn't handled by stripSchemaQualifications diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 30bb00c9..64d4178c 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -112,10 +112,14 @@ func (ed *ExternalDatabase) ApplySchema(ctx context.Context, schema string, sql return fmt.Errorf("failed to set search_path: %w", err) } + // Strip SET search_path statements from SQL to prevent overriding the temp schema's search_path. + // pgschema dump includes SET search_path for non-public schemas to make dumps self-contained. + cleanedSQL := stripSetSearchPath(sql) + // Strip schema qualifications from SQL before applying to temporary schema // This ensures that objects are created in the temporary schema via search_path // rather than being explicitly qualified with the original schema name - schemaAgnosticSQL := stripSchemaQualifications(sql, schema) + schemaAgnosticSQL := stripSchemaQualifications(cleanedSQL, schema) // Replace schema names in ALTER DEFAULT PRIVILEGES statements // These use "IN SCHEMA " syntax which isn't handled by stripSchemaQualifications