From 1e2807284f414fcb2e86159ef6b2eb79b1c364c9 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Sat, 15 Aug 2026 13:02:10 +0800 Subject: [PATCH 1/3] fix: validate extension schema consistency between plan and target databases (#518) Co-Authored-By: Claude Fable 5 --- cmd/plan/external_db_integration_test.go | 63 +++++++++++++++ cmd/plan/plan.go | 16 ++++ internal/postgres/external.go | 98 ++++++++++++++++++++++++ 3 files changed, 177 insertions(+) diff --git a/cmd/plan/external_db_integration_test.go b/cmd/plan/external_db_integration_test.go index 4aa19972..7acb5901 100644 --- a/cmd/plan/external_db_integration_test.go +++ b/cmd/plan/external_db_integration_test.go @@ -115,6 +115,69 @@ func TestExternalDatabase_VersionMismatch(t *testing.T) { assert.NotEmpty(t, pgVersion, "version should not be empty") } +// TestExternalDatabase_ExtensionSchemaMismatch tests that provider creation fails fast +// when an extension is installed in different schemas on the plan and target databases. +// Extension-owned types (e.g. pgvector's vector, citext) are schema-qualified by whatever +// schema their extension resolves to, so a mismatch silently produces a wrong plan: +// spurious ALTER COLUMN TYPE or CREATE TABLE DDL referencing a schema that doesn't +// exist on the target (issue #518). +func TestExternalDatabase_ExtensionSchemaMismatch(t *testing.T) { + // Skip in short mode + if testing.Short() { + t.Skip("skipping integration test") + } + + targetDB := testutil.SetupPostgres(t) + defer targetDB.Stop() + + externalPlanDB := testutil.SetupPostgres(t) + defer externalPlanDB.Stop() + + // Install citext into schema "exts" on the target, but "public" on the plan database + targetConn, targetHost, targetPort, targetDatabase, targetUser, targetPassword := testutil.ConnectToPostgres(t, targetDB) + defer targetConn.Close() + _, err := targetConn.Exec("CREATE SCHEMA exts") + require.NoError(t, err) + _, err = targetConn.Exec("CREATE EXTENSION citext SCHEMA exts") + require.NoError(t, err) + + planConn, planHost, planPort, planDatabase, planUser, planPassword := testutil.ConnectToPostgres(t, externalPlanDB) + defer planConn.Close() + _, err = planConn.Exec("CREATE EXTENSION citext SCHEMA public") + require.NoError(t, err) + + config := &PlanConfig{ + Host: targetHost, + Port: targetPort, + DB: targetDatabase, + User: targetUser, + Password: targetPassword, + Schema: "public", + ApplicationName: "pgschema-test", + PlanDBHost: planHost, + PlanDBPort: planPort, + PlanDBDatabase: planDatabase, + PlanDBUser: planUser, + PlanDBPassword: planPassword, + } + + _, err = CreateDesiredStateProvider(config) + require.Error(t, err, "should fail when extension schemas differ between plan and target databases") + assert.Contains(t, err.Error(), "citext", "error should name the mismatched extension") + assert.Contains(t, err.Error(), "exts", "error should name the target schema") + assert.Contains(t, err.Error(), "public", "error should name the plan database schema") + + // After moving the extension to the matching schema, provider creation succeeds + _, err = planConn.Exec("CREATE SCHEMA exts") + require.NoError(t, err) + _, err = planConn.Exec("ALTER EXTENSION citext SET SCHEMA exts") + require.NoError(t, err) + + provider2, err := CreateDesiredStateProvider(config) + require.NoError(t, err, "matching extension schemas should pass validation") + provider2.Stop() +} + // TestExternalDatabase_CleanupOnError tests that temporary schema is cleaned up on errors func TestExternalDatabase_CleanupOnError(t *testing.T) { // Skip in short mode diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index 76fb5fc7..5f56840b 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -221,6 +221,21 @@ func CreateDesiredStateProvider(config *PlanConfig) (postgres.DesiredStateProvid // If plan-host is provided, use external database if config.PlanDBHost != "" { + // Collect the target's extension installation schemas so the external + // database can verify shared extensions live in the same schema on both + // sides (issue #518). + targetExtensions, err := postgres.GetExtensionSchemasFromDB( + config.Host, + config.Port, + config.DB, + config.User, + config.Password, + config.SSLMode, + ) + if err != nil { + return nil, fmt.Errorf("failed to query target database extensions: %w", err) + } + externalConfig := &postgres.ExternalDatabaseConfig{ Host: config.PlanDBHost, Port: config.PlanDBPort, @@ -229,6 +244,7 @@ func CreateDesiredStateProvider(config *PlanConfig) (postgres.DesiredStateProvid Password: config.PlanDBPassword, SSLMode: config.PlanDBSSLMode, TargetMajorVersion: targetMajorVersion, + TargetExtensions: targetExtensions, } return postgres.NewExternalDatabase(externalConfig) } diff --git a/internal/postgres/external.go b/internal/postgres/external.go index 4208711c..b1d284cd 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -5,6 +5,8 @@ import ( "context" "database/sql" "fmt" + "sort" + "strings" _ "github.com/jackc/pgx/v5/stdlib" "github.com/pgplex/pgschema/cmd/util" @@ -32,6 +34,12 @@ type ExternalDatabaseConfig struct { Password string SSLMode string TargetMajorVersion int // Expected major version to match + // TargetExtensions maps extension name to its installation schema on the + // target database. Extensions installed on both databases must live in the + // same schema on each side; otherwise extension-owned types (e.g. pgvector's + // vector) resolve to different schema-qualified names and the generated plan + // is silently wrong (issue #518). + TargetExtensions map[string]string } // sslModeOrDefault returns the configured SSL mode, defaulting to "prefer" if empty @@ -77,6 +85,12 @@ func NewExternalDatabase(config *ExternalDatabaseConfig) (*ExternalDatabase, err ) } + // Validate that extensions installed on both databases live in the same schema + if err := validateExtensionSchemas(db, config.TargetExtensions); err != nil { + db.Close() + return nil, err + } + // Generate temporary schema name with unique timestamp tempSchema := GenerateTempSchemaName() @@ -185,6 +199,90 @@ func (ed *ExternalDatabase) Stop() error { return nil } +// validateExtensionSchemas ensures every extension installed on both the plan and +// target databases lives in the same schema on each side. Extension-owned types +// (e.g. pgvector's vector) are schema-qualified by whatever schema their extension +// is installed in, so a mismatch makes the two sides' type names differ and yields +// a silently wrong plan: spurious ALTER COLUMN TYPE, or CREATE TABLE DDL that +// references a schema that doesn't exist on the target (issue #518). +// Extensions present on only one side are not an error: if the desired state +// actually needs a missing extension, applying it to the plan database fails +// loudly with the existing hint. +func validateExtensionSchemas(db *sql.DB, targetExtensions map[string]string) error { + if len(targetExtensions) == 0 { + return nil + } + + planExtensions, err := getExtensionSchemas(db) + if err != nil { + return fmt.Errorf("failed to query plan database extensions: %w", err) + } + + var mismatches []string + for name, planSchema := range planExtensions { + if targetSchema, ok := targetExtensions[name]; ok && targetSchema != planSchema { + mismatches = append(mismatches, fmt.Sprintf( + "extension %q is installed in schema %q on the plan database but schema %q on the target database", + name, planSchema, targetSchema)) + } + } + if len(mismatches) == 0 { + return nil + } + + sort.Strings(mismatches) + return fmt.Errorf( + "extension schema mismatch: %s; the plan database must mirror the target's extension schemas (use ALTER EXTENSION ... SET SCHEMA or reinstall with CREATE EXTENSION ... WITH SCHEMA), see https://www.pgschema.com/cli/plan-db", + strings.Join(mismatches, "; ")) +} + +// getExtensionSchemas returns the installation schema of every extension in the +// connected database, keyed by extension name. +func getExtensionSchemas(db *sql.DB) (map[string]string, error) { + rows, err := db.QueryContext(context.Background(), + "SELECT e.extname, n.nspname FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace") + if err != nil { + return nil, err + } + defer rows.Close() + + extensions := make(map[string]string) + for rows.Next() { + var name, schema string + if err := rows.Scan(&name, &schema); err != nil { + return nil, err + } + extensions[name] = schema + } + return extensions, rows.Err() +} + +// GetExtensionSchemasFromDB connects to a database and returns the installation +// schema of every installed extension, keyed by extension name. +// This is a convenience function that opens a connection, queries, and closes it. +func GetExtensionSchemasFromDB(host string, port int, database, user, password, sslmode string) (map[string]string, error) { + finalSSLMode := sslmode + if finalSSLMode == "" { + finalSSLMode = "prefer" + } + config := &util.ConnectionConfig{ + Host: host, + Port: port, + Database: database, + User: user, + Password: password, + SSLMode: finalSSLMode, + } + + db, err := util.Connect(config) + if err != nil { + return nil, fmt.Errorf("failed to connect to database: %w", err) + } + defer db.Close() + + return getExtensionSchemas(db) +} + // detectMajorVersion queries the database to determine its PostgreSQL major version func detectMajorVersion(db *sql.DB) (int, error) { ctx := context.Background() From 87b10f6637a0dbe3f19f9e16a8e671aabcc2c808 Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Sat, 15 Aug 2026 13:11:38 +0800 Subject: [PATCH 2/3] refactor: gather target version and extensions in one connection Co-Authored-By: Claude Fable 5 --- cmd/plan/external_db_integration_test.go | 2 +- cmd/plan/plan.go | 19 +++-------------- internal/postgres/embedded.go | 22 +++++++++++++++----- internal/postgres/external.go | 26 ------------------------ 4 files changed, 21 insertions(+), 48 deletions(-) diff --git a/cmd/plan/external_db_integration_test.go b/cmd/plan/external_db_integration_test.go index 7acb5901..dd92a343 100644 --- a/cmd/plan/external_db_integration_test.go +++ b/cmd/plan/external_db_integration_test.go @@ -103,7 +103,7 @@ func TestExternalDatabase_VersionMismatch(t *testing.T) { targetHost, targetPort, targetDatabase, targetUser, targetPassword := targetDB.GetConnectionDetails() // Detect version from target database - pgVersion, err := postgres.DetectPostgresVersionFromDB( + pgVersion, _, err := postgres.DetectPostgresVersionAndExtensionsFromDB( targetHost, targetPort, targetDatabase, diff --git a/cmd/plan/plan.go b/cmd/plan/plan.go index 5f56840b..93d2703c 100644 --- a/cmd/plan/plan.go +++ b/cmd/plan/plan.go @@ -199,7 +199,9 @@ type PlanConfig struct { // for validating the desired state schema. The caller is responsible for calling Stop() on the returned provider. func CreateDesiredStateProvider(config *PlanConfig) (postgres.DesiredStateProvider, error) { // Detect target database PostgreSQL version (needed for both embedded and external) - pgVersion, err := postgres.DetectPostgresVersionFromDB( + // and its extension installation schemas (needed for the external database's + // extension schema consistency check, issue #518) in a single connection. + pgVersion, targetExtensions, err := postgres.DetectPostgresVersionAndExtensionsFromDB( config.Host, config.Port, config.DB, @@ -221,21 +223,6 @@ func CreateDesiredStateProvider(config *PlanConfig) (postgres.DesiredStateProvid // If plan-host is provided, use external database if config.PlanDBHost != "" { - // Collect the target's extension installation schemas so the external - // database can verify shared extensions live in the same schema on both - // sides (issue #518). - targetExtensions, err := postgres.GetExtensionSchemasFromDB( - config.Host, - config.Port, - config.DB, - config.User, - config.Password, - config.SSLMode, - ) - if err != nil { - return nil, fmt.Errorf("failed to query target database extensions: %w", err) - } - externalConfig := &postgres.ExternalDatabaseConfig{ Host: config.PlanDBHost, Port: config.PlanDBPort, diff --git a/internal/postgres/embedded.go b/internal/postgres/embedded.go index 90c8ea4b..3113d9af 100644 --- a/internal/postgres/embedded.go +++ b/internal/postgres/embedded.go @@ -48,9 +48,11 @@ type EmbeddedPostgresConfig struct { Password string } -// DetectPostgresVersionFromDB connects to a database and detects its version -// This is a convenience function that opens a connection, detects the version, and closes it -func DetectPostgresVersionFromDB(host string, port int, database, user, password, sslmode string) (PostgresVersion, error) { +// DetectPostgresVersionAndExtensionsFromDB connects to a database and detects its +// version along with the installation schema of every installed extension (keyed by +// extension name). Gathering both over a single connection avoids a second +// connect/close round trip during plan generation. +func DetectPostgresVersionAndExtensionsFromDB(host string, port int, database, user, password, sslmode string) (PostgresVersion, map[string]string, error) { // Build connection config finalSSLMode := sslmode if finalSSLMode == "" { @@ -68,12 +70,22 @@ func DetectPostgresVersionFromDB(host string, port int, database, user, password // Connect to database db, err := util.Connect(config) if err != nil { - return "", fmt.Errorf("failed to connect to database: %w", err) + return "", nil, fmt.Errorf("failed to connect to database: %w", err) } defer db.Close() // Detect version - return detectPostgresVersion(db) + version, err := detectPostgresVersion(db) + if err != nil { + return "", nil, err + } + + extensions, err := getExtensionSchemas(db) + if err != nil { + return "", nil, fmt.Errorf("failed to query extensions: %w", err) + } + + return version, extensions, nil } // StartEmbeddedPostgres starts a temporary embedded PostgreSQL instance diff --git a/internal/postgres/external.go b/internal/postgres/external.go index b1d284cd..fc3e53f3 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -257,32 +257,6 @@ func getExtensionSchemas(db *sql.DB) (map[string]string, error) { return extensions, rows.Err() } -// GetExtensionSchemasFromDB connects to a database and returns the installation -// schema of every installed extension, keyed by extension name. -// This is a convenience function that opens a connection, queries, and closes it. -func GetExtensionSchemasFromDB(host string, port int, database, user, password, sslmode string) (map[string]string, error) { - finalSSLMode := sslmode - if finalSSLMode == "" { - finalSSLMode = "prefer" - } - config := &util.ConnectionConfig{ - Host: host, - Port: port, - Database: database, - User: user, - Password: password, - SSLMode: finalSSLMode, - } - - db, err := util.Connect(config) - if err != nil { - return nil, fmt.Errorf("failed to connect to database: %w", err) - } - defer db.Close() - - return getExtensionSchemas(db) -} - // detectMajorVersion queries the database to determine its PostgreSQL major version func detectMajorVersion(db *sql.DB) (int, error) { ctx := context.Background() From a2de9a3d9e048a7d9f194fc592403c9017dae80a Mon Sep 17 00:00:00 2001 From: Tianzhou Date: Sat, 15 Aug 2026 13:12:51 +0800 Subject: [PATCH 3/3] docs: clarify TargetExtensions is an optional validation input Co-Authored-By: Claude Fable 5 --- internal/postgres/external.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/internal/postgres/external.go b/internal/postgres/external.go index fc3e53f3..5c904471 100644 --- a/internal/postgres/external.go +++ b/internal/postgres/external.go @@ -35,10 +35,12 @@ type ExternalDatabaseConfig struct { SSLMode string TargetMajorVersion int // Expected major version to match // TargetExtensions maps extension name to its installation schema on the - // target database. Extensions installed on both databases must live in the - // same schema on each side; otherwise extension-owned types (e.g. pgvector's - // vector) resolve to different schema-qualified names and the generated plan - // is silently wrong (issue #518). + // target database. Optional: when populated (typically from querying the + // target database), NewExternalDatabase validates that extensions installed + // on both databases live in the same schema on each side; when empty or nil, + // the validation is skipped. Without it, extension-owned types (e.g. + // pgvector's vector) installed in different schemas resolve to different + // schema-qualified names and the generated plan is silently wrong (issue #518). TargetExtensions map[string]string }