Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,21 @@ The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

### Changed — observable outcomes for automation callers

- **`diff` now exits 2 when the derived plan contains a statement execution
would refuse**, in all three output modes (default report, `--sql`,
`--json`) — the same CI-gate contract as `migrate --dry-run`. Previously
`diff` always exited 0, so CI gating on the exit code saw refusals as
green. The report is still complete and valid on stdout; a caller must
read the plan before branching on the exit code.
- **`diff` prints the diagnostic report by default; the executable SQL
script moved behind `--sql`** (`--sql --json` is rejected at parse time).
A caller consuming default `diff` stdout as SQL must ask for the script
explicitly.
- **`lint` and `suggest` text output renders in the dry-run's diagnostic
grammar.** Display only — the JSON reports and exit codes are unchanged —
but the one-line `file:line:column: severity: …` shape is gone, so
errorformat-style CI annotators should consume `--json` and supply the
file name they passed in.
- **The plan report is format version 2**: rewrite-required statements now
carry a `guidance` field naming the typed manual path, drawn from the
suggest report's Guidance vocabulary. The fingerprint definition is
Expand Down
63 changes: 53 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,64 @@ and suggest the safer form:

```console
$ pg-sprite lint changes.sql
changes.sql:1:1: warning: blocking-idiom — CREATE INDEX users_email_idx
safer form (not equivalent — see https://github.com/block/pg-sprite/blob/main/docs/postgres-online-ddl-reference.md): CREATE INDEX CONCURRENTLY users_email_idx ON users USING btree (email);
run each statement in its own transaction, never one block; after a failed CONCURRENTLY build, check pg_index.indisvalid and rebuild
changes.sql:1:1:
CREATE INDEX users_email_idx ON users (email);

warning[blocking-idiom]:
CREATE INDEX users_email_idx — holds a blocking lock on the table for
the whole operation — writes (and for some forms reads) wait until it
finishes

help:
a safer online form exists — not a semantic equivalent, and running it
by hand forgoes the engine's execution-time guards:
1. CREATE INDEX CONCURRENTLY users_email_idx ON users USING btree (email);

note:
run each statement in its own transaction, never one block; after a
failed CONCURRENTLY build, check pg_index.indisvalid and rebuild

docs:
https://github.com/block/pg-sprite/blob/main/docs/postgres-online-ddl-reference.md#safer-idiom

lint:
changes.sql — 1 finding, 0 errors, 1 warning
```

**Diff: declarative desired state in, executable plan out.** Point at a
reviewed `CREATE TABLE` file and get the classified statements that converge
the live table onto it:
**Diff: declarative desired state in, classified plan out.** Point at a
reviewed `CREATE TABLE` file and get the statements that converge the live
table onto it, reported in the same diagnostic grammar as the dry run.
`--sql` prints the plan as an executable SQL script instead, and a plan
containing a statement execution would refuse exits 2 — the same CI gate
as the dry run:

```console
$ pg-sprite diff --desired users.sql
-- plan derived by pg-sprite diff; execute statements via pg-sprite migrate,
-- which refuses blocking forms — running this script directly bypasses that gate
-- native (metadata-only)
ALTER TABLE public.users ADD COLUMN nickname text;
statement 1:
ALTER TABLE public.users ADD COLUMN nickname text;

note[metadata-only]:
ADD COLUMN nickname — a brief catalog-only change; takes a short
exclusive lock but does not scan or rewrite the table

note:
runs as written

docs:
https://github.com/block/pg-sprite/blob/main/docs/postgres-online-ddl-reference.md#metadata-only

plan:
public.users (PostgreSQL 16.14) — 1 statement, 1 step to run, 0 refused

diff:
nothing was executed

sql:
re-run with --sql to print the plan as an executable SQL script

apply:
run each statement via pg-sprite migrate --alter '…', which refuses
blocking forms and substitutes safer online sequences
```

More shapes — every disposition as JSON, destructive warnings, and exit
Expand Down
6 changes: 4 additions & 2 deletions docs/cli-output-examples.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ that must stop drops checks `.statements[].destructive` in the JSON report.
Exit 2 covers every refusal cause; the exit code answers *should this
proceed*, the report's typed `reason` and disposition answer *why not*. The contract is
`migrate`'s: `lint` exits 1 when a script has error-severity findings
(warnings alone exit 0), and `diff` prints the plan and exits 0 regardless of
disposition. Statement kinds `migrate` does not support (`DROP INDEX`,
(warnings alone exit 0), and `diff` prints the plan and exits 2 when it
contains a statement execution would refuse — a missing table is diff's
greenfield case (the plan creates the table), not a refusal, unlike the
dry run. Statement kinds `migrate` does not support (`DROP INDEX`,
`REINDEX`, `CREATE TABLE`, and anything that is not `ALTER TABLE` or
`CREATE INDEX`) emit the same refusal verdict on `--dry-run` as on apply —
a verdict, not a plan report — and exit 2. The JSON report schema is
Expand Down
28 changes: 24 additions & 4 deletions docs/lint-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ codes make the conservatism visible instead of burying it:
| `app-breaking-rename` | warning | `ALTER TABLE t RENAME COLUMN email TO email_address` | PostgreSQL runs a column or table rename as a metadata-only catalog flip, but a rename cannot land atomically across running application instances — code still referencing the old name breaks the instant it commits. For a column, expand/contract instead: add the new column, dual-write and backfill, switch reads, then drop the old column as its own reviewed change. For a table, coordinate the rename with the application deploy that adopts the new name. Index renames are not flagged — SQL never references an index by name. |
| `destructive` | warning | `ALTER TABLE t DROP COLUMN legacy` | The operation discards live structure (a column, constraint, or index drop) and cannot be undone by re-running the schema. |

A lint `code` is what the finding *is*; the finding's `reason` field is the classifier's
typed cause for it, drawn from the plan report's Reason vocabulary
([postgres-online-ddl-reference.md#dry-run-diagnostic-codes](postgres-online-ddl-reference.md#dry-run-diagnostic-codes)) —
the text output prints the code as the diagnostic label and links the reason's reference
anchor. The mapping:

| `code` | `reason` carried |
|---|---|
| `blocking-idiom` | `safer-idiom` |
| `table-rewrite`, `possible-table-rewrite` | the specific rewrite cause: `volatile-default`, `generated-stored`, `type-rewrite`, or `relocation` |
| `app-breaking-rename` | `app-breaking-rename` |
| `unsupported-operation` | `unsupported-operation` |
| `destructive` | none — destructiveness is a property of the operation, not a routing cause |

## Severities (`severity`)

| Value | Meaning | Exit behavior |
Expand All @@ -93,7 +107,13 @@ inline suppression) is a planned extension and will be introduced as a contract

## Text output

Without `--json`, findings render one per line in the conventional linter shape —
`name:line:column: severity: code — operation` — where `name` is the linted file path or
`<stdin>`. The text form is for humans and editors; automation consumes the JSON report.
A clean script prints nothing and exits zero.
Without `--json`, findings render in the same compiler-diagnostic grammar as the dry-run
and diff reports: each flagged statement leads its group under the conventional
`name:line:column:` label (where `name` is the linted file path or `<stdin>`) so a reader
can jump to the source, each finding is a `severity[code]:` entry beneath it with the
impact prose, any safer form follows as a `help:` entry with its execution caveat, a
`docs:` entry links the reference anchors, and the report closes with a `lint:` summary.
The text form is for humans; it is not a machine surface — the one-line
`file:line:column: severity: …` shape errorformat-style annotators parse is not emitted,
so automation (including CI annotation) consumes the JSON report and supplies the file
name it passed in. A clean script prints nothing and exits zero.
12 changes: 8 additions & 4 deletions docs/suggest-report.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,11 @@ A guidance suggestion (`ALTER TABLE t ALTER COLUMN c SET NOT NULL, ADD COLUMN d

## Text output

Without `--json`, each suggestion renders as the statement header
(`statement N: operation — reason`) followed by either the safer sequence with its caveat
list or the guidance code with its manual path. The text form is for humans; automation
consumes the JSON report. A clean script prints nothing and always exits zero.
Without `--json`, suggestions render in the same compiler-diagnostic grammar as the
dry-run and diff reports: each risky statement leads its group under the conventional
`name:line:column:` label (where `name` is the advised file path or `<stdin>`), the
classification is a `warning[reason]:` entry beneath it, the safer sequence (or the
guidance code naming the manual path) follows as a `help:` entry with its caveats, a
`docs:` entry links the reference anchors, and the report closes with a `suggest:`
summary. The text form is for humans; automation consumes the JSON report. A clean
script prints nothing and always exits zero.
11 changes: 11 additions & 0 deletions internal/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ type DiffCmd struct {
Desired string `help:"Path to the desired-state CREATE TABLE .sql file." name:"desired" type:"existingfile" required:""`
Schema string `help:"Schema containing the live table." default:"public"`
JSON bool `help:"Emit the plan as JSON."`
SQL bool `help:"Print the plan as an executable SQL script instead of the diagnostic report."`
}

// Validate rejects flag combinations with no coherent meaning: --json and
// --sql each replace the default report with a different whole-output
// format, so combining them names no single output.
func (c *DiffCmd) Validate() error {
if c.JSON && c.SQL {
return errors.New("--sql cannot be combined with --json: each replaces the report with a different format")
}
return nil
}

// Run implements the diff subcommand.
Expand Down
30 changes: 23 additions & 7 deletions internal/cli/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/block/pg-sprite/pkg/planner"
"github.com/block/pg-sprite/pkg/router"
"github.com/block/pg-sprite/pkg/statement"
"github.com/block/pg-sprite/pkg/verdict"
)

// run is the diff flow: parse and admit the desired file, derive the routed
Expand Down Expand Up @@ -47,10 +48,24 @@ func (c *DiffCmd) run(ctx context.Context, out io.Writer) error {
"table_exists", report.TableExists != nil && *report.TableExists,
"disposition", string(report.Disposition))

if c.JSON {
return writeJSON(out, report)
switch {
case c.JSON:
err = writeJSON(out, report)
case c.SQL:
err = writePlanText(out, report)
default:
err = writeDiffText(out, report)
}
return writePlanText(out, report)
if err != nil {
return err
}
// A plan execution would refuse exits with the refusal code — the same
// contract the dry run uses — so CI can gate on the diff without
// parsing the report.
if diffRefused(report) {
return verdict.ErrRefused
}
return nil
}

// writeJSON emits the plan report as JSON.
Expand All @@ -66,10 +81,11 @@ func writeJSON(out io.Writer, report plan.Report) error {
return nil
}

// writePlanText emits the plan as an executable SQL script: one statement
// per line, each annotated with its route, destructive statements flagged,
// and SQL comments for the no-change and missing-table cases so the output
// stays valid SQL. Safer sequences appear as comment lines — never
// writePlanText emits the plan as an executable SQL script (the --sql
// rendering): one statement per line, each annotated with its route,
// destructive statements flagged, and SQL comments for the no-change and
// missing-table cases so the output stays valid SQL. Safer sequences
// appear as comment lines — never
// substituted into the script body, which stays the literal convergence
// plan (a CONCURRENTLY rewrite could not run inside a transaction block).
// The header points at migrate as the executing front door: running this
Expand Down
7 changes: 5 additions & 2 deletions internal/cli/diff_integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"github.com/block/pg-sprite/pkg/planner"
"github.com/block/pg-sprite/pkg/router"
"github.com/block/pg-sprite/pkg/schemadiff"
"github.com/block/pg-sprite/pkg/verdict"
)

// newDiffCmd builds a DiffCmd with the flag defaults kong would apply,
Expand Down Expand Up @@ -128,7 +129,8 @@ func TestDiffRoutesRewriteToCopyAndSwap(t *testing.T) {
cmd := newDiffCmd(t, url, schema, "CREATE TABLE events (id bigint PRIMARY KEY)")
cmd.JSON = true
var out strings.Builder
require.NoError(t, cmd.run(t.Context(), &out))
require.ErrorIs(t, cmd.run(t.Context(), &out), verdict.ErrRefused,
"a plan execution would refuse exits with the refusal code")

var report plan.Report
require.NoError(t, json.Unmarshal([]byte(out.String()), &report))
Expand Down Expand Up @@ -232,10 +234,11 @@ func TestDiffTextPlanIsExecutableSQL(t *testing.T) {
require.NoError(t, err)

cmd := newDiffCmd(t, url, schema, "CREATE TABLE events (id bigint PRIMARY KEY, name text NOT NULL)")
cmd.SQL = true
var out strings.Builder
require.NoError(t, cmd.run(t.Context(), &out))

// The text plan is an executable script: running it converges the table.
// The --sql plan is an executable script: running it converges the table.
_, err = pool.Exec(t.Context(), out.String())
require.NoError(t, err, "text plan must be executable SQL: %s", out.String())

Expand Down
23 changes: 23 additions & 0 deletions internal/cli/diff_test.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,38 @@
package cli

import (
"os"
"path/filepath"
"strings"
"testing"

"github.com/alecthomas/kong"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/block/pg-sprite/pkg/statement"
)

// --json and --sql each replace the default diagnostic report with a
// different whole-output format; combining them names no single output, so
// the parse is rejected rather than one flag silently winning.
func TestDiffRejectsSQLWithJSON(t *testing.T) {
desired := filepath.Join(t.TempDir(), "schema.sql")
require.NoError(t, os.WriteFile(desired, []byte("CREATE TABLE t (id bigint PRIMARY KEY)"), 0o600))
c := New()
k, err := kong.New(c, kong.Vars{"version": "test"})
require.NoError(t, err)
_, err = k.Parse([]string{
"diff",
"--url", "postgres://user@localhost:5432/app",
"--desired", desired,
"--sql",
"--json",
})
require.Error(t, err)
assert.Contains(t, err.Error(), "--sql cannot be combined with --json")
}

func TestFmtCanonicalizesFromStdin(t *testing.T) {
cmd := &FmtCmd{}
in := strings.NewReader(`create table events (
Expand Down
62 changes: 62 additions & 0 deletions internal/cli/diff_text.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
package cli

import (
"fmt"
"io"

"github.com/block/pg-sprite/pkg/plan"
"github.com/block/pg-sprite/pkg/router"
)

// writeDiffText renders the derived convergence plan in the same
// compiler-diagnostic grammar as the dry-run report: one labeled entry per
// finding, the typed reason as the rule code, doc anchors, and a closing
// plan summary. Display only — the JSON report is the machine contract, and
// --sql prints the plan as an executable script (writePlanText). The
// framing differs from the dry-run report where the semantics differ: diff
// never executes, so execution is routed through the migrate front door,
// and a missing table is the greenfield case — the plan creates the table
// from the full desired schema — not an error.
func writeDiffText(out io.Writer, report plan.Report) error {
w := &stickyWriter{out: out}
if tableMissing(report) {
w.diag("note", "", fmt.Sprintf("the table %s.%s does not exist — the plan creates it from the full desired schema", report.Schema, report.Table))
}
if len(report.Statements) == 0 {
w.entry("plan:")
w.printf(" %s — no changes; the live table matches the desired schema\n", targetText(report))
return w.err
}
steps, refused := 0, 0
for i, ps := range report.Statements {
s, r := writeStatementDiagnostics(w, i+1, ps, "pg-sprite migrate")
steps += s
refused += r
}
w.entry("plan:")
w.printf(" %s — %s, %s to run, %d refused\n", targetText(report),
countNoun(len(report.Statements), "statement"), countNoun(steps, "step"), refused)
w.entry("diff:")
w.printf(" nothing was executed\n")
w.entry("sql:")
w.printf(" re-run with --sql to print the plan as an executable SQL script\n")
// No apply pointer for a greenfield plan: migrate changes an existing
// table and refuses CREATE TABLE, so pointing the reader at it would
// send them in a circle — the leading note and the --sql pointer are
// the honest route.
if refused == 0 && steps > 0 && !tableMissing(report) {
w.entry("apply:")
w.printf(" run each statement via pg-sprite migrate --alter '…', which refuses\n")
w.printf(" blocking forms and substitutes safer online sequences\n")
}
return w.err
}

// diffRefused reports whether the derived plan contains any statement
// execution would refuse. The diff exits with the refusal code in that
// case — the same contract the dry run uses — so CI can gate on the diff
// without parsing the report. A missing table does not refuse: for diff it
// is the greenfield case, and the plan is the full desired schema.
func diffRefused(report plan.Report) bool {
return report.Disposition != router.DispositionExecute
}
Loading
Loading