From d6820cb540ee3f924713646e99e553acfd3baa50 Mon Sep 17 00:00:00 2001 From: Kiran Muddukrishna Date: Thu, 6 Aug 2026 16:28:46 +1000 Subject: [PATCH] Add pkg/suggest: advisory safer-form rewrites with typed caveats Completes the P2.5 advisory surface (PLAT-38440): original -> recommended with typed reason and caveat metadata, offline and never executing. Refusals and rewrites stay lint findings; suggest reports only constructed rewrites, and an unmapped rewrite fails closed rather than shipping caveat-less advice. --- README.md | 4 +- SAFETY.md | 4 +- docs/low-level-design.md | 4 +- internal/cli/cli.go | 14 ++- internal/cli/suggest.go | 73 ++++++++++++++++ internal/cli/suggest_test.go | 78 +++++++++++++++++ pkg/suggest/suggest.go | 161 +++++++++++++++++++++++++++++++++++ pkg/suggest/suggest_test.go | 154 +++++++++++++++++++++++++++++++++ 8 files changed, 486 insertions(+), 6 deletions(-) create mode 100644 internal/cli/suggest.go create mode 100644 internal/cli/suggest_test.go create mode 100644 pkg/suggest/suggest.go create mode 100644 pkg/suggest/suggest_test.go diff --git a/README.md b/README.md index 34d3ddf..0258b92 100644 --- a/README.md +++ b/README.md @@ -26,8 +26,8 @@ verdict when it can't prove one (see [docs/postgres-online-ddl-reference.md](docs/postgres-online-ddl-reference.md)). **Status: Phases 1 and 2.1–2.5.** The parse boundary, declarative diff, -classifier, router seam, versioned dry-run plan report, and offline linter -are implemented. `pg-sprite migrate --alter '…'` runs a bounded optimistic +classifier, router seam, versioned dry-run plan report, offline linter, and +advisory `suggest` command are implemented. `pg-sprite migrate --alter '…'` runs a bounded optimistic native attempt; routed execution beyond that attempt lands in Phase 3. Changes without an available backend get a structured refusal (exit code 2). The design docs and the phased diff --git a/SAFETY.md b/SAFETY.md index c5bed47..1eec290 100644 --- a/SAFETY.md +++ b/SAFETY.md @@ -27,9 +27,9 @@ The invariant registry (invariant IDs referenced below) lives in | `pkg/checkpoint` — durable resume state | ✅ core | planned (Phase 8) | ST-1, ST-2 | | slot lifecycle (in `pkg/decode`) — create, reap, lag ceiling | ✅ core | planned (Phase 8) | ST-3 | | `pkg/migration` — orchestrator, **cutover swap + fidelity gate** | ✅ core | planned (Phase 7) | LK-2, LK-4, ST-5 | -| `pkg/statement`, `pkg/planner`, `pkg/schemadiff`, `pkg/router`, `pkg/plan`, `pkg/lint` — classify/diff/route/report | ❌ periphery¹ | `pkg/statement` (parse boundary), `pkg/schemadiff` (introspect/diff via scratch execute-and-introspect), `pkg/planner` (classifier), `pkg/router` (backend assignment + availability policy), `pkg/plan` (versioned dry-run plan report), and `pkg/lint` (offline typed findings) exist (Phases 2.1–2.5) | (CO-7 holds at the parse boundary) | +| `pkg/statement`, `pkg/planner`, `pkg/schemadiff`, `pkg/router`, `pkg/plan`, `pkg/lint`, `pkg/suggest` — classify/diff/route/report | ❌ periphery¹ | `pkg/statement` (parse boundary), `pkg/schemadiff` (introspect/diff via scratch execute-and-introspect), `pkg/planner` (classifier), `pkg/router` (backend assignment + availability policy), `pkg/plan` (versioned dry-run plan report), `pkg/lint` (offline typed findings), and `pkg/suggest` (advisory rewrites with typed caveats) exist (Phases 2.1–2.5) | (CO-7 holds at the parse boundary) | | `pkg/verdict` — structured outcome contract, rendering, exit codes | ❌ periphery | exists (Phase 1) | — | -| `internal/cli` — CLI, flags, help, prompts | ❌ periphery | `migrate`, `status`, `diff`, `fmt`, and `lint` exist | — | +| `internal/cli` — CLI, flags, help, prompts | ❌ periphery | `migrate`, `status`, `diff`, `fmt`, `lint`, and `suggest` exist | — | | status / progress / advisory rendering, metrics | ❌ periphery | planned | — | | orchestrator adapter | ❌ periphery | planned (Phase 11) | OC-* hold *at* the boundary | | `internal/testutil` | ❌ test-only | exists | — | diff --git a/docs/low-level-design.md b/docs/low-level-design.md index 3574dec..473dfe7 100644 --- a/docs/low-level-design.md +++ b/docs/low-level-design.md @@ -288,7 +288,9 @@ request, not a permission. The [advisory behaviour](high-level-design.md#advisory-mode-suggest-the-safe-rewrite-dont-silently-run-the-risky-one) is a property of the **planner's classifier output**, not a separate code path. Every current `planner.Decision` carries the operation, route, typed reason, and safer SQL where applicable. -The CLI renders that output in `diff` and `migrate --dry-run`. +The CLI renders that output in `diff` and `migrate --dry-run`, and the offline `suggest` +command (`pkg/suggest`) emits it as a standalone advisory report — original → recommended +with typed reason and caveat metadata, never executing anything. ### What the classifier emits per operation diff --git a/internal/cli/cli.go b/internal/cli/cli.go index e2ff9b7..4d1830d 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 (the offline checker). +// door), and lint and suggest (the offline checker and advisor). package cli import ( @@ -23,6 +23,7 @@ type CLI struct { 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."` + Suggest SuggestCmd `cmd:"" help:"Recommend safer native forms for risky DDL."` Status StatusCmd `cmd:"" help:"Report the status of a running migration."` } @@ -121,6 +122,17 @@ type LintCmd struct { // Run implements the lint subcommand. func (c *LintCmd) Run() error { return c.runLint(os.Stdin, os.Stdout) } +// SuggestCmd maps risky-as-written DDL to the safer native form the engine +// would run instead, with typed caveats. It is offline and advisory — no +// database flags, nothing executes, and it always exits zero. +type SuggestCmd struct { + Path string `arg:"" optional:"" help:"DDL file to advise on; stdin when omitted." type:"existingfile"` + JSON bool `help:"Emit the suggestions report as JSON."` +} + +// Run implements the suggest subcommand. +func (c *SuggestCmd) Run() error { return c.runSuggest(os.Stdin, os.Stdout) } + // StatusCmd reports migration progress. type StatusCmd struct { DBFlags `embed:""` diff --git a/internal/cli/suggest.go b/internal/cli/suggest.go new file mode 100644 index 0000000..6a44822 --- /dev/null +++ b/internal/cli/suggest.go @@ -0,0 +1,73 @@ +package cli + +import ( + "encoding/json" + "fmt" + "io" + "os" + "strings" + + "github.com/block/pg-sprite/pkg/suggest" +) + +// runSuggest maps a DDL script to its advisory rewrites: parse every +// statement through the PostgreSQL grammar, classify it with zero live +// facts, and report the safer native form for anything risky as written. +// Offline — no database, nothing executes. A script with no rewrites +// prints nothing and the command always exits zero: suggest advises, lint +// gates. +func (c *SuggestCmd) runSuggest(in io.Reader, out io.Writer) error { + var src []byte + var err error + if c.Path == "" { + if src, err = io.ReadAll(in); err != nil { + return fmt.Errorf("read DDL from stdin: %w", err) + } + } else if src, err = os.ReadFile(c.Path); err != nil { + return fmt.Errorf("read DDL file: %w", err) + } + report, err := suggest.Advise(string(src)) + if err != nil { + return err + } + if c.JSON { + enc := json.NewEncoder(out) + enc.SetIndent("", " ") + if err := enc.Encode(report); err != nil { + return fmt.Errorf("write suggest report: %w", err) + } + return nil + } + return writeSuggestText(out, report) +} + +// writeSuggestText renders each suggestion as the original, the safer +// sequence, and its caveats. A report with no suggestions prints nothing. +func writeSuggestText(out io.Writer, report suggest.Report) error { + for _, s := range report.Suggestions { + if _, err := fmt.Fprintf(out, "statement %d: %s — %s\n", s.Statement, s.Operation, s.Reason); err != nil { + return fmt.Errorf("write suggest report: %w", err) + } + if _, err := fmt.Fprintf(out, " safer form (not equivalent — see docs/postgres-online-ddl-reference.md):\n"); err != nil { + return fmt.Errorf("write suggest report: %w", err) + } + for _, sql := range s.Recommended { + if _, err := fmt.Fprintf(out, " %s;\n", sql); err != nil { + return fmt.Errorf("write suggest report: %w", err) + } + } + if _, err := fmt.Fprintf(out, " caveats: %s\n", joinCaveats(s.Caveats)); err != nil { + return fmt.Errorf("write suggest report: %w", err) + } + } + return nil +} + +// joinCaveats renders the typed caveats as a comma-separated list. +func joinCaveats(caveats []suggest.Caveat) string { + names := make([]string, len(caveats)) + for i, c := range caveats { + names[i] = string(c) + } + return strings.Join(names, ", ") +} diff --git a/internal/cli/suggest_test.go b/internal/cli/suggest_test.go new file mode 100644 index 0000000..20cdd00 --- /dev/null +++ b/internal/cli/suggest_test.go @@ -0,0 +1,78 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/suggest" +) + +func TestSuggestCleanScriptPrintsNothing(t *testing.T) { + var out strings.Builder + cmd := SuggestCmd{} + err := cmd.runSuggest(strings.NewReader("CREATE TABLE t (id int)"), &out) + require.NoError(t, err) + assert.Empty(t, out.String()) +} + +// Suggest is advisory: a script full of findings still exits zero — lint +// owns the gate. +func TestSuggestAlwaysExitsZeroOnValidScripts(t *testing.T) { + var out strings.Builder + cmd := SuggestCmd{JSON: true} + err := cmd.runSuggest(strings.NewReader(` + CREATE INDEX t_c_idx ON t (c); + ALTER TABLE t ADD CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =); + `), &out) + require.NoError(t, err) + + var report suggest.Report + require.NoError(t, json.Unmarshal([]byte(out.String()), &report)) + assert.Equal(t, suggest.FormatVersion, report.FormatVersion) + require.Len(t, report.Suggestions, 1, "the refused statement yields no advice") + assert.Equal(t, 1, report.Suggestions[0].Statement) +} + +func TestSuggestReadsFromFile(t *testing.T) { + path := filepath.Join(t.TempDir(), "change.sql") + require.NoError(t, os.WriteFile(path, + []byte("ALTER TABLE t ALTER COLUMN c SET NOT NULL"), 0o600)) + + var out strings.Builder + cmd := SuggestCmd{Path: path, JSON: true} + require.NoError(t, cmd.runSuggest(strings.NewReader(""), &out)) + + var report suggest.Report + require.NoError(t, json.Unmarshal([]byte(out.String()), &report)) + require.Len(t, report.Suggestions, 1) + assert.Equal(t, + []suggest.Caveat{suggest.CaveatSeparateTransactions, suggest.CaveatValidationScan}, + report.Suggestions[0].Caveats) +} + +func TestSuggestParseFailureIsError(t *testing.T) { + var out strings.Builder + cmd := SuggestCmd{} + err := cmd.runSuggest(strings.NewReader("CREATE TABEL t (id int)"), &out) + require.Error(t, err) +} + +// The text rendering is this renderer's own unit test: each suggestion +// shows the safer sequence and its caveats. +func TestSuggestTextRendering(t *testing.T) { + var out strings.Builder + cmd := SuggestCmd{} + err := cmd.runSuggest(strings.NewReader("CREATE INDEX t_c_idx ON t (c)"), &out) + require.NoError(t, err) + text := out.String() + assert.Contains(t, text, "statement 1:") + assert.Contains(t, text, "CONCURRENTLY") + assert.Contains(t, text, string(suggest.CaveatNonTransactional)) + assert.Contains(t, text, string(suggest.CaveatInvalidIndexOnFailure)) +} diff --git a/pkg/suggest/suggest.go b/pkg/suggest/suggest.go new file mode 100644 index 0000000..749902d --- /dev/null +++ b/pkg/suggest/suggest.go @@ -0,0 +1,161 @@ +// Package suggest is the advisory surface: it maps DDL that is risky as +// written to the safer native form the engine would run instead, offline +// and without executing anything. It reports only constructed rewrites — +// refusals, table rewrites, and destructive drops are pkg/lint's job — and +// every recommendation carries typed caveats, because a safer form is not +// a semantic equivalent: it reaches the same end state with different +// locking, transactionality, and failure modes. +package suggest + +import ( + "fmt" + + "github.com/block/pg-sprite/pkg/planner" + "github.com/block/pg-sprite/pkg/statement" +) + +// FormatVersion identifies the report contract. A consumer must reject a +// report whose version it does not understand instead of guessing at the +// field semantics. +const FormatVersion = 1 + +// Caveat is a typed condition attached to a recommendation; automation +// branches on it, never on prose. +type Caveat string + +// The caveats a recommendation can carry. +const ( + // CaveatNonTransactional: the recommended sequence contains a + // CONCURRENTLY statement, which cannot run inside a transaction + // block. + CaveatNonTransactional Caveat = "non-transactional" + // CaveatSeparateTransactions: the steps must commit separately — the + // weaker locks the sequence exists for are held to commit, so one + // enclosing transaction reproduces the blocking the rewrite avoids. + CaveatSeparateTransactions Caveat = "separate-transactions" + // CaveatInvalidIndexOnFailure: a failed or cancelled concurrent build + // leaves an INVALID index that must be detected (pg_index.indisvalid) + // and dropped or rebuilt; the engine's executor owns that check when + // it runs the sequence. + CaveatInvalidIndexOnFailure Caveat = "invalid-index-on-failure" + // CaveatDetachFinalizeOnFailure: an interrupted concurrent detach + // leaves the partition half-detached; it must be finished with + // DETACH PARTITION FINALIZE. + CaveatDetachFinalizeOnFailure Caveat = "detach-finalize-on-failure" + // CaveatValidationScan: the VALIDATE step still scans every row — the + // rewrite trades the lock strength, not the scan. + CaveatValidationScan Caveat = "validation-scan" +) + +// Suggestion is one advisory rewrite: the statement as written, the safer +// native sequence, and the typed metadata explaining the trade. +type Suggestion struct { + // Statement is the 1-based index of the statement in the script. + Statement int `json:"statement"` + // Original is the canonical text of the statement as submitted. + Original string `json:"original"` + // Operation is the operator-facing label of the risky operation + // (display only). + Operation string `json:"operation"` + // Reason is the classifier's typed cause for preferring the rewrite. + Reason planner.Reason `json:"reason"` + // Recommended is the ordered safer SQL to run instead. + Recommended []string `json:"recommended"` + // Caveats are the typed conditions under which the recommendation + // differs from the original; never empty — a rewrite with no trade + // would be the same statement. + Caveats []Caveat `json:"caveats"` +} + +// Report is the advisory result for one script. +type Report struct { + // FormatVersion is the report contract version; always FormatVersion. + FormatVersion int `json:"format_version"` + // Suggestions are the rewrites in statement order; empty means every + // statement is already in its safest known form or is outside the + // advisory surface (refusals and rewrites are lint findings). + Suggestions []Suggestion `json:"suggestions"` +} + +// Advise maps a DDL script to its advisory rewrites: every statement is +// parsed with the PostgreSQL grammar and classified with zero live facts, +// and each risky-as-written operation with a constructible safer form +// yields a Suggestion. Nothing is executed and no database is touched. A +// parse failure is an error. +func Advise(sql string) (Report, error) { + stmts, err := statement.Split(sql) + if err != nil { + return Report{}, err + } + report := Report{FormatVersion: FormatVersion, Suggestions: []Suggestion{}} + for i, stmt := range stmts { + suggestions, err := adviseStatement(i+1, stmt) + if err != nil { + return Report{}, fmt.Errorf("statement %d: %w", i+1, err) + } + report.Suggestions = append(report.Suggestions, suggestions...) + } + return report, nil +} + +// adviseStatement produces the suggestions for one statement: one per +// safer-idiom decision whose rewrite the planner could construct. The +// operation list and decision list are index-aligned by the planner's +// contract (one decision per operation, in order); a mismatch is a +// contract violation and fails closed. +func adviseStatement(index int, sql string) ([]Suggestion, error) { + plan, err := planner.Classify(sql, planner.Facts{}) + if err != nil { + return nil, err + } + ops, err := statement.ParseOps(sql) + if err != nil { + return nil, err + } + if len(ops) != len(plan.Decisions) { + return nil, fmt.Errorf("planner produced %d decisions for %d operations", len(plan.Decisions), len(ops)) + } + var suggestions []Suggestion + for i, d := range plan.Decisions { + if d.Reason != planner.ReasonSaferIdiom || len(d.SaferSQL) == 0 { + continue + } + caveats, err := rewriteCaveats(ops[i]) + if err != nil { + return nil, err + } + suggestions = append(suggestions, Suggestion{ + Statement: index, + Original: sql, + Operation: d.Operation, + Reason: d.Reason, + Recommended: d.SaferSQL, + Caveats: caveats, + }) + } + return suggestions, nil +} + +// rewriteCaveats maps an operation to the typed caveats of its safer +// rewrite. An operation with a rewrite this table does not know is a +// contract violation — when the planner learns a new rewrite, its caveats +// must be recorded here before the advice ships — so it fails closed +// rather than emitting caveat-less advice. +func rewriteCaveats(op statement.Op) ([]Caveat, error) { + switch op.Kind { + case statement.OpCreateIndex, statement.OpDropIndex, statement.OpReindex: + return []Caveat{CaveatNonTransactional, CaveatInvalidIndexOnFailure}, nil + case statement.OpDetachPartition: + return []Caveat{CaveatNonTransactional, CaveatDetachFinalizeOnFailure}, nil + case statement.OpSetNotNull: + return []Caveat{CaveatSeparateTransactions, CaveatValidationScan}, nil + case statement.OpAddConstraint: + switch op.Constraint { + case statement.ConstraintPrimaryKey, statement.ConstraintUnique: + return []Caveat{CaveatNonTransactional, CaveatInvalidIndexOnFailure}, nil + case statement.ConstraintCheck, statement.ConstraintForeignKey: + return []Caveat{CaveatSeparateTransactions, CaveatValidationScan}, nil + } + } + return nil, fmt.Errorf("no caveat mapping for rewritten operation %q", op.Describe()) +} diff --git a/pkg/suggest/suggest_test.go b/pkg/suggest/suggest_test.go new file mode 100644 index 0000000..b2ed768 --- /dev/null +++ b/pkg/suggest/suggest_test.go @@ -0,0 +1,154 @@ +package suggest_test + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/block/pg-sprite/pkg/planner" + "github.com/block/pg-sprite/pkg/suggest" +) + +func TestAdviseCleanScriptHasNoSuggestions(t *testing.T) { + report, err := suggest.Advise(` + CREATE TABLE t (id bigint PRIMARY KEY); + ALTER TABLE t ADD COLUMN age int DEFAULT 0; + CREATE INDEX CONCURRENTLY t_age_idx ON t (age); + `) + require.NoError(t, err) + assert.Equal(t, suggest.FormatVersion, report.FormatVersion) + assert.Empty(t, report.Suggestions) +} + +func TestAdviseEmptyScriptIsClean(t *testing.T) { + report, err := suggest.Advise("") + require.NoError(t, err) + assert.Empty(t, report.Suggestions) +} + +func TestAdviseParseFailureIsError(t *testing.T) { + _, err := suggest.Advise("ALTER TABEL t ADD COLUMN c int") + require.Error(t, err) +} + +func TestAdviseCreateIndexGetsConcurrentRewrite(t *testing.T) { + report, err := suggest.Advise("CREATE INDEX t_c_idx ON t (c)") + require.NoError(t, err) + require.Len(t, report.Suggestions, 1) + s := report.Suggestions[0] + assert.Equal(t, 1, s.Statement) + assert.Equal(t, planner.ReasonSaferIdiom, s.Reason) + require.Len(t, s.Recommended, 1) + assert.NotEqual(t, s.Original, s.Recommended[0], "the recommendation is the concurrent rewrite") + assert.Equal(t, + []suggest.Caveat{suggest.CaveatNonTransactional, suggest.CaveatInvalidIndexOnFailure}, + s.Caveats) +} + +func TestAdviseAddCheckGetsNotValidValidateSequence(t *testing.T) { + report, err := suggest.Advise("ALTER TABLE t ADD CONSTRAINT t_age_pos CHECK (age > 0)") + require.NoError(t, err) + require.Len(t, report.Suggestions, 1) + s := report.Suggestions[0] + require.Len(t, s.Recommended, 2, "NOT VALID then VALIDATE") + assert.Equal(t, + []suggest.Caveat{suggest.CaveatSeparateTransactions, suggest.CaveatValidationScan}, + s.Caveats) +} + +func TestAdviseAddPrimaryKeyGetsUsingIndexSequence(t *testing.T) { + report, err := suggest.Advise("ALTER TABLE t ADD PRIMARY KEY (id)") + require.NoError(t, err) + require.Len(t, report.Suggestions, 1) + s := report.Suggestions[0] + require.Len(t, s.Recommended, 2, "concurrent unique index build then USING INDEX attach") + assert.Equal(t, + []suggest.Caveat{suggest.CaveatNonTransactional, suggest.CaveatInvalidIndexOnFailure}, + s.Caveats) +} + +func TestAdviseSetNotNullGetsConstraintSequence(t *testing.T) { + report, err := suggest.Advise("ALTER TABLE t ALTER COLUMN c SET NOT NULL") + require.NoError(t, err) + require.Len(t, report.Suggestions, 1) + s := report.Suggestions[0] + require.Len(t, s.Recommended, 4, "add NOT VALID, validate, set not null, drop scaffold") + assert.Equal(t, + []suggest.Caveat{suggest.CaveatSeparateTransactions, suggest.CaveatValidationScan}, + s.Caveats) +} + +// Statements outside the advisory surface — refusals, table rewrites, +// destructive drops, and forms already safe as written — produce no +// suggestions; they are lint findings, not advice. +func TestAdviseSkipsNonRewritableStatements(t *testing.T) { + report, err := suggest.Advise(` + ALTER TABLE t ADD CONSTRAINT no_overlap EXCLUDE USING gist (room WITH =); + ALTER TABLE t ALTER COLUMN id TYPE bigint; + ALTER TABLE t DROP COLUMN legacy; + ALTER TABLE t ADD CONSTRAINT t_fk FOREIGN KEY (o) REFERENCES orders (id) NOT VALID; + `) + require.NoError(t, err) + assert.Empty(t, report.Suggestions) +} + +// A multi-operation statement gets no partial rewrite; a rewrite of one +// subcommand of a compound ALTER would be misleading. +func TestAdviseSkipsMultiOperationStatements(t *testing.T) { + report, err := suggest.Advise( + "ALTER TABLE t ALTER COLUMN c SET NOT NULL, ADD COLUMN d int") + require.NoError(t, err) + assert.Empty(t, report.Suggestions) +} + +// Suggestion indexes track the statement position in the script, not the +// suggestion count. +func TestAdviseMultiStatementIndexes(t *testing.T) { + report, err := suggest.Advise(` + CREATE TABLE t (id bigint PRIMARY KEY); + CREATE INDEX t_c_idx ON t (c); + DROP INDEX t_c_idx; + `) + require.NoError(t, err) + require.Len(t, report.Suggestions, 2) + assert.Equal(t, 2, report.Suggestions[0].Statement) + assert.Equal(t, 3, report.Suggestions[1].Statement) +} + +// The JSON shape is the automation-facing contract: exact keys, exact +// omissions, suggestions as [] when clean. +func TestReportJSONShape(t *testing.T) { + report, err := suggest.Advise("CREATE INDEX t_c_idx ON t (c)") + require.NoError(t, err) + raw, err := json.Marshal(report) + require.NoError(t, err) + require.Len(t, report.Suggestions, 1) + recommended, err := json.Marshal(report.Suggestions[0].Recommended) + require.NoError(t, err) + assert.JSONEq(t, `{ + "format_version": 1, + "suggestions": [ + { + "statement": 1, + "original": "CREATE INDEX t_c_idx ON t USING btree (c)", + "operation": "CREATE INDEX t_c_idx", + "reason": "safer-idiom", + "recommended": `+string(recommended)+`, + "caveats": ["non-transactional", "invalid-index-on-failure"] + } + ] + }`, string(raw)) +} + +func TestReportJSONCleanSuggestionsAreEmptyArray(t *testing.T) { + report, err := suggest.Advise("CREATE TABLE t (id int)") + require.NoError(t, err) + raw, err := json.Marshal(report) + require.NoError(t, err) + assert.JSONEq(t, `{ + "format_version": 1, + "suggestions": [] + }`, string(raw)) +}