From fc190dca74babd10f36069ba5e354764c95f0637 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Mon, 31 Aug 2026 15:31:12 +1000 Subject: [PATCH 1/2] Add pull command for declarative schema exports Enumerates every table in one schema and drives each through the introspect -> render path, one desired-state file per table, reporting per-table outcomes (pulled / refused / error) instead of failing fast. --- internal/cli/cli.go | 16 ++- internal/cli/pull.go | 185 ++++++++++++++++++++++++++ internal/cli/pull_integration_test.go | 59 ++++++++ internal/cli/pull_test.go | 76 +++++++++++ 4 files changed, 334 insertions(+), 2 deletions(-) create mode 100644 internal/cli/pull.go create mode 100644 internal/cli/pull_integration_test.go create mode 100644 internal/cli/pull_test.go diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 234ed9b..f5ac754 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -1,6 +1,6 @@ // Package cli defines the pg-sprite command tree (Kong): migrate and -// status (the optimistic front door), diff and fmt (the declarative front -// door), and lint and suggest (the offline checker and advisor). +// status (the optimistic front door), pull, diff, and fmt (the declarative +// front door), and lint and suggest (the offline checker and advisor). package cli import ( @@ -21,6 +21,7 @@ type CLI struct { Version kong.VersionFlag `help:"Print version and exit."` Migrate MigrateCmd `cmd:"" help:"Run a schema change safely."` + Pull PullCmd `cmd:"" help:"Export live tables as desired-state schema files."` Diff DiffCmd `cmd:"" help:"Diff a desired-state schema file against the live schema."` Fmt FmtCmd `cmd:"" help:"Canonicalize a schema file."` Lint LintCmd `cmd:"" help:"Lint DDL for unsafe patterns."` @@ -122,6 +123,17 @@ func (c *MigrateCmd) Validate() error { // Run implements the migrate subcommand. func (c *MigrateCmd) Run() error { return c.run(context.Background(), os.Stdout) } +// PullCmd exports every table in one schema as a desired-state schema file. +type PullCmd struct { + DBFlags `embed:""` + + Schema string `help:"Schema containing the tables to export." default:"public"` + Out string `help:"Directory for the exported .sql files." short:"o" default:"schema" type:"path"` +} + +// Run implements the pull subcommand. +func (c *PullCmd) Run() error { return c.run(context.Background(), os.Stdout) } + // DiffCmd derives statements from a desired-state schema (declarative // front-end): introspect the live table, materialize the desired state on a // rolled-back scratch schema, and print the ordered plan without executing diff --git a/internal/cli/pull.go b/internal/cli/pull.go new file mode 100644 index 0000000..6ce8c80 --- /dev/null +++ b/internal/cli/pull.go @@ -0,0 +1,185 @@ +package cli + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "path/filepath" + + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/schemadiff" + "github.com/block/pg-sprite/pkg/verdict" +) + +// ErrPullFailed is returned when at least one table could not be +// introspected or written. Render refusals use verdict.ErrRefused instead. +var ErrPullFailed = errors.New("one or more tables could not be pulled") + +type pullStatus string + +const ( + pullStatusPulled pullStatus = "pulled" + pullStatusRefused pullStatus = "refused" + pullStatusError pullStatus = "error" +) + +type pullResult struct { + table string + path string + status pullStatus + err error +} + +type tablePuller func(context.Context, *pgxpool.Pool, string, string, string) error + +func (c *PullCmd) run(ctx context.Context, out io.Writer) error { + pool, err := dbconn.NewPool(ctx, c.Config()) + if err != nil { + return err + } + defer pool.Close() + + if err := os.MkdirAll(c.Out, 0o755); err != nil { + return fmt.Errorf("create output directory %s: %w", c.Out, err) + } + tables, err := listTables(ctx, pool, c.Schema) + if err != nil { + return err + } + results := pullTables(ctx, pool, c.Schema, c.Out, tables, pullOneTable) + if err := writePullText(out, results); err != nil { + return err + } + return pullResultsError(results) +} + +func listTables(ctx context.Context, pool *pgxpool.Pool, schema string) ([]string, error) { + rows, err := pool.Query(ctx, ` + SELECT c.relname + FROM pg_catalog.pg_class c + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE n.nspname = $1 AND c.relkind IN ('r', 'p') + ORDER BY c.relname`, schema) + if err != nil { + return nil, fmt.Errorf("list tables in schema %s: %w", schema, err) + } + defer rows.Close() + var tables []string + for rows.Next() { + var table string + if err := rows.Scan(&table); err != nil { + return nil, fmt.Errorf("scan table in schema %s: %w", schema, err) + } + tables = append(tables, table) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("read tables in schema %s: %w", schema, err) + } + return tables, nil +} + +func pullTables(ctx context.Context, pool *pgxpool.Pool, schema, outDir string, tables []string, pull tablePuller) []pullResult { + results := make([]pullResult, 0, len(tables)) + for _, table := range tables { + path, err := tableOutputPath(outDir, table) + if err != nil { + results = append(results, pullResult{table: table, status: pullStatusError, err: err}) + continue + } + err = pull(ctx, pool, schema, table, path) + result := pullResult{table: table, path: path, status: pullStatusPulled} + if err != nil { + result.status = pullStatusError + result.err = err + var refusal *renderRefusal + if errors.As(err, &refusal) { + result.status = pullStatusRefused + } + } + results = append(results, result) + } + return results +} + +func tableOutputPath(outDir, table string) (string, error) { + name := table + ".sql" + if filepath.Base(name) != name || name == ".sql" { + return "", fmt.Errorf("table %q cannot be represented as a safe file name", table) + } + return filepath.Join(outDir, name), nil +} + +type renderRefusal struct{ err error } + +func (e *renderRefusal) Error() string { return e.err.Error() } +func (e *renderRefusal) Unwrap() error { return e.err } + +func pullOneTable(ctx context.Context, pool *pgxpool.Pool, schema, table, path string) error { + model, err := schemadiff.Introspect(ctx, pool, schema, table) + if err != nil { + return fmt.Errorf("introspect %s.%s: %w", schema, table, err) + } + rendered, err := schemadiff.Render(model) + if err != nil { + return &renderRefusal{err: err} + } + return pullRenderedFile(path, rendered) +} + +func pullRenderedFile(path, rendered string) error { + file, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o644) + if err != nil { + return fmt.Errorf("create %s: %w", path, err) + } + if _, err := io.WriteString(file, rendered); err != nil { + closeErr := file.Close() + removeErr := os.Remove(path) + return errors.Join(fmt.Errorf("write %s: %w", path, err), closeErr, removeErr) + } + if err := file.Close(); err != nil { + return fmt.Errorf("close %s: %w", path, err) + } + return nil +} + +func writePullText(out io.Writer, results []pullResult) error { + counts := map[pullStatus]int{} + for _, result := range results { + counts[result.status]++ + var err error + switch result.status { + case pullStatusPulled: + _, err = fmt.Fprintf(out, "PULLED %s -> %s\n", result.table, result.path) + case pullStatusRefused: + _, err = fmt.Fprintf(out, "REFUSED %s: %v\n", result.table, result.err) + case pullStatusError: + _, err = fmt.Fprintf(out, "ERROR %s: %v\n", result.table, result.err) + } + if err != nil { + return fmt.Errorf("write pull report: %w", err) + } + } + if _, err := fmt.Fprintf(out, "Summary: %d pulled, %d refused, %d errors\n", + counts[pullStatusPulled], counts[pullStatusRefused], counts[pullStatusError]); err != nil { + return fmt.Errorf("write pull report: %w", err) + } + return nil +} + +func pullResultsError(results []pullResult) error { + refused := false + for _, result := range results { + if result.status == pullStatusError { + return ErrPullFailed + } + refused = refused || result.status == pullStatusRefused + } + if refused { + return verdict.ErrRefused + } + return nil +} diff --git a/internal/cli/pull_integration_test.go b/internal/cli/pull_integration_test.go new file mode 100644 index 0000000..f79f3b9 --- /dev/null +++ b/internal/cli/pull_integration_test.go @@ -0,0 +1,59 @@ +package cli + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/internal/testutil" + "github.com/block/pg-sprite/pkg/dbconn" + "github.com/block/pg-sprite/pkg/lint" + "github.com/block/pg-sprite/pkg/verdict" +) + +func TestPullExportsAllRenderableTablesAndReportsRefusals(t *testing.T) { + url := testutil.StartPostgres(t) + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: url}) + require.NoError(t, err) + t.Cleanup(pool.Close) + schema := testutil.NewSchema(t, pool) + + for _, ddl := range []string{ + fmt.Sprintf("CREATE TABLE %s.accounts (id bigint PRIMARY KEY, name text NOT NULL)", schema), + fmt.Sprintf("CREATE TABLE %s.events (id bigint PRIMARY KEY, created_at timestamptz DEFAULT now())", schema), + fmt.Sprintf("CREATE INDEX events_created_at_idx ON %s.events (created_at)", schema), + fmt.Sprintf("CREATE TABLE %s.metrics (id bigint, day date) PARTITION BY RANGE (day)", schema), + } { + _, err := pool.Exec(t.Context(), ddl) + require.NoError(t, err) + } + + outDir := filepath.Join(t.TempDir(), "pulled") + cmd := &PullCmd{DBFlags: DBFlags{URL: url}, Schema: schema, Out: outDir} + var out strings.Builder + err = cmd.run(t.Context(), &out) + require.ErrorIs(t, err, verdict.ErrRefused) + + assert.Contains(t, out.String(), "PULLED accounts -> ") + assert.Contains(t, out.String(), "PULLED events -> ") + assert.Contains(t, out.String(), "REFUSED metrics: render table \"metrics\": partitioned parent") + assert.Contains(t, out.String(), "Summary: 2 pulled, 1 refused, 0 errors") + + entries, err := os.ReadDir(outDir) + require.NoError(t, err) + require.Len(t, entries, 2) + assert.Equal(t, "accounts.sql", entries[0].Name()) + assert.Equal(t, "events.sql", entries[1].Name()) + for _, entry := range entries { + raw, err := os.ReadFile(filepath.Join(outDir, entry.Name())) + require.NoError(t, err) + report, err := lint.Check(string(raw)) + require.NoError(t, err, entry.Name()) + assert.Zero(t, report.Errors, entry.Name()) + } +} diff --git a/internal/cli/pull_test.go b/internal/cli/pull_test.go new file mode 100644 index 0000000..6d909fb --- /dev/null +++ b/internal/cli/pull_test.go @@ -0,0 +1,76 @@ +package cli + +import ( + "context" + "errors" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/verdict" +) + +func TestPullTablesContinuesAfterFailures(t *testing.T) { + wantErr := errors.New("database read failed") + called := make([]string, 0, 3) + pull := func(_ context.Context, _ *pgxpool.Pool, _, table, _ string) error { + called = append(called, table) + switch table { + case "bad_render": + return &renderRefusal{err: errors.New("unsupported table")} + case "bad_read": + return wantErr + default: + return nil + } + } + + results := pullTables(t.Context(), nil, "public", t.TempDir(), + []string{"bad_render", "good", "bad_read"}, pull) + + assert.Equal(t, []string{"bad_render", "good", "bad_read"}, called) + require.Len(t, results, 3) + assert.Equal(t, pullStatusRefused, results[0].status) + assert.Equal(t, pullStatusPulled, results[1].status) + assert.Equal(t, pullStatusError, results[2].status) + assert.ErrorIs(t, pullResultsError(results), ErrPullFailed) +} + +func TestPullResultsErrorReturnsRefusalWhenNoOperationalErrors(t *testing.T) { + results := []pullResult{{status: pullStatusPulled}, {status: pullStatusRefused}} + assert.ErrorIs(t, pullResultsError(results), verdict.ErrRefused) +} + +func TestTableOutputPathRejectsPathSeparators(t *testing.T) { + _, err := tableOutputPath(t.TempDir(), "../outside") + require.Error(t, err) +} + +func TestWritePullTextSummarizesOutcomes(t *testing.T) { + results := []pullResult{ + {table: "accounts", path: "schema/accounts.sql", status: pullStatusPulled}, + {table: "metrics", status: pullStatusRefused, err: errors.New("partitioned")}, + {table: "events", status: pullStatusError, err: errors.New("exists")}, + } + var out strings.Builder + require.NoError(t, writePullText(&out, results)) + assert.Equal(t, "PULLED accounts -> schema/accounts.sql\n"+ + "REFUSED metrics: partitioned\n"+ + "ERROR events: exists\n"+ + "Summary: 1 pulled, 1 refused, 1 errors\n", out.String()) +} + +func TestPullOneTableDoesNotOverwriteExistingFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "events.sql") + require.NoError(t, os.WriteFile(path, []byte("keep me"), 0o600)) + err := pullRenderedFile(path, "replacement") + require.Error(t, err) + got, readErr := os.ReadFile(path) + require.NoError(t, readErr) + assert.Equal(t, "keep me", string(got)) +} From 39aff8d2f59f4853a15f29ebc66f1848b1f8d974 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Mon, 31 Aug 2026 17:03:53 +1000 Subject: [PATCH 2/2] Refuse classic inheritance in export; harden pull edge cases Review found INHERITS children exported as silently lossy baselines (flattened columns, zero diff). Introspection now records inheritance edges both ways and Render refuses both sides. Also: extension-owned tables excluded, missing schema errors instead of exiting 0, truncated files removed on Close failure, case-collision detection, exhaustive status switch, partition children filtered, create-only contract documented. --- README.md | 1 + docs/capabilities.md | 3 +- docs/limitations.md | 3 ++ internal/cli/cli.go | 4 +- internal/cli/pull.go | 41 +++++++++++++++++-- internal/cli/pull_integration_test.go | 31 ++++++++++++++ internal/cli/pull_test.go | 38 +++++++++++++++++ pkg/schemadiff/introspect.go | 50 +++++++++++++++++++++++ pkg/schemadiff/render.go | 11 +++++ pkg/schemadiff/render_integration_test.go | 26 ++++++++++++ pkg/schemadiff/render_test.go | 12 ++++++ pkg/schemadiff/schemadiff.go | 8 ++++ 12 files changed, 221 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 1d5b0fb..855421b 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,7 @@ change — every other command is read-only or fully offline. | `migrate` | required | Resolve the target table, preflight it (privileges, partitioning, size and catalog facts), classify and route the change, then **execute** the routed SQL under bounded budgets | | `migrate --dry-run` | required | The same introspection as a real run — server version, target resolution, table facts — so the printed plan reflects the actual target; executes nothing | | `diff` | required | Introspect the live table (read-only) and materialize the desired-state file on a scratch schema inside a transaction that is always rolled back; prints the plan, changes nothing | +| `pull` | required | Introspect each supported table in a schema and create one desired-state file per table; existing files are never overwritten | | `status` | required | Read-only view over `pg_stat_activity` for live pg-sprite sessions on the connected database | | `fmt` | none | Canonicalize a schema file — parser only | | `lint` | none | Flag patterns the engine would refuse, rewrite, or gate, from the DDL text alone | diff --git a/docs/capabilities.md b/docs/capabilities.md index 4185aee..fc49a2e 100644 --- a/docs/capabilities.md +++ b/docs/capabilities.md @@ -125,7 +125,7 @@ Support differs by front door, so the matrix marks the exceptions: file against the live table. This door depends on the canonical table *model*, which is deliberately narrower: a table the model cannot fully describe gets a typed refusal rather than a silently lossy description. Today that means tables that are - partitioned (or are partitions), own or are referenced by foreign keys, are unlogged, + partitioned (or are partitions), participate in classic table inheritance, own or are referenced by foreign keys, are unlogged, carry explicit collations, or take defaults from sequences they do not own. An operation can therefore be T1 imperatively and T2 declaratively — foreign keys are @@ -193,6 +193,7 @@ Status legend: ✅ T1 (supported today) · 🟡 T2 (planned; typed refusal today | Table shape | Status | Engine path | Online-safety problem? | Behavior and why | | --- | --- | --- | --- | --- | | Plain tables + their indexes | ✅ | native, as-is | Yes | `diff`, `pull`, and desired-file rendering round-trip the canonical model | +| Classic table inheritance (`INHERITS`) | 🟡 | native, planned flow | Yes | Typed refusal for both parents and children: the model cannot express inheritance edges, and flattening inherited columns would produce a silently lossy baseline | | Tables that own **or are referenced by** foreign keys | 🟡 | native, planned flow | Yes | Typed refusal on both sides — an incoming FK cannot be expressed in the table's own desired file, and a lossy description would be worse than none. Declarative FK support (composite keys as the primary case, two-phase `NOT VALID` → `VALIDATE` execution) is planned | | Unlogged tables | 🟡 | native, planned flow | Yes | Typed refusal: persistence is not modeled, converging it (`SET LOGGED`) is a full rewrite, and rendering the table as plain `CREATE TABLE` would silently change crash-safety | | Explicit column collations | 🟡 | native, planned flow | Yes | Typed refusal: dropping a `COLLATE` clause from a rendered baseline silently changes sort order and index semantics; a collation delta cannot converge without a rewrite | diff --git a/docs/limitations.md b/docs/limitations.md index 5a13998..545bbe1 100644 --- a/docs/limitations.md +++ b/docs/limitations.md @@ -25,7 +25,10 @@ with a typed refusal — never a silently wrong or incomplete result: | --- | --- | | Foreign keys (either direction) | Unsupported in the declarative model, in both directions. A desired file cannot declare a `REFERENCES` clause (refused at parse), and export refuses both sides of a foreign-key relationship: a table whose definition carries foreign-key constraints surfaces the parse gate's typed error, and a table that other tables reference refuses with its own typed error — a single-table baseline cannot carry incoming foreign-key topology, so rendering one would silently drop the relationship. Foreign-key **DDL is still supported through the statement front door** (`ADD FOREIGN KEY` routes to the online `NOT VALID` + `VALIDATE` sequence). Because a desired file can never declare a foreign key, `diff` on a live table that carries one plans a **destructive** `DROP CONSTRAINT` for it — gated like every destructive change, never auto-executed — and incoming foreign keys are invisible to a single-table diff entirely, so tables participating in foreign-key relationships in either direction should not be managed declaratively yet. | | Partitioned tables | Partitioned parents and their partitions are introspectable, and the statement front door supports in-place changes on them (see the partitioned-parent rows above), but they cannot be expressed in or exported to a desired file: the model captures the partition key and attachment only to refuse — it does not carry partition bounds or the parent/partition topology. A partitioning mismatch between live and desired is a typed `diff` refusal, never a zero diff. | +| Classic table inheritance (`INHERITS`) | Both parents and children are refused during export. The model records classic inheritance edges in both directions only to fail closed: rendering a child would flatten inherited columns and rendering a parent would omit its children. Declarative partitions share `pg_inherits` but remain classified by `relispartition` and use the partition refusal above. | | Unlogged tables | Persistence is not managed: converging it (`SET LOGGED` / `SET UNLOGGED`) is a full table rewrite. Export refuses an unlogged table — a plain `CREATE TABLE` baseline would silently change its crash-safety and replication behavior — and a persistence mismatch between live and desired is a typed `diff` refusal, never a zero diff. | +| Table and column comments | Comments are metadata outside the table-shape model. Desired files and exports do not carry them; manage them with owner tooling. | +| Storage parameters (`fillfactor`, etc.) | Storage parameters are not represented in desired files or exports. Manage them separately until the declarative model can classify their convergence and rewrite implications. | | Column collations | An explicit `COLLATE` on a column is not managed: converging a collation delta rewrites the column and its indexes. Export refuses a collated column — a baseline without the clause would silently change sort order and index semantics — and a collation delta (including on an added column) is a typed `diff` refusal. | | Non-table objects | Views, materialized views, standalone sequences, enums, domains, extensions, functions, and triggers are outside the model. A serial column's owned sequence is the one exception: it round-trips through the `serial` pseudo-types — and ownership is verified through the catalog (`pg_depend`), so a hand-written `nextval` default on a standalone sequence that merely carries the serial-style name refuses rather than exporting as `serial` and silently privatizing a shared sequence. A column may *use* an unmanaged type (an enum, a domain) — the type text round-trips — but the type's definition is not managed. | | Multiple tables per file | A desired file is single-table scoped: exactly one `CREATE TABLE` plus `CREATE INDEX` statements on it. Multi-table schemas are managed as one file per table. | diff --git a/internal/cli/cli.go b/internal/cli/cli.go index f5ac754..b52bf1c 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -21,7 +21,7 @@ type CLI struct { Version kong.VersionFlag `help:"Print version and exit."` Migrate MigrateCmd `cmd:"" help:"Run a schema change safely."` - Pull PullCmd `cmd:"" help:"Export live tables as desired-state schema files."` + Pull PullCmd `cmd:"" help:"Export live tables to new desired-state schema files. Output is create-only: a second run into a populated directory fails per table; delete or move existing files to refresh them."` Diff DiffCmd `cmd:"" help:"Diff a desired-state schema file against the live schema."` Fmt FmtCmd `cmd:"" help:"Canonicalize a schema file."` Lint LintCmd `cmd:"" help:"Lint DDL for unsafe patterns."` @@ -128,7 +128,7 @@ type PullCmd struct { DBFlags `embed:""` Schema string `help:"Schema containing the tables to export." default:"public"` - Out string `help:"Directory for the exported .sql files." short:"o" default:"schema" type:"path"` + Out string `help:"Directory for create-only exported .sql files; delete or move existing files before refreshing." short:"o" default:"schema" type:"path"` } // Run implements the pull subcommand. diff --git a/internal/cli/pull.go b/internal/cli/pull.go index 6ce8c80..8956d0c 100644 --- a/internal/cli/pull.go +++ b/internal/cli/pull.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/jackc/pgx/v5/pgxpool" @@ -43,13 +44,20 @@ func (c *PullCmd) run(ctx context.Context, out io.Writer) error { } defer pool.Close() - if err := os.MkdirAll(c.Out, 0o755); err != nil { - return fmt.Errorf("create output directory %s: %w", c.Out, err) + var schemaExists bool + if err := pool.QueryRow(ctx, `SELECT EXISTS (SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = $1)`, c.Schema).Scan(&schemaExists); err != nil { + return fmt.Errorf("check schema %s: %w", c.Schema, err) + } + if !schemaExists { + return fmt.Errorf("schema %q does not exist", c.Schema) } tables, err := listTables(ctx, pool, c.Schema) if err != nil { return err } + if err := os.MkdirAll(c.Out, 0o755); err != nil { + return fmt.Errorf("create output directory %s: %w", c.Out, err) + } results := pullTables(ctx, pool, c.Schema, c.Out, tables, pullOneTable) if err := writePullText(out, results); err != nil { return err @@ -63,6 +71,12 @@ func listTables(ctx context.Context, pool *pgxpool.Pool, schema string) ([]strin FROM pg_catalog.pg_class c JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace WHERE n.nspname = $1 AND c.relkind IN ('r', 'p') + AND NOT c.relispartition + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_depend d + WHERE d.classid = 'pg_class'::regclass + AND d.objid = c.oid AND d.deptype = 'e' + ) ORDER BY c.relname`, schema) if err != nil { return nil, fmt.Errorf("list tables in schema %s: %w", schema, err) @@ -84,12 +98,26 @@ func listTables(ctx context.Context, pool *pgxpool.Pool, schema string) ([]strin func pullTables(ctx context.Context, pool *pgxpool.Pool, schema, outDir string, tables []string, pull tablePuller) []pullResult { results := make([]pullResult, 0, len(tables)) + seenPaths := make(map[string]string, len(tables)) for _, table := range tables { + if err := ctx.Err(); err != nil { + results = append(results, pullResult{table: table, status: pullStatusError, err: err}) + break + } path, err := tableOutputPath(outDir, table) if err != nil { results = append(results, pullResult{table: table, status: pullStatusError, err: err}) continue } + pathKey := strings.ToLower(filepath.Base(path)) + if other, ok := seenPaths[pathKey]; ok { + results = append(results, pullResult{ + table: table, status: pullStatusError, + err: fmt.Errorf("output file name has a case collision with table %q", other), + }) + continue + } + seenPaths[pathKey] = table err = pull(ctx, pool, schema, table, path) result := pullResult{table: table, path: path, status: pullStatusPulled} if err != nil { @@ -141,7 +169,8 @@ func pullRenderedFile(path, rendered string) error { return errors.Join(fmt.Errorf("write %s: %w", path, err), closeErr, removeErr) } if err := file.Close(); err != nil { - return fmt.Errorf("close %s: %w", path, err) + removeErr := os.Remove(path) + return errors.Join(fmt.Errorf("close %s: %w", path, err), removeErr) } return nil } @@ -149,15 +178,19 @@ func pullRenderedFile(path, rendered string) error { func writePullText(out io.Writer, results []pullResult) error { counts := map[pullStatus]int{} for _, result := range results { - counts[result.status]++ var err error switch result.status { case pullStatusPulled: + counts[result.status]++ _, err = fmt.Fprintf(out, "PULLED %s -> %s\n", result.table, result.path) case pullStatusRefused: + counts[result.status]++ _, err = fmt.Fprintf(out, "REFUSED %s: %v\n", result.table, result.err) case pullStatusError: + counts[result.status]++ _, err = fmt.Fprintf(out, "ERROR %s: %v\n", result.table, result.err) + default: + return fmt.Errorf("write pull report: unexpected pull status %q for table %q", result.status, result.table) } if err != nil { return fmt.Errorf("write pull report: %w", err) diff --git a/internal/cli/pull_integration_test.go b/internal/cli/pull_integration_test.go index f79f3b9..e0d85a2 100644 --- a/internal/cli/pull_integration_test.go +++ b/internal/cli/pull_integration_test.go @@ -28,6 +28,7 @@ func TestPullExportsAllRenderableTablesAndReportsRefusals(t *testing.T) { fmt.Sprintf("CREATE TABLE %s.events (id bigint PRIMARY KEY, created_at timestamptz DEFAULT now())", schema), fmt.Sprintf("CREATE INDEX events_created_at_idx ON %s.events (created_at)", schema), fmt.Sprintf("CREATE TABLE %s.metrics (id bigint, day date) PARTITION BY RANGE (day)", schema), + fmt.Sprintf("CREATE TABLE %s.metrics_2026 PARTITION OF %s.metrics FOR VALUES FROM ('2026-01-01') TO ('2027-01-01')", schema, schema), } { _, err := pool.Exec(t.Context(), ddl) require.NoError(t, err) @@ -42,6 +43,7 @@ func TestPullExportsAllRenderableTablesAndReportsRefusals(t *testing.T) { assert.Contains(t, out.String(), "PULLED accounts -> ") assert.Contains(t, out.String(), "PULLED events -> ") assert.Contains(t, out.String(), "REFUSED metrics: render table \"metrics\": partitioned parent") + assert.NotContains(t, out.String(), "metrics_2026") assert.Contains(t, out.String(), "Summary: 2 pulled, 1 refused, 0 errors") entries, err := os.ReadDir(outDir) @@ -57,3 +59,32 @@ func TestPullExportsAllRenderableTablesAndReportsRefusals(t *testing.T) { assert.Zero(t, report.Errors, entry.Name()) } } + +func TestPullRefusesBothSidesOfClassicInheritance(t *testing.T) { + url := testutil.StartPostgres(t) + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: url}) + require.NoError(t, err) + t.Cleanup(pool.Close) + schema := testutil.NewSchema(t, pool) + _, err = pool.Exec(t.Context(), fmt.Sprintf("CREATE TABLE %s.parent (id bigint NOT NULL)", schema)) + require.NoError(t, err) + _, err = pool.Exec(t.Context(), fmt.Sprintf("CREATE TABLE %s.child (note text) INHERITS (%s.parent)", schema, schema)) + require.NoError(t, err) + + var out strings.Builder + err = (&PullCmd{DBFlags: DBFlags{URL: url}, Schema: schema, Out: t.TempDir()}).run(t.Context(), &out) + require.ErrorIs(t, err, verdict.ErrRefused) + assert.Contains(t, out.String(), "REFUSED child:") + assert.Contains(t, out.String(), "REFUSED parent:") + assert.Contains(t, out.String(), "Summary: 0 pulled, 2 refused, 0 errors") +} + +func TestPullRejectsMissingSchema(t *testing.T) { + url := testutil.StartPostgres(t) + outDir := filepath.Join(t.TempDir(), "must-not-exist") + var out strings.Builder + err := (&PullCmd{DBFlags: DBFlags{URL: url}, Schema: "missing_schema", Out: outDir}).run(t.Context(), &out) + require.ErrorContains(t, err, `schema "missing_schema" does not exist`) + assert.NoDirExists(t, outDir) + assert.Empty(t, out.String()) +} diff --git a/internal/cli/pull_test.go b/internal/cli/pull_test.go index 6d909fb..dcb664d 100644 --- a/internal/cli/pull_test.go +++ b/internal/cli/pull_test.go @@ -41,6 +41,37 @@ func TestPullTablesContinuesAfterFailures(t *testing.T) { assert.ErrorIs(t, pullResultsError(results), ErrPullFailed) } +func TestPullTablesReportsCaseFoldedPathCollision(t *testing.T) { + var called []string + pull := func(_ context.Context, _ *pgxpool.Pool, _, table, _ string) error { + called = append(called, table) + return nil + } + + results := pullTables(t.Context(), nil, "public", t.TempDir(), []string{"Accounts", "accounts"}, pull) + + assert.Equal(t, []string{"Accounts"}, called) + require.Len(t, results, 2) + assert.Equal(t, pullStatusPulled, results[0].status) + assert.Equal(t, pullStatusError, results[1].status) + assert.ErrorContains(t, results[1].err, `case collision with table "Accounts"`) +} + +func TestPullTablesStopsAfterContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(t.Context()) + pull := func(_ context.Context, _ *pgxpool.Pool, _, _, _ string) error { + cancel() + return nil + } + + results := pullTables(ctx, nil, "public", t.TempDir(), []string{"first", "second", "third"}, pull) + + require.Len(t, results, 2) + assert.Equal(t, pullStatusPulled, results[0].status) + assert.Equal(t, "second", results[1].table) + assert.ErrorIs(t, results[1].err, context.Canceled) +} + func TestPullResultsErrorReturnsRefusalWhenNoOperationalErrors(t *testing.T) { results := []pullResult{{status: pullStatusPulled}, {status: pullStatusRefused}} assert.ErrorIs(t, pullResultsError(results), verdict.ErrRefused) @@ -65,6 +96,13 @@ func TestWritePullTextSummarizesOutcomes(t *testing.T) { "Summary: 1 pulled, 1 refused, 1 errors\n", out.String()) } +func TestWritePullTextRejectsUnknownStatus(t *testing.T) { + var out strings.Builder + err := writePullText(&out, []pullResult{{table: "events", status: pullStatus("future")}}) + require.ErrorContains(t, err, `unexpected pull status "future"`) + assert.Empty(t, out.String()) +} + func TestPullOneTableDoesNotOverwriteExistingFile(t *testing.T) { path := filepath.Join(t.TempDir(), "events.sql") require.NoError(t, os.WriteFile(path, []byte("keep me"), 0o600)) diff --git a/pkg/schemadiff/introspect.go b/pkg/schemadiff/introspect.go index ccb4799..3bff6fd 100644 --- a/pkg/schemadiff/introspect.go +++ b/pkg/schemadiff/introspect.go @@ -77,6 +77,9 @@ func introspectInTx(ctx context.Context, tx pgx.Tx, schema, table string) (Model } m := Model{Table: table, PartitionKey: partitionKey, IsPartition: isPartition, Unlogged: persistence == "u"} + if m.InheritsParents, m.InheritanceChildren, err = introspectInheritance(ctx, tx, oid); err != nil { + return Model{}, fmt.Errorf("introspect inheritance of %s.%s: %w", schema, table, err) + } if m.Columns, err = introspectColumns(ctx, tx, oid); err != nil { return Model{}, fmt.Errorf("introspect columns of %s.%s: %w", schema, table, err) } @@ -92,6 +95,53 @@ func introspectInTx(ctx context.Context, tx pgx.Tx, schema, table string) (Model return m, nil } +// introspectInheritance reads classic PostgreSQL inheritance in both +// directions. Declarative partitioning also uses pg_inherits, so only edges +// whose child is not marked relispartition belong to classic inheritance. +func introspectInheritance(ctx context.Context, tx pgx.Tx, oid uint32) ([]string, []string, error) { + rows, err := tx.Query(ctx, ` + SELECT direction, nspname || '.' || relname + FROM ( + SELECT 0 AS direction, pn.nspname, p.relname + FROM pg_inherits i + JOIN pg_class child ON child.oid = i.inhrelid + JOIN pg_class p ON p.oid = i.inhparent + JOIN pg_namespace pn ON pn.oid = p.relnamespace + WHERE i.inhrelid = $1 AND NOT child.relispartition + UNION ALL + SELECT 1 AS direction, cn.nspname, child.relname + FROM pg_inherits i + JOIN pg_class child ON child.oid = i.inhrelid + JOIN pg_namespace cn ON cn.oid = child.relnamespace + WHERE i.inhparent = $1 AND NOT child.relispartition + ) inheritance + ORDER BY direction, nspname, relname`, oid) + if err != nil { + return nil, nil, fmt.Errorf("query inheritance: %w", err) + } + defer rows.Close() + var parents, children []string + for rows.Next() { + var direction int + var relation string + if err := rows.Scan(&direction, &relation); err != nil { + return nil, nil, fmt.Errorf("scan inheritance: %w", err) + } + switch direction { + case 0: + parents = append(parents, relation) + case 1: + children = append(children, relation) + default: + return nil, nil, fmt.Errorf("unexpected inheritance direction %d", direction) + } + } + if err := rows.Err(); err != nil { + return nil, nil, fmt.Errorf("read inheritance: %w", err) + } + return parents, children, nil +} + // introspectColumns reads the canonical column list: server-formatted types // and server-decompiled default/generation expressions, in attribute order. // Two dependency facts ride along for each sequence-backed default: that diff --git a/pkg/schemadiff/render.go b/pkg/schemadiff/render.go index ffbbbda..302c05c 100644 --- a/pkg/schemadiff/render.go +++ b/pkg/schemadiff/render.go @@ -26,6 +26,11 @@ var ErrUnrenderableDefault = errors.New("sequence-backed default cannot be rende // instead of emitting a wrong baseline. var ErrUnrenderablePartition = errors.New("partitioned tables cannot be rendered as a desired schema") +// ErrUnrenderableInheritance is returned for either side of a classic +// PostgreSQL inheritance relationship. The desired model cannot express +// inheritance edges, so rendering would flatten a child or omit its children. +var ErrUnrenderableInheritance = errors.New("table inheritance cannot be rendered as a desired schema") + // ErrUnrenderableForeignKey is returned when other tables reference this // one with foreign keys. A desired file cannot declare foreign keys, so // the single-table model carries no incoming foreign-key topology — a @@ -62,6 +67,12 @@ func Render(m Model) (string, error) { if m.IsPartition { return "", fmt.Errorf("render table %q: partition of a partitioned parent: %w", m.Table, ErrUnrenderablePartition) } + if len(m.InheritsParents) != 0 { + return "", fmt.Errorf("render table %q: inherits from %s: %w", m.Table, strings.Join(m.InheritsParents, ", "), ErrUnrenderableInheritance) + } + if len(m.InheritanceChildren) != 0 { + return "", fmt.Errorf("render table %q: has inheritance children %s: %w", m.Table, strings.Join(m.InheritanceChildren, ", "), ErrUnrenderableInheritance) + } if len(m.ReferencedBy) != 0 { return "", fmt.Errorf("render table %q: referenced by foreign keys (%s): %w", m.Table, strings.Join(m.ReferencedBy, ", "), ErrUnrenderableForeignKey) } diff --git a/pkg/schemadiff/render_integration_test.go b/pkg/schemadiff/render_integration_test.go index a92de29..44330b1 100644 --- a/pkg/schemadiff/render_integration_test.go +++ b/pkg/schemadiff/render_integration_test.go @@ -157,6 +157,32 @@ func TestRenderRefusesLivePartitionedTables(t *testing.T) { require.ErrorIs(t, err, schemadiff.ErrUnsupportedChange) } +func TestRenderRefusesLiveClassicInheritanceOnBothSides(t *testing.T) { + pool, err := dbconn.NewPool(t.Context(), dbconn.Config{URL: testutil.StartPostgres(t)}) + require.NoError(t, err) + defer pool.Close() + schema := testutil.NewSchema(t, pool) + + _, err = pool.Exec(t.Context(), fmt.Sprintf("CREATE TABLE %s.parent (id bigint NOT NULL)", schema)) + require.NoError(t, err) + _, err = pool.Exec(t.Context(), fmt.Sprintf("CREATE TABLE %s.child (note text) INHERITS (%s.parent)", schema, schema)) + require.NoError(t, err) + + parent, err := schemadiff.Introspect(t.Context(), pool, schema, "parent") + require.NoError(t, err) + assert.Equal(t, []string{schema + ".child"}, parent.InheritanceChildren) + assert.Empty(t, parent.InheritsParents) + _, err = schemadiff.Render(parent) + require.ErrorIs(t, err, schemadiff.ErrUnrenderableInheritance) + + child, err := schemadiff.Introspect(t.Context(), pool, schema, "child") + require.NoError(t, err) + assert.Equal(t, []string{schema + ".parent"}, child.InheritsParents) + assert.Empty(t, child.InheritanceChildren) + _, err = schemadiff.Render(child) + require.ErrorIs(t, err, schemadiff.ErrUnrenderableInheritance) +} + // A live table with a foreign key cannot be rendered: the desired-file // grammar refuses foreign keys, and the renderer surfaces that gate's typed // error rather than emitting a file the front door would reject. diff --git a/pkg/schemadiff/render_test.go b/pkg/schemadiff/render_test.go index 9c27860..18985b3 100644 --- a/pkg/schemadiff/render_test.go +++ b/pkg/schemadiff/render_test.go @@ -119,6 +119,18 @@ func TestRenderRefusesPartitionedTables(t *testing.T) { require.ErrorIs(t, err, ErrUnrenderablePartition) } +func TestRenderRefusesClassicInheritance(t *testing.T) { + child := base() + child.InheritsParents = []string{"public.parent"} + _, err := Render(child) + require.ErrorIs(t, err, ErrUnrenderableInheritance) + + parent := base() + parent.InheritanceChildren = []string{"public.child"} + _, err = Render(parent) + require.ErrorIs(t, err, ErrUnrenderableInheritance) +} + // A zero-column table is legal PostgreSQL and renders as an empty body, // not an empty line between the parentheses. func TestRenderZeroColumnTable(t *testing.T) { diff --git a/pkg/schemadiff/schemadiff.go b/pkg/schemadiff/schemadiff.go index 8a550a0..73c3435 100644 --- a/pkg/schemadiff/schemadiff.go +++ b/pkg/schemadiff/schemadiff.go @@ -98,6 +98,14 @@ type Model struct { // IsPartition reports that the table is itself a partition of a // partitioned parent (pg_class.relispartition). IsPartition bool + // InheritsParents lists the classic-inheritance parents of this table. + // Declarative partitions use the same pg_inherits catalog but are + // excluded using pg_class.relispartition. + InheritsParents []string + // InheritanceChildren lists the classic-inheritance children of this + // table. The model carries both directions so rendering either side can + // fail closed rather than flattening inherited columns or losing edges. + InheritanceChildren []string // Unlogged reports that the table is unlogged // (pg_class.relpersistence 'u'). The declarative model does not manage // persistence yet — converging it (SET LOGGED / SET UNLOGGED) is a