diff --git a/cmd/dump/dump_integration_test.go b/cmd/dump/dump_integration_test.go index cb21bad3..8c2011cf 100644 --- a/cmd/dump/dump_integration_test.go +++ b/cmd/dump/dump_integration_test.go @@ -109,6 +109,92 @@ func TestDumpCommand_Issue252FunctionSchemaQualifier(t *testing.T) { runExactMatchTest(t, "issue_252_function_schema_qualifier") } +func TestDumpCommand_Issue307ViewDependencyOrder(t *testing.T) { + if testing.Short() { + t.Skip("Skipping integration test in short mode") + } + runExactMatchTest(t, "issue_307_view_dependency_order") +} + +func TestDumpCommand_Issue307MultiFileViewDependencyOrder(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() + + // Read and execute the pgdump.sql file + pgdumpPath := "../../testdata/dump/issue_307_view_dependency_order/pgdump.sql" + pgdumpContent, err := os.ReadFile(pgdumpPath) + if err != nil { + t.Fatalf("Failed to read %s: %v", pgdumpPath, err) + } + + // Execute the SQL to create the schema + _, err = conn.ExecContext(context.Background(), string(pgdumpContent)) + if err != nil { + t.Fatalf("Failed to execute pgdump.sql: %v", err) + } + + // Create temp directory for multi-file output + tmpDir := t.TempDir() + outputPath := tmpDir + "/schema.sql" + + // Create dump configuration for multi-file mode + config := &DumpConfig{ + Host: host, + Port: port, + DB: dbname, + User: user, + Password: password, + Schema: "public", + MultiFile: true, + File: outputPath, + } + + // Execute pgschema dump in multi-file mode + _, err = ExecuteDump(config) + if err != nil { + t.Fatalf("Dump command failed: %v", err) + } + + // Read the main schema file to check include order + mainContent, err := os.ReadFile(outputPath) + if err != nil { + t.Fatalf("Failed to read main file: %v", err) + } + + // Parse include directives to check view ordering + lines := strings.Split(string(mainContent), "\n") + var viewIncludes []string + for _, line := range lines { + trimmed := strings.TrimSpace(line) + if strings.HasPrefix(trimmed, `\i views/`) { + viewIncludes = append(viewIncludes, trimmed) + } + } + + // Verify we have both view includes + if len(viewIncludes) != 2 { + t.Fatalf("Expected 2 view includes, got %d: %v", len(viewIncludes), viewIncludes) + } + + // item_summary must come before dashboard because dashboard depends on item_summary + // (even though "dashboard" sorts before "item_summary" alphabetically) + if viewIncludes[0] != `\i views/item_summary.sql` { + t.Errorf("Expected first view include to be item_summary (dependency), got: %s", viewIncludes[0]) + } + if viewIncludes[1] != `\i views/dashboard.sql` { + t.Errorf("Expected second view include to be dashboard (depends on item_summary), got: %s", viewIncludes[1]) + } +} + func runExactMatchTest(t *testing.T, testDataDir string) { runExactMatchTestWithContext(t, context.Background(), testDataDir) } diff --git a/internal/dump/formatter.go b/internal/dump/formatter.go index 6ad8614d..6d1524cf 100644 --- a/internal/dump/formatter.go +++ b/internal/dump/formatter.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "regexp" - "sort" "strings" "github.com/pgplex/pgschema/internal/diff" @@ -84,9 +83,13 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er // Organization by object type filesByType := make(map[string]map[string][]diff.Diff) + // Track insertion order per directory to preserve dependency ordering from the diff package. + // The diff package topologically sorts views, functions, tables, and types, so preserving + // the order in which each object first appears maintains correct dependency ordering. + orderByDir := make(map[string][]string) includes := []string{} - // Group diffs by object type and name + // Group diffs by object type and name, tracking first-appearance order for _, step := range diffs { objType := step.Type.String() @@ -104,6 +107,11 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er filesByType[dir] = make(map[string][]diff.Diff) } + // Track first appearance of each object name per directory + if _, exists := filesByType[dir][objName]; !exists { + orderByDir[dir] = append(orderByDir[dir], objName) + } + filesByType[dir][objName] = append(filesByType[dir][objName], step) } @@ -118,12 +126,10 @@ func (f *DumpFormatter) FormatMultiFile(diffs []diff.Diff, outputPath string) er return fmt.Errorf("failed to create directory %s: %w", dirPath, err) } - // Sort object names for deterministic output (Go maps have random iteration order) - objNames := make([]string, 0, len(objects)) - for objName := range objects { - objNames = append(objNames, objName) - } - sort.Strings(objNames) + // Use the order objects first appeared in the diffs. + // This preserves dependency ordering from the diff package (e.g., topological + // sort for views, tables, functions) instead of sorting alphabetically. + objNames := orderByDir[dir] // Create files for each object for _, objName := range objNames { diff --git a/testdata/dump/issue_307_view_dependency_order/manifest.json b/testdata/dump/issue_307_view_dependency_order/manifest.json new file mode 100644 index 00000000..c82f488b --- /dev/null +++ b/testdata/dump/issue_307_view_dependency_order/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "issue_307_view_dependency_order", + "description": "Test case for view dependency ordering in dump output (GitHub issue #307)", + "source": "https://github.com/pgplex/pgschema/issues/307", + "notes": [ + "Reproduces the bug where views are emitted alphabetically instead of by dependency order", + "Tests that item_summary is emitted before dashboard (since dashboard depends on item_summary)", + "dashboard sorts before item_summary alphabetically, but must come after it in dependency order" + ] +} diff --git a/testdata/dump/issue_307_view_dependency_order/pgdump.sql b/testdata/dump/issue_307_view_dependency_order/pgdump.sql new file mode 100644 index 00000000..556aac43 --- /dev/null +++ b/testdata/dump/issue_307_view_dependency_order/pgdump.sql @@ -0,0 +1,67 @@ +-- +-- PostgreSQL database dump +-- + +-- Dumped from database version 17.5 (Debian 17.5-1.pgdg120+1) +-- Dumped by pg_dump version 17.5 (Homebrew) + +SET statement_timeout = 0; +SET lock_timeout = 0; +SET idle_in_transaction_session_timeout = 0; +SET client_encoding = 'UTF8'; +SET standard_conforming_strings = on; +SELECT pg_catalog.set_config('search_path', '', false); +SET check_function_bodies = false; +SET xmloption = content; +SET client_min_messages = warning; +SET row_security = off; + +SET default_tablespace = ''; + +SET default_table_access_method = heap; + +-- +-- Name: base_data; Type: TABLE; Schema: public; Owner: - +-- + +CREATE TABLE public.base_data ( + id integer NOT NULL, + value text NOT NULL, + category text +); + + +-- +-- Name: item_summary; Type: VIEW; Schema: public; Owner: - +-- + +CREATE VIEW public.item_summary AS + SELECT id, + value, + category + FROM public.base_data + WHERE (category IS NOT NULL); + + +-- +-- Name: dashboard; Type: VIEW; Schema: public; Owner: - +-- + +CREATE VIEW public.dashboard AS + SELECT id, + value + FROM public.item_summary; + + +-- +-- Name: base_data base_data_pkey; Type: CONSTRAINT; Schema: public; Owner: - +-- + +ALTER TABLE ONLY public.base_data + ADD CONSTRAINT base_data_pkey PRIMARY KEY (id); + + +-- +-- PostgreSQL database dump complete +-- + diff --git a/testdata/dump/issue_307_view_dependency_order/pgschema.sql b/testdata/dump/issue_307_view_dependency_order/pgschema.sql new file mode 100644 index 00000000..e3039a5c --- /dev/null +++ b/testdata/dump/issue_307_view_dependency_order/pgschema.sql @@ -0,0 +1,39 @@ +-- +-- pgschema database dump +-- + +-- Dumped from database version PostgreSQL 18.0 +-- Dumped by pgschema version 1.7.2 + + +-- +-- Name: base_data; Type: TABLE; Schema: -; Owner: - +-- + +CREATE TABLE IF NOT EXISTS base_data ( + id integer, + value text NOT NULL, + category text, + CONSTRAINT base_data_pkey PRIMARY KEY (id) +); + +-- +-- Name: item_summary; Type: VIEW; Schema: -; Owner: - +-- + +CREATE OR REPLACE VIEW item_summary AS + SELECT id, + value, + category + FROM base_data + WHERE category IS NOT NULL; + +-- +-- Name: dashboard; Type: VIEW; Schema: -; Owner: - +-- + +CREATE OR REPLACE VIEW dashboard AS + SELECT id, + value + FROM item_summary; + diff --git a/testdata/dump/issue_307_view_dependency_order/raw.sql b/testdata/dump/issue_307_view_dependency_order/raw.sql new file mode 100644 index 00000000..7b479e22 --- /dev/null +++ b/testdata/dump/issue_307_view_dependency_order/raw.sql @@ -0,0 +1,26 @@ +-- +-- Test case for GitHub issue #307: View dependency ordering +-- +-- This test reproduces a bug where views are emitted in alphabetical order +-- instead of dependency order. "dashboard" sorts before "item_summary" +-- alphabetically, but dashboard depends on item_summary and must come after it. +-- + +-- Base table +CREATE TABLE base_data ( + id integer PRIMARY KEY, + value text NOT NULL, + category text +); + +-- View that other views depend on (must be created first) +CREATE VIEW item_summary AS +SELECT id, value, category +FROM base_data +WHERE category IS NOT NULL; + +-- View that depends on item_summary (must be created second) +-- Alphabetically "dashboard" comes before "item_summary" +CREATE VIEW dashboard AS +SELECT id, value +FROM item_summary;