feat(lint): flag a CONCURRENTLY operation inside a transaction block

demo/tour.sh

@@ -326,7 +326,7 @@ run_offline() {
     if [ "$CHECK" = 1 ]; then
         out=$("$PGS" lint --json risky.sql) || status=$?
         assert_eq "lint exit" 1 "$status"
-        assert_eq "lint format_version" 2 "$(jq -r '.format_version' <<<"$out")"
+        assert_eq "lint format_version" 3 "$(jq -r '.format_version' <<<"$out")"
         assert_eq "lint errors" 1 "$(jq -r '.errors' <<<"$out")"
     else
         "$PGS" lint risky.sql || echo "(exit $? — error findings gate, by design)"

docs/cli-output-examples.md

@@ -453,7 +453,7 @@ ALTER TABLE users ADD CONSTRAINT users_email_key UNIQUE (email);
 ```console
 $ pg-sprite lint /tmp/changes.sql --json
 {
-  "format_version": 2,
+  "format_version": 3,
   "postgres_versions": "14-18",
   "findings": [
     {

docs/lint-report.md

@@ -15,9 +15,10 @@ and the closed vocabularies below (codes, severities, and the embedded planner r
 adding a value to any of them is a contract change and bumps `format_version`, even if no
 field is added or renamed.
 
-The current version is **2**: version 2 added the `replica-identity` reason to the embedded
-Reasons vocabulary. The linter itself never reports it, because the reason is judged from live
-replica identity facts the linter does not have.
+The current version is **3**: version 3 added the `concurrent-in-transaction` code. Version 2
+added the `replica-identity` reason to the embedded Reasons vocabulary. The linter itself never
+reports that reason, because it is judged from live replica identity facts the linter does not
+have.
 
 ### Relationship to the plan report
 
@@ -70,7 +71,7 @@ codes make the conservatism visible instead of burying it:
 | `operation` | string | always | Operator-facing label of the flagged operation (`DROP COLUMN legacy_a`). Display only — never branch on it. |
 | `code` | string | always | The typed finding kind (see Codes). Automation branches on this, never on prose. |
 | `severity` | string | always | What the engine would do about it (see Severities). |
-| `reason` | string | classifier findings | The planner's typed cause, drawn from the plan report's Reasons vocabulary. Absent for destructive findings, which are a property of the operation, not a routing decision. |
+| `reason` | string | classifier findings | The planner's typed cause, drawn from the plan report's Reasons vocabulary. Absent for `destructive` and `concurrent-in-transaction` findings, which are a property of the operation or the script, not a routing decision. |
 | `suggestion` | array | when constructible | The ordered safer SQL, present only for `blocking-idiom` findings where the planner constructed the rewrite. A safer form of the submitted statement, not a semantic equivalent — running it by hand forgoes the engine's execution-time guards (invalid-index detection after a concurrent build). Its absence still means the submitted form blocks — the planner does not construct rewrites for multi-operation statements or for operations that need catalog knowledge (ATTACH PARTITION's proving CHECK); the [suggest report](suggest-report.md) emits typed guidance naming the manual path for exactly those cases. |
 | `suggestion_execution` | string | with `suggestion` | The typed execution contract for `suggestion`, drawn from the plan report's Execution contracts vocabulary (`autocommit-each-step`: each step in its own implicit transaction, never inside an enclosing transaction block; a failed step leaves partial state the runner must detect and recover). A consumer that runs the suggestion branches on this. Present exactly when `suggestion` is. |
 
@@ -84,6 +85,7 @@ codes make the conservatism visible instead of burying it:
 | `possible-table-rewrite` | warning | `ALTER TABLE t ALTER COLUMN c TYPE bigint` | The linter cannot verify the operation against live column facts, so the engine would fail closed to the rewrite path — but the change may be a free relabel a live database would prove. |
 | `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. |
+| `concurrent-in-transaction` | error | `BEGIN; CREATE INDEX CONCURRENTLY i ON t (c); COMMIT` | A `CONCURRENTLY` operation sits inside a transaction block the script opens with `BEGIN` or `START TRANSACTION`. PostgreSQL refuses every `CONCURRENTLY` form inside a block, so the script fails at that statement and the block rolls back. Run the statement outside the block. A savepoint, or a `COMMIT AND CHAIN` that opens the next block at once, leaves the script inside its block. |
 
 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

docs/low-level-design.md

@@ -347,7 +347,7 @@ Classification belongs to `pkg/planner`; `pkg/statement` supplies typed operatio
 
 | Invocation | Behaviour |
 | --- | --- |
-| `lint` | Offline (no database): classify every statement with zero live facts and report typed findings — unsupported operations are errors (non-zero exit), blocking idioms (with the safer SQL), conservative rewrites, and destructive drops are warnings. |
+| `lint` | Offline (no database): classify every statement with zero live facts and report typed findings — unsupported operations and a `CONCURRENTLY` operation inside an explicit `BEGIN` … `COMMIT` block, which PostgreSQL refuses, are errors (non-zero exit), blocking idioms (with the safer SQL), conservative rewrites, and destructive drops are warnings. |
 | `diff` / `migrate --dry-run` | Print the classified, routed plan and safer SQL where applicable. **Never writes the live table.** `diff` has no `--dry-run` flag because it never executes the plan (its desired-state diff does run the desired DDL in an always-rolled-back scratch transaction — see the scratch-schema note above). |
 | `migrate` (default) | Run the statement gate, classify and route exactly as dry-run would, then execute the routed SQL: the planner's **safer native sequence by default** when the submitted form blocks, the submitted form as a bounded optimistic attempt otherwise. The verdict's `executed_sql` reports the substitution. |
 | `migrate --force` | Run the statement **exactly as submitted**, overriding a safer-sequence substitution or a rewrite-required / backend-unavailable refusal. Gated — see below. |

docs/postgres-online-ddl-reference.md

@@ -526,6 +526,19 @@ about existing rows and is recreated by a metadata-only statement). Emitted
 alongside the routing decision as a warning so destructive intent is always
 visible in review.
 
+### `concurrent-in-transaction`
+
+| Verdict | Lock | Scan / rewrite | Lint exit |
+|---|---|---|---|
+| lint error — the script fails at this statement | none: PostgreSQL refuses the statement before it starts | — | 1 |
+
+A `CONCURRENTLY` operation (`CREATE INDEX`, `DROP INDEX`, `REINDEX`, or `DETACH
+PARTITION`) sits inside a transaction block the script opens with `BEGIN` or
+`START TRANSACTION`. PostgreSQL refuses every `CONCURRENTLY` form inside a block,
+so the script stops at that statement and the whole block rolls back. Move the
+statement out of the block and run it on its own. Only `lint` reports this code:
+it is a property of the script, and `migrate` takes one statement at a time.
+
 ### `rewrite-required`
 
 | Verdict | Lock (as written) | Scan / rewrite | Dry-run exit |

internal/cli/lint_test.go

@@ -113,6 +113,30 @@ func TestLintTextSuggestionCarriesExecutionCaveat(t *testing.T) {
 		"the caveat names the recovery check a manual run takes on")
 }
 
+// A CONCURRENTLY statement inside a BEGIN … COMMIT block names why the
+// script fails there and how to fix it, and links its own reference
+// section. This is the renderer's own unit test — everything else asserts
+// typed fields.
+func TestLintTextConcurrentInTransactionSaysHowToFixIt(t *testing.T) {
+	var out strings.Builder
+	cmd := LintCmd{}
+	err := cmd.runLint(strings.NewReader("BEGIN;\nCREATE INDEX CONCURRENTLY i ON t (c);\nCOMMIT;\n"), &out)
+	require.ErrorIs(t, err, ErrLintFindings)
+	assert.Contains(t, out.String(), `<stdin>:2:1:
+  CREATE INDEX CONCURRENTLY i ON t (c);
+
+error[concurrent-in-transaction]:
+  CREATE INDEX i — PostgreSQL refuses CONCURRENTLY inside a transaction
+  block, so the script fails here and the block rolls back
+
+help:
+  run this statement on its own, outside the BEGIN … COMMIT block
+
+docs:
+  `+onlineDDLReferenceURL+`#concurrent-in-transaction
+`)
+}
+
 func TestLintParseFailureIsErrorNotFinding(t *testing.T) {
 	var out strings.Builder
 	cmd := LintCmd{}

internal/cli/lint_text.go

@@ -31,6 +31,9 @@ func writeLintText(out io.Writer, pal palette, name string, report lint.Report)
 			statement = f.Statement
 		}
 		w.diag(string(f.Severity), string(f.Code), lintMessage(f))
+		if f.Code == lint.CodeConcurrentInTransaction {
+			w.diag("help", "", "run this statement on its own, outside the BEGIN … COMMIT block")
+		}
 		if f.Code == lint.CodePossibleTableRewrite {
 			w.diag("note", "", "the table was not introspected, so this is the conservative classification; with live facts the same change may classify as cheaper")
 		}
@@ -60,6 +63,9 @@ func lintMessage(f lint.Finding) string {
 	if f.Code == lint.CodeDestructive {
 		return f.Operation + " — discards live data or structure"
 	}
+	if f.Code == lint.CodeConcurrentInTransaction {
+		return f.Operation + " — PostgreSQL refuses CONCURRENTLY inside a transaction block, so the script fails here and the block rolls back"
+	}
 	if f.Reason != "" {
 		return fmt.Sprintf("%s — %s", f.Operation, impactText(f.Reason))
 	}

pkg/lint/lint.go

@@ -17,8 +17,9 @@ import (
 // FormatVersion identifies the report contract. A consumer must reject a
 // report whose version it does not understand instead of guessing at the
 // field semantics. Version 2 added the replica-identity planner reason to
-// the embedded Reasons vocabulary.
-const FormatVersion = 2
+// the embedded Reasons vocabulary; version 3 added the
+// concurrent-in-transaction code.
+const FormatVersion = 3
 
 // Severity ranks a finding by what the engine would do with it.
 type Severity string
@@ -71,6 +72,12 @@ const (
 	// whether an index is unique — and a dropped unique index whose gap
 	// admitted duplicates cannot be recreated at all.
 	CodeDestructive Code = "destructive"
+	// CodeConcurrentInTransaction: a CONCURRENTLY operation sits inside
+	// an explicit transaction block the script opens with BEGIN or START
+	// TRANSACTION. PostgreSQL refuses every CONCURRENTLY form inside a
+	// block, so the script fails at that statement and the block rolls
+	// back. Run the statement outside the block, on its own.
+	CodeConcurrentInTransaction Code = "concurrent-in-transaction"
 )
 
 // Finding is one lint result: the statement it is about, the typed code,
@@ -142,13 +149,24 @@ func Check(sql string) (Report, error) {
 		PostgresVersions: planner.RulesPostgresVersions,
 		Findings:         []Finding{},
 	}
+	inBlock := false
 	var classifier planner.ScriptClassifier
 	for i, stmt := range stmts {
-		findings, err := checkStatement(&classifier, i+1, stmt)
+		findings, err := checkStatement(&classifier, i+1, stmt, inBlock)
 		if err != nil {
 			return Report{}, fmt.Errorf("statement %d: %w", i+1, err)
 		}
 		report.Findings = append(report.Findings, findings...)
+		control, err := statement.TransactionControlOf(stmt.SQL)
+		if err != nil {
+			return Report{}, fmt.Errorf("statement %d: %w", i+1, err)
+		}
+		switch control {
+		case statement.TransactionBegin:
+			inBlock = true
+		case statement.TransactionEnd:
+			inBlock = false
+		}
 	}
 	for _, f := range report.Findings {
 		if f.Severity == SeverityError {
@@ -164,8 +182,11 @@ func Check(sql string) (Report, error) {
 // from the classifier's routing decisions: a routing finding where the
 // route warrants one, and a destructive finding where the decision is
 // marked destructive. The classifier is the single rulebook — the linter
-// adds severity and presentation, never a second opinion.
-func checkStatement(classifier *planner.ScriptClassifier, index int, stmt statement.SourceStatement) ([]Finding, error) {
+// adds severity and presentation, never a second opinion. The one finding
+// it adds is about the script rather than the statement: a CONCURRENTLY
+// operation inside an explicit transaction block, which PostgreSQL refuses
+// whatever the classifier makes of the operation.
+func checkStatement(classifier *planner.ScriptClassifier, index int, stmt statement.SourceStatement, inBlock bool) ([]Finding, error) {
 	classified, err := classifier.Classify(stmt.SQL)
 	if err != nil {
 		return nil, err
@@ -175,6 +196,21 @@ func checkStatement(classifier *planner.ScriptClassifier, index int, stmt statem
 		f.Statement, f.Line, f.Column, f.SQL = index, stmt.Line, stmt.Column, stmt.SQL
 		return f
 	}
+	if inBlock {
+		ops, err := statement.ParseOps(stmt.SQL)
+		if err != nil {
+			return nil, err
+		}
+		for _, op := range ops {
+			if op.Concurrent {
+				findings = append(findings, place(Finding{
+					Operation: op.Describe(),
+					Code:      CodeConcurrentInTransaction,
+					Severity:  SeverityError,
+				}))
+			}
+		}
+	}
 	for _, d := range classified.Decisions {
 		if f, flagged := decisionFinding(d); flagged {
 			findings = append(findings, place(f))

pkg/lint/lint_test.go

@@ -196,7 +196,7 @@ func TestReportJSONShape(t *testing.T) {
 	suggestion, err := json.Marshal(report.Findings[0].Suggestion)
 	require.NoError(t, err)
 	assert.JSONEq(t, `{
-		"format_version": 2,
+		"format_version": 3,
 		"postgres_versions": "14-18",
 		"findings": [
 			{
@@ -223,14 +223,77 @@ func TestReportJSONCleanFindingsAreEmptyArray(t *testing.T) {
 	raw, err := json.Marshal(report)
 	require.NoError(t, err)
 	assert.JSONEq(t, `{
-		"format_version": 2,
+		"format_version": 3,
 		"postgres_versions": "14-18",
 		"findings": [],
 		"errors": 0,
 		"warnings": 0
 	}`, string(raw))
 }
 
+// A CONCURRENTLY operation inside an explicit transaction block is an
+// error whatever the classifier makes of it, and only while the block is
+// open: a savepoint or COMMIT AND CHAIN keeps the script inside it, and a
+// COMMIT, ROLLBACK, or PREPARE TRANSACTION closes it.
+func TestCheckFlagsConcurrentInsideTransactionBlock(t *testing.T) {
+	cases := []struct {
+		name   string
+		script string
+		// flagged lists the statement indexes carrying the finding.
+		flagged []int
+	}{
+		{name: "create index in a block", script: "BEGIN; CREATE INDEX CONCURRENTLY i ON t (c); COMMIT", flagged: []int{2}},
+		{name: "start transaction", script: "START TRANSACTION; CREATE INDEX CONCURRENTLY i ON t (c); COMMIT", flagged: []int{2}},
+		{name: "drop index", script: "BEGIN; DROP INDEX CONCURRENTLY i; COMMIT", flagged: []int{2}},
+		{name: "reindex", script: "BEGIN; REINDEX INDEX CONCURRENTLY i; COMMIT", flagged: []int{2}},
+		{name: "reindex with concurrently true", script: "BEGIN; REINDEX (CONCURRENTLY true) INDEX i; COMMIT", flagged: []int{2}},
+		{name: "reindex with concurrently false", script: "BEGIN; REINDEX (CONCURRENTLY false) INDEX i; COMMIT"},
+		{name: "reindex with concurrently 0", script: "BEGIN; REINDEX (CONCURRENTLY 0) INDEX i; COMMIT"},
+		{name: "detach partition", script: "BEGIN; ALTER TABLE t DETACH PARTITION p CONCURRENTLY; COMMIT", flagged: []int{2}},
+		{name: "block never closed", script: "BEGIN; CREATE INDEX CONCURRENTLY i ON t (c)", flagged: []int{2}},
+		{name: "after a savepoint", script: "BEGIN; SAVEPOINT s; CREATE INDEX CONCURRENTLY i ON t (c); COMMIT", flagged: []int{3}},
+		{name: "after commit and chain", script: "BEGIN; COMMIT AND CHAIN; CREATE INDEX CONCURRENTLY i ON t (c); COMMIT", flagged: []int{3}},
+		{name: "after commit", script: "BEGIN; ALTER TABLE t ADD COLUMN c int; COMMIT; CREATE INDEX CONCURRENTLY i ON t (c)"},
+		{name: "after rollback", script: "BEGIN; ROLLBACK; CREATE INDEX CONCURRENTLY i ON t (c)"},
+		{name: "after prepare transaction", script: "BEGIN; PREPARE TRANSACTION 'g'; CREATE INDEX CONCURRENTLY i ON t (c)"},
+		{name: "no block", script: "CREATE INDEX CONCURRENTLY i ON t (c)"},
+		{name: "blocking form in a block", script: "BEGIN; CREATE INDEX i ON t (c); COMMIT"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			report, err := lint.Check(tc.script)
+			require.NoError(t, err)
+			var flagged []int
+			for _, f := range report.Findings {
+				if f.Code != lint.CodeConcurrentInTransaction {
+					continue
+				}
+				flagged = append(flagged, f.Statement)
+				assert.Equal(t, lint.SeverityError, f.Severity)
+				assert.Empty(t, f.Reason)
+				assert.Empty(t, f.Suggestion)
+			}
+			assert.Equal(t, tc.flagged, flagged)
+		})
+	}
+}
+
+func TestCheckConcurrentInTransactionNamesTheOperation(t *testing.T) {
+	report, err := lint.Check("BEGIN;\nCREATE INDEX CONCURRENTLY t_c_idx ON t (c);\nCOMMIT;")
+	require.NoError(t, err)
+	var found *lint.Finding
+	for i := range report.Findings {
+		if report.Findings[i].Code == lint.CodeConcurrentInTransaction {
+			found = &report.Findings[i]
+		}
+	}
+	require.NotNil(t, found)
+	assert.Equal(t, 2, found.Statement)
+	assert.Equal(t, 2, found.Line)
+	assert.Equal(t, "CREATE INDEX CONCURRENTLY t_c_idx ON t (c)", found.SQL)
+	assert.Equal(t, "CREATE INDEX t_c_idx", found.Operation)
+}
+
 // Two unnamed CHECKs on one column in one script: the second finding's
 // suggestion names its constraint past the first's, as the server would.
 func TestCheckUnnamedChecksInOneScriptSuggestDistinctNames(t *testing.T) {