diff --git a/cmd/migrate_integration_test.go b/cmd/migrate_integration_test.go index 569199f4..de4d2210 100644 --- a/cmd/migrate_integration_test.go +++ b/cmd/migrate_integration_test.go @@ -239,8 +239,17 @@ func runPlanAndApplyTest(t *testing.T, ctx context.Context, container *struct { t.Fatalf("Failed to read setup.sql: %v", err) } if len(strings.TrimSpace(string(setupContent))) > 0 { + // Execute setup.sql to target database if err := executeSQL(ctx, containerHost, portMapped, dbName, string(setupContent)); err != nil { - t.Fatalf("Failed to execute setup.sql: %v", err) + t.Fatalf("Failed to execute setup.sql to target database: %v", err) + } + + // Also execute setup.sql to shared embedded postgres instance + // This ensures both databases have the setup objects available + embeddedConn, _, _, _, _, _ := testutil.ConnectToPostgres(t, sharedEmbeddedPG) + defer embeddedConn.Close() + if _, err := embeddedConn.ExecContext(ctx, string(setupContent)); err != nil { + t.Fatalf("Failed to execute setup.sql to embedded postgres: %v", err) } } } @@ -259,44 +268,19 @@ func runPlanAndApplyTest(t *testing.T, ctx context.Context, container *struct { } // STEP 2: Test plan command with new.sql as target - // If setup.sql exists, create a temporary combined file (setup + new) - schemaFileForPlan := tc.newFile - if _, err := os.Stat(tc.setupFile); err == nil { - setupContent, err := os.ReadFile(tc.setupFile) - if err != nil { - t.Fatalf("Failed to read setup.sql for plan: %v", err) - } - newContent, err := os.ReadFile(tc.newFile) - if err != nil { - t.Fatalf("Failed to read new.sql for plan: %v", err) - } - - // Create temporary combined file - combinedContent := string(setupContent) + "\n\n" + string(newContent) - tmpFile, err := os.CreateTemp("", "pgschema_test_*.sql") - if err != nil { - t.Fatalf("Failed to create temporary file: %v", err) - } - defer tmpFile.Close() - defer os.Remove(tmpFile.Name()) - - if _, err := tmpFile.WriteString(combinedContent); err != nil { - t.Fatalf("Failed to write to temporary file: %v", err) - } - schemaFileForPlan = tmpFile.Name() - } - - testPlanOutputs(t, container, dbName, schemaFileForPlan, tc.planSQLFile, tc.planJSONFile, tc.planTXTFile) + // Note: setup.sql has already been executed to both databases in STEP 0, + // so we only need to use new.sql for plan and apply operations + testPlanOutputs(t, container, dbName, tc.newFile, tc.planSQLFile, tc.planJSONFile, tc.planTXTFile) if !*generate { // STEP 3: Apply the migration using apply command - err = applySchemaChanges(containerHost, portMapped, dbName, container.User, container.Password, "public", schemaFileForPlan) + err = applySchemaChanges(containerHost, portMapped, dbName, container.User, container.Password, "public", tc.newFile) if err != nil { t.Fatalf("Failed to apply schema changes using pgschema apply: %v", err) } // STEP 4: Test idempotency - plan should produce no changes - secondPlanOutput, err := generatePlanSQLFormatted(containerHost, portMapped, dbName, container.User, container.Password, "public", schemaFileForPlan) + secondPlanOutput, err := generatePlanSQLFormatted(containerHost, portMapped, dbName, container.User, container.Password, "public", tc.newFile) if err != nil { t.Fatalf("Failed to generate plan SQL for idempotency check: %v", err) } diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go index f44d5325..bd05cf94 100644 --- a/internal/diff/diff_test.go +++ b/internal/diff/diff_test.go @@ -1,13 +1,13 @@ package diff import ( + "context" "os" "path/filepath" "strings" "testing" "github.com/pgschema/pgschema/internal/postgres" - "github.com/pgschema/pgschema/ir" "github.com/pgschema/pgschema/testutil" ) @@ -53,17 +53,6 @@ func buildSQLFromSteps(diffs []Diff) string { return sqlOutput.String() } -// parseSQLWithSetup applies optional setup SQL before main SQL, then parses to IR -// This allows tests to have shared setup (e.g., types in different schemas) before the main schema -func parseSQLWithSetup(t *testing.T, setup, sql string) *ir.IR { - t.Helper() - - // Combine setup and main SQL with separator if both exist - combinedSQL := setup + "\n\n" + sql - - return testutil.ParseSQLToIR(t, sharedTestPostgres, combinedSQL, "public") -} - // TestDiffFromFiles runs file-based diff tests from testdata directory. // It walks through the testdata/diff directory structure looking for test cases // that contain old.sql, new.sql, and plan.sql files. For each test case, @@ -165,13 +154,18 @@ func runFileBasedDiffTest(t *testing.T, oldFile, newFile, diffFile, testName str // Read optional setup.sql (for cross-schema setup, extension types, etc.) setupFile := filepath.Join(filepath.Dir(oldFile), "setup.sql") - var setupSQL string if _, err := os.Stat(setupFile); err == nil { setupContent, err := os.ReadFile(setupFile) if err != nil { t.Fatalf("Failed to read setup.sql: %v", err) } - setupSQL = string(setupContent) + + // Execute setup.sql once before parsing old/new SQL + // This creates shared infrastructure (e.g., custom types in utils schema) + // that will be available to both old.sql and new.sql + if _, err := conn.ExecContext(context.Background(), string(setupContent)); err != nil { + t.Fatalf("Failed to execute setup.sql: %v", err) + } } // Read old DDL @@ -192,9 +186,9 @@ func runFileBasedDiffTest(t *testing.T, oldFile, newFile, diffFile, testName str t.Fatalf("Failed to read plan.sql: %v", err) } - // Parse DDL to IR (with optional setup SQL) - oldIR := parseSQLWithSetup(t, setupSQL, string(oldDDL)) - newIR := parseSQLWithSetup(t, setupSQL, string(newDDL)) + // Parse DDL to IR (setup.sql already applied, so just parse old/new SQL directly) + oldIR := testutil.ParseSQLToIR(t, sharedTestPostgres, string(oldDDL), "public") + newIR := testutil.ParseSQLToIR(t, sharedTestPostgres, string(newDDL), "public") // Run diff diffs := GenerateMigration(oldIR, newIR, "public") diff --git a/ir/queries/queries.sql.go b/ir/queries/queries.sql.go index 838890c2..0641fc5e 100644 --- a/ir/queries/queries.sql.go +++ b/ir/queries/queries.sql.go @@ -216,6 +216,13 @@ SELECT CASE WHEN dn.nspname = c.table_schema THEN dt.typname ELSE dn.nspname || '.' || dt.typname END + WHEN dt.typtype = 'b' THEN + -- Base types: qualify if not in pg_catalog or table's schema + CASE + WHEN dn.nspname = 'pg_catalog' THEN c.udt_name + WHEN dn.nspname = c.table_schema THEN dt.typname + ELSE dn.nspname || '.' || dt.typname + END ELSE c.udt_name END AS resolved_type, c.is_identity, @@ -343,6 +350,13 @@ SELECT CASE WHEN dn.nspname = c.table_schema THEN dt.typname ELSE dn.nspname || '.' || dt.typname END + WHEN dt.typtype = 'b' THEN + -- Base types: qualify if not in pg_catalog or table's schema + CASE + WHEN dn.nspname = 'pg_catalog' THEN c.udt_name + WHEN dn.nspname = c.table_schema THEN dt.typname + ELSE dn.nspname || '.' || dt.typname + END ELSE c.udt_name END AS resolved_type, c.is_identity, diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/diff.sql b/testdata/diff/create_table/add_column_cross_schema_custom_type/diff.sql new file mode 100644 index 00000000..0a58b5c2 --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/diff.sql @@ -0,0 +1,4 @@ +ALTER TABLE users ADD COLUMN metadata utils.hstore; +ALTER TABLE users ADD COLUMN fqdn utils.citext NOT NULL; +ALTER TABLE users ADD COLUMN description utils.custom_text; +ALTER TABLE users ADD COLUMN status utils.custom_enum DEFAULT 'active'; \ No newline at end of file diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/new.sql b/testdata/diff/create_table/add_column_cross_schema_custom_type/new.sql new file mode 100644 index 00000000..72460f13 --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/new.sql @@ -0,0 +1,18 @@ +-- New state: Add columns using extension types, custom domain, and enum +-- Types are created via setup.sql +-- This tests GitHub #144 fix and PR #145 functionality: +-- - Extension types in external schemas (hstore) should be qualified +-- - Custom domains and enums in external schemas should be qualified + +CREATE TABLE public.users ( + id bigint PRIMARY KEY, + username text NOT NULL, + created_at timestamp DEFAULT CURRENT_TIMESTAMP, + -- Extension type from utils schema: must be qualified + metadata utils.hstore, + fqdn utils.citext NOT NULL, + -- Custom domain from utils schema: must be qualified + description utils.custom_text, + -- Enum type from utils schema: must be qualified + status utils.custom_enum DEFAULT 'active' +); \ No newline at end of file diff --git a/testdata/diff/create_table/add_column_custom_type/old.sql b/testdata/diff/create_table/add_column_cross_schema_custom_type/old.sql similarity index 100% rename from testdata/diff/create_table/add_column_custom_type/old.sql rename to testdata/diff/create_table/add_column_cross_schema_custom_type/old.sql diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.json b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.json new file mode 100644 index 00000000..e63e3aa0 --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.json @@ -0,0 +1,38 @@ +{ + "version": "1.0.0", + "pgschema_version": "1.4.1", + "created_at": "1970-01-01T00:00:00Z", + "source_fingerprint": { + "hash": "a9e46b459eae7b043790b1744ca6210ad942beafbef337ae3b787e120a2474eb" + }, + "groups": [ + { + "steps": [ + { + "sql": "ALTER TABLE users ADD COLUMN metadata utils.hstore;", + "type": "table.column", + "operation": "create", + "path": "public.users.metadata" + }, + { + "sql": "ALTER TABLE users ADD COLUMN fqdn utils.citext NOT NULL;", + "type": "table.column", + "operation": "create", + "path": "public.users.fqdn" + }, + { + "sql": "ALTER TABLE users ADD COLUMN description utils.custom_text;", + "type": "table.column", + "operation": "create", + "path": "public.users.description" + }, + { + "sql": "ALTER TABLE users ADD COLUMN status utils.custom_enum DEFAULT 'active';", + "type": "table.column", + "operation": "create", + "path": "public.users.status" + } + ] + } + ] +} diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.sql b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.sql new file mode 100644 index 00000000..ca0332ff --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.sql @@ -0,0 +1,7 @@ +ALTER TABLE users ADD COLUMN metadata utils.hstore; + +ALTER TABLE users ADD COLUMN fqdn utils.citext NOT NULL; + +ALTER TABLE users ADD COLUMN description utils.custom_text; + +ALTER TABLE users ADD COLUMN status utils.custom_enum DEFAULT 'active'; diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.txt b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.txt new file mode 100644 index 00000000..9093bdd5 --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/plan.txt @@ -0,0 +1,22 @@ +Plan: 1 to modify. + +Summary by type: + tables: 1 to modify + +Tables: + ~ users + + description (column) + + fqdn (column) + + metadata (column) + + status (column) + +DDL to be executed: +-------------------------------------------------- + +ALTER TABLE users ADD COLUMN metadata utils.hstore; + +ALTER TABLE users ADD COLUMN fqdn utils.citext NOT NULL; + +ALTER TABLE users ADD COLUMN description utils.custom_text; + +ALTER TABLE users ADD COLUMN status utils.custom_enum DEFAULT 'active'; diff --git a/testdata/diff/create_table/add_column_cross_schema_custom_type/setup.sql b/testdata/diff/create_table/add_column_cross_schema_custom_type/setup.sql new file mode 100644 index 00000000..82c5f550 --- /dev/null +++ b/testdata/diff/create_table/add_column_cross_schema_custom_type/setup.sql @@ -0,0 +1,16 @@ +-- Setup: Create extension type, custom domain, and enum to test type qualification +-- This reproduces GitHub #144 and validates PR #145 fixes +-- All types from external schemas (not the target schema) should be schema-qualified +-- This includes: +-- - Extension types (hstore) +-- - Custom domains and enums + +CREATE SCHEMA IF NOT EXISTS utils; + +CREATE DOMAIN utils.custom_text AS text; + +CREATE TYPE utils.custom_enum AS ENUM ('active', 'inactive', 'pending'); + +-- hstore type stays in utils schema +CREATE EXTENSION IF NOT EXISTS hstore SCHEMA utils; +CREATE EXTENSION IF NOT EXISTS citext SCHEMA utils; \ No newline at end of file diff --git a/testdata/diff/create_table/add_column_custom_type/diff.sql b/testdata/diff/create_table/add_column_custom_type/diff.sql deleted file mode 100644 index 16c3cfff..00000000 --- a/testdata/diff/create_table/add_column_custom_type/diff.sql +++ /dev/null @@ -1,3 +0,0 @@ -ALTER TABLE users ADD COLUMN email email_address NOT NULL; - -ALTER TABLE users ADD COLUMN status user_status DEFAULT 'active' NOT NULL; diff --git a/testdata/diff/create_table/add_column_custom_type/new.sql b/testdata/diff/create_table/add_column_custom_type/new.sql deleted file mode 100644 index 22d5c9c1..00000000 --- a/testdata/diff/create_table/add_column_custom_type/new.sql +++ /dev/null @@ -1,13 +0,0 @@ --- New state: Add columns using custom types from setup.sql --- Types (email_address, user_status) are created in setup.sql --- Use unqualified names - types will be resolved via setup.sql context - --- Modified table with new columns using custom types -CREATE TABLE public.users ( - id bigint PRIMARY KEY, - username text NOT NULL, - created_at timestamp DEFAULT CURRENT_TIMESTAMP, - -- New columns using types from setup.sql - email email_address NOT NULL, - status user_status NOT NULL DEFAULT 'active' -); diff --git a/testdata/diff/create_table/add_column_custom_type/plan.json b/testdata/diff/create_table/add_column_custom_type/plan.json deleted file mode 100644 index c46910be..00000000 --- a/testdata/diff/create_table/add_column_custom_type/plan.json +++ /dev/null @@ -1,26 +0,0 @@ -{ - "version": "1.0.0", - "pgschema_version": "1.4.1", - "created_at": "1970-01-01T00:00:00Z", - "source_fingerprint": { - "hash": "4c7808528324af927b7ef815b8b70beac95edd62ed24c6da247a89cdf74ebb0f" - }, - "groups": [ - { - "steps": [ - { - "sql": "ALTER TABLE users ADD COLUMN email email_address NOT NULL;", - "type": "table.column", - "operation": "create", - "path": "public.users.email" - }, - { - "sql": "ALTER TABLE users ADD COLUMN status user_status DEFAULT 'active' NOT NULL;", - "type": "table.column", - "operation": "create", - "path": "public.users.status" - } - ] - } - ] -} diff --git a/testdata/diff/create_table/add_column_custom_type/plan.sql b/testdata/diff/create_table/add_column_custom_type/plan.sql deleted file mode 100644 index 16c3cfff..00000000 --- a/testdata/diff/create_table/add_column_custom_type/plan.sql +++ /dev/null @@ -1,3 +0,0 @@ -ALTER TABLE users ADD COLUMN email email_address NOT NULL; - -ALTER TABLE users ADD COLUMN status user_status DEFAULT 'active' NOT NULL; diff --git a/testdata/diff/create_table/add_column_custom_type/plan.txt b/testdata/diff/create_table/add_column_custom_type/plan.txt deleted file mode 100644 index 0569c66a..00000000 --- a/testdata/diff/create_table/add_column_custom_type/plan.txt +++ /dev/null @@ -1,16 +0,0 @@ -Plan: 1 to modify. - -Summary by type: - tables: 1 to modify - -Tables: - ~ users - + email (column) - + status (column) - -DDL to be executed: --------------------------------------------------- - -ALTER TABLE users ADD COLUMN email email_address NOT NULL; - -ALTER TABLE users ADD COLUMN status user_status DEFAULT 'active' NOT NULL; diff --git a/testdata/diff/create_table/add_column_custom_type/setup.sql b/testdata/diff/create_table/add_column_custom_type/setup.sql deleted file mode 100644 index 6fc1a2d6..00000000 --- a/testdata/diff/create_table/add_column_custom_type/setup.sql +++ /dev/null @@ -1,14 +0,0 @@ --- Setup: Create types in public schema --- This simulates extension types like citext installed in public schema - -CREATE TYPE public.email_address AS ( - local_part text, - domain text -); - -CREATE TYPE public.user_status AS ENUM ( - 'active', - 'inactive', - 'suspended', - 'pending' -);