fix(github): classify and format plan DDL under its own dialect

[Kiran01bm]

Plan comments now classify and format each DDL statement under the target database's own SQL dialect instead of always the MySQL grammar.

Why

The plan comment's summary counts (tables to create/alter/drop) and its DDL blocks both ran every statement through the MySQL parser. Valid PostgreSQL statements (uuid, jsonb, bytea columns, and other Postgres-only constructs) were unparseable under that grammar, so the summary miscounted them and the DDL block reformatted them under the wrong dialect's quoting rules.

What

• ddl.FormatDDLForDialect(dialect, stmt): the MySQL family keeps the existing FormatDDL treatment; any other dialect renders its own parser's canonical form with the same line-break layout (each column/clause on its own line), skipping only the MySQL-specific lowercasing pass. Like FormatDDL, it is a best-effort display formatter — an unparseable statement renders as-is, and a dialect with no registered parser is logged.
• Plan summary counting resolves the statement parser from the plan's database type. When no parser is registered for the dialect, the summary falls back to the raw statement total instead of reporting miscounted zeros.
• Plan DDL blocks format each statement with FormatDDLForDialect.
• PostgreSQL plans group changes by schema, so they share MySQL's "Schema Name" heading label and the single-namespace heading suppression; Vitess and Strata keep the keyspace vocabulary.

MySQL-family plans render byte-identically; TEMPLATES.md gains a new Postgres Plan preview section and is otherwise unchanged.

Before / after

Before:
  statements ──▶ MySQL parser (classify + format)
                   postgres DDL: unparseable → counted as 0,
                   reformatted under MySQL quoting rules
                 ──▶ plan comment

After:
  statements ──▶ dialect := DialectForDatabaseType(database_type)
                   ├─ mysql / vitess / strata ──▶ MySQL parser (unchanged)
                   └─ postgres ────────────────▶ Postgres parser
                 classify + canonicalize + line-break under own grammar
                 ──▶ plan comment

Rendered: PostgreSQL plan comment

Schema Change Plan — Staging

Database: testapp | Type: PostgreSQL

Requested by @jackjackbits at 2026-01-01 00:00:00 UTC · planned from abcdef1

CREATE TABLE sessions (
    id uuid PRIMARY KEY,
    user_id bigint NOT NULL,
    payload jsonb,
    created_at timestamptz NOT NULL DEFAULT now()
);

ALTER TABLE users
    ADD COLUMN last_seen_at timestamptz,
    ADD COLUMN preferences jsonb;

📋 Plan: 1 table to create, 1 table to alter

---

▶️ To apply all schema changes from this PR, comment:

schemabot apply -e staging

[Copilot]

Pull request overview

This PR fixes plan comment rendering so DDL classification and formatting are performed using the target database’s own SQL dialect (instead of always using the MySQL grammar), improving correctness for PostgreSQL plans and keeping MySQL-family output unchanged.

Changes:

• Add ddl.FormatDDLForDialect and use it when rendering plan DDL blocks so statements are formatted under the plan’s dialect.
• Classify plan statements for summary counting using the dialect-specific parser derived from DatabaseType, with a raw-statement fallback when classification isn’t available.
• Extend/adjust tests and preview fixtures to include DatabaseType and validate PostgreSQL plan rendering/counting behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/webhook/templates/plan.go	Switch plan summary classification to dialect-specific parser and format DDL blocks via FormatDDLForDialect.
pkg/ddl/format.go	Introduce FormatDDLForDialect to canonicalize statements via the dialect’s parser (best-effort).
pkg/ddl/format_test.go	Add coverage for MySQL vs Postgres formatting behavior and best-effort fallbacks.
pkg/webhook/templates/plan_dialect_test.go	New test asserting PostgreSQL plans classify/render without MySQL backtick quoting.
pkg/webhook/templates/summarize_changes_test.go	Update tests to provide DatabaseType and verify PostgreSQL classification affects summary counts.
pkg/webhook/templates/preview.go	Populate DatabaseType in preview plan data so previews exercise dialect-aware logic.
pkg/webhook/plan_test.go	Update multi-env plan tests to use the canonical DatabaseType values (e.g. vitess).

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[aparajon]

🤖 Adversarial correctness review, requested by @aparajon and performed by their agent. Reviewed at head 1a86fa89.

Verdict: this is the right fix in the right place — routing classification and formatting through the target's own parser, with a raw-statement-total fallback so an unclassifiable plan never reads as "no changes". Two findings, neither blocking: the fallback closes a narrower window than its comment claims, and the layout pass this PR newly points at Postgres DDL mis-splits quoted string literals.

Threading DatabaseType all the way to countStatementTypes and writeKeyspaceChanges is the correct seam, and adding a Postgres Plan scenario to TEMPLATES.md means the rendering is reviewable without a checkout.

Findings

1. The raw-total fallback only fires when every typed count is zero, so a mixed plan still undercounts. writePlanSummary adds the total only under len(parts) == 0, so any non-zero create/alter/drop suppresses it. I rendered a greenfield Postgres plan — CREATE TABLE sessions (...) plus CREATE INDEX sessions_user_id_idx and CREATE INDEX sessions_created_at_idx — and got 📋 **Plan**: **1** table to create, with SummarizeChanges (the check-run summary) agreeing at 1 create. The two index statements vanish from both surfaces. The comment directly above the new fallback says statements that classify as none of create/alter/drop "still count", which is only true when nothing else counted. This is pre-existing and also reproduces on MySQL (verified), but it becomes the common Postgres case rather than an edge one, since a greenfield Postgres table is idiomatically a CREATE TABLE plus its index statements — exactly the shape block/pg-sprite#63 makes reachable.

2. The layout pass splits inside single-quoted literals, so the rendered DDL is not the DDL that runs. layoutDDL → formatCreateTable respects parentheses but not quotes. CREATE TABLE t (..., note text DEFAULT 'x, y') renders as:

    note text DEFAULT 'x,
    y'

The literal now contains a newline and four spaces — an operator copying the block out of the PR comment gets a different default value. numeric(10, 2) and CHECK (kind IN ('a', 'b')) both survive, so it is specifically the quote case. Pre-existing on the MySQL path (verified identically there), but this PR is what routes Postgres canonical form into the same function, and text defaults and CHECK ... IN (...) lists are ordinary Postgres shapes.

Action items

1. (Finding 1) Make the raw-statement total additive rather than a last-resort substitute — or count every statement type the vocabulary knows, so index and rename statements are not silently dropped from a mixed plan.
2. (Finding 1) Either way, fix the fallback's comment: as written it promises coverage the code only provides when all three typed counts are zero.
3. (Finding 2) Teach the comma splitter to skip over single-quoted literals (it already tracks parenthesis depth), so a string default or an IN (...) list is never broken across lines.

Verified (tried to break, couldn't)

Every non-test PlanCommentData construction site sets DatabaseType — plan.go, rollback.go, preview.go, preview_sharded.go — so no existing plan silently degrades to the raw-total fallback; DialectForDatabaseType covers all four storage.DatabaseType* values and lowercases its input, so "PostgreSQL", "Vitess" and friends all resolve; DatabaseType is the canonical engine string with the display label derived through schemaChangePlanDatabaseTypeLabel, which is why the "PlanetScale" → "vitess" test-data correction was needed and it is complete; the plan comment's summary and SummarizeChanges still derive from the same helper and agree statement for statement; both unclassifiable branches log with the database type and keyspace, satisfying the no-silent-fallback rule; the Postgres ALTER rendering keeps bare identifiers where the MySQL formatter would backtick them, which is what pins the routing; no test functions were deleted and no assertions weakened anywhere in the diff; go build ./... plus ./pkg/webhook/... ./pkg/ddl/... pass locally at head, and CI is green across all 37 checks.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving on @aparajon's behalf after the adversarial correctness review above (no blocking findings).

This stamp was left by Claude Code (claude-opus-5).

[Kiran01bm]

Review response from Kiran's (@Kiran01bm) code review assessment agent (Amp / Claude Opus 4.5)

Both findings are valid; the fallback comment is corrected in this PR, and the two behavioural fixes (additive statement totals, quote-aware comma splitting) are tracked as internal follow-ups since both predate this PR and touch surfaces beyond it.

#	Finding	Status	Explanation
1	Raw-total fallback only fires when every typed count is zero, so a mixed plan (Postgres CREATE TABLE + CREATE INDEXes) undercounts and drops the index statements from both the plan comment and SummarizeChanges	deferred	Valid and well demonstrated. Pre-existing on MySQL too, but this PR makes it the common Postgres case. Making the total additive (or counting every known statement type) changes both the plan comment and the check-run summary, so it deserves its own reviewable PR — tracked as an internal follow-up covering writePlanSummary and SummarizeChanges together.
3	layoutDDL → formatCreateTable splits on commas inside single-quoted literals, so DEFAULT 'x, y' renders with an embedded newline — the displayed DDL is not the DDL that runs	deferred	Valid; pre-existing on the MySQL path and now reachable from Postgres canonical form. The splitter already tracks parenthesis depth, so teaching it to skip quoted literals is a contained pkg/ddl fix — tracked as an internal follow-up.
2	The fallback's comment promises unclassified statements "still count", which is only true when nothing else counted	fixed	Comment rewritten to state the real coverage: it is a last-resort total, mixed plans render only the typed counts, and SummarizeChanges shares the behaviour — so the next reader of this code sees the gap the follow-up will close.