schema_risk/
recommendation.rs

1//! Rule-based fix suggestion engine.
2//!
3//! Given a list of parsed SQL statements and optional row-count metadata,
4//! this module produces structured `FixSuggestion` objects — each with a
5//! corrected SQL snippet or a step-by-step zero-downtime migration plan.
6//!
7//! ## Rules implemented
8//!
9//! | ID   | Trigger                              | Severity |
10//! |------|--------------------------------------|----------|
11//! | R01  | CREATE INDEX without CONCURRENTLY    | Blocking |
12//! | R02  | ADD COLUMN NOT NULL without DEFAULT  | Blocking |
13//! | R03  | DROP COLUMN on large table           | Warning  |
14//! | R04  | Missing index on foreign key column  | Warning  |
15//! | R05  | RENAME COLUMN                        | Blocking |
16//! | R06  | RENAME TABLE                         | Blocking |
17//! | R07  | ALTER COLUMN TYPE on large table     | Blocking |
18//! | R08  | Long ACCESS EXCLUSIVE lock duration  | Warning  |
19
20use crate::parser::ParsedStatement;
21use serde::{Deserialize, Serialize};
22use std::collections::HashMap;
23
24// ─────────────────────────────────────────────
25// Core types
26// ─────────────────────────────────────────────
27
28/// Severity of a fix suggestion — used for ordering and exit-code decisions.
29#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
30#[serde(rename_all = "snake_case")]
31pub enum FixSeverity {
32    /// A nice-to-have improvement.
33    Info,
34    /// May cause degraded performance or brief downtime on large tables.
35    Warning,
36    /// Will likely cause production downtime, data loss, or a failed migration.
37    Blocking,
38}
39
40impl std::fmt::Display for FixSeverity {
41    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
42        match self {
43            FixSeverity::Info => write!(f, "INFO"),
44            FixSeverity::Warning => write!(f, "WARNING"),
45            FixSeverity::Blocking => write!(f, "BLOCKING"),
46        }
47    }
48}
49
50/// A single recommended fix for a risky migration operation.
51#[derive(Debug, Clone, Serialize, Deserialize)]
52pub struct FixSuggestion {
53    /// Short rule identifier, e.g. "R01".
54    pub rule_id: String,
55    /// Short, actionable title shown in the terminal and CI comments.
56    pub title: String,
57    /// Full explanation: what will go wrong, and why the fix helps.
58    pub explanation: String,
59    /// Drop-in replacement SQL (if the fix is a single-statement rewrite).
60    pub fixed_sql: Option<String>,
61    /// Ordered migration steps for complex zero-downtime patterns.
62    pub migration_steps: Option<Vec<String>>,
63    /// How serious this finding is.
64    pub severity: FixSeverity,
65    /// Link to relevant PostgreSQL documentation or best-practices guide.
66    pub docs_url: Option<String>,
67    /// `true` when `apply_fixes()` can mechanically patch the raw SQL text.
68    pub auto_fixable: bool,
69}
70
71// ─────────────────────────────────────────────
72// Public API
73// ─────────────────────────────────────────────
74
75/// Analyse a slice of parsed statements and return a de-duplicated,
76/// severity-sorted list of `FixSuggestion`s.
77///
78/// `row_counts` maps table name → estimated row count. May be empty in
79/// offline mode; rules will fall back to heuristic thresholds.
80pub fn suggest_fixes(
81    statements: &[ParsedStatement],
82    row_counts: &HashMap<String, u64>,
83) -> Vec<FixSuggestion> {
84    let mut suggestions: Vec<FixSuggestion> = Vec::new();
85
86    for stmt in statements {
87        match stmt {
88            // ── R01: CREATE INDEX without CONCURRENTLY ────────────────────
89            ParsedStatement::CreateIndex {
90                table,
91                columns,
92                concurrently,
93                index_name,
94                unique,
95            } => {
96                if !concurrently {
97                    let rows = row_counts.get(table).copied().unwrap_or(0);
98                    let col_list = columns.join(", ");
99                    let unique_kw = if *unique { "UNIQUE " } else { "" };
100                    let name = index_name.as_deref().unwrap_or("idx_name");
101                    suggestions.push(rule_r01_index_concurrently(
102                        table, &col_list, unique_kw, name, rows,
103                    ));
104                }
105            }
106
107            // ── R02 + R09: ADD COLUMN — NOT NULL without DEFAULT / DEFAULT on old PG ──
108            ParsedStatement::AlterTableAddColumn { table, column } => {
109                let rows = row_counts.get(table).copied().unwrap_or(0);
110                // R02: NOT NULL without any DEFAULT will fail on non-empty tables
111                if !column.nullable && !column.has_default {
112                    suggestions.push(rule_r02_add_not_null(
113                        table,
114                        &column.name,
115                        &column.data_type,
116                        rows,
117                    ));
118                }
119                // R09: Adding a column WITH DEFAULT — safe on PG11+, table-rewrite on PG10-
120                if column.has_default {
121                    suggestions.push(rule_r09_add_column_default(
122                        table,
123                        &column.name,
124                        &column.data_type,
125                        rows,
126                    ));
127                }
128            }
129
130            // ── R03: DROP COLUMN ──────────────────────────────────────────
131            ParsedStatement::AlterTableDropColumn { table, column, .. } => {
132                let rows = row_counts.get(table).copied().unwrap_or(0);
133                suggestions.push(rule_r03_drop_column(table, column, rows));
134            }
135
136            // ── R04: ADD FOREIGN KEY without index on FK column ───────────
137            ParsedStatement::AlterTableAddForeignKey { table, fk } => {
138                suggestions.push(rule_r04_missing_fk_index(table, &fk.columns));
139            }
140
141            // ── R05: RENAME COLUMN ────────────────────────────────────────
142            ParsedStatement::AlterTableRenameColumn { table, old, new } => {
143                suggestions.push(rule_r05_rename_column(table, old, new));
144            }
145
146            // ── R06: RENAME TABLE ─────────────────────────────────────────
147            ParsedStatement::AlterTableRenameTable { old, new } => {
148                suggestions.push(rule_r06_rename_table(old, new));
149            }
150
151            // ── R07: ALTER COLUMN TYPE on a non-trivial table ─────────────
152            ParsedStatement::AlterTableAlterColumnType {
153                table,
154                column,
155                new_type,
156            } => {
157                let rows = row_counts.get(table).copied().unwrap_or(0);
158                // Always emit for type changes — even small tables deserve a warning
159                suggestions.push(rule_r07_alter_column_type(table, column, new_type, rows));
160            }
161
162            // ── R08: Long ACCESS EXCLUSIVE lock (catch-all for other DDL) ─
163            ParsedStatement::DropTable { tables, .. } => {
164                for t in tables {
165                    let rows = row_counts.get(t).copied().unwrap_or(0);
166                    let est = estimate_lock_secs(rows);
167                    if est > 5 {
168                        suggestions.push(rule_r08_long_lock(&format!("DROP TABLE {t}"), est));
169                    }
170                }
171            }
172
173            _ => {}
174        }
175    }
176
177    // Sort so most severe issues appear first
178    suggestions.sort_by(|a, b| b.severity.cmp(&a.severity));
179    suggestions
180}
181
182/// Rewrite the raw SQL text, applying all auto-fixable suggestions in-place.
183///
184/// Currently handles:
185/// - **R01**: Inserts `CONCURRENTLY` after `CREATE [UNIQUE] INDEX`
186///
187/// **B-04 fix**: If a `CREATE INDEX` is inside a `BEGIN` / `COMMIT` transaction
188/// block, inserts a warning comment instead of adding `CONCURRENTLY`, because
189/// PostgreSQL forbids `CREATE INDEX CONCURRENTLY` inside a transaction.
190///
191/// Returns the modified SQL string. Lines not matched by any rule are
192/// passed through unchanged.
193pub fn apply_fixes(sql: &str, suggestions: &[FixSuggestion]) -> String {
194    let has_r01 = suggestions
195        .iter()
196        .any(|s| s.rule_id == "R01" && s.auto_fixable);
197    if !has_r01 {
198        return sql.to_string();
199    }
200    if is_inside_transaction_block(sql) {
201        // B-04: Cannot use CONCURRENTLY inside a transaction — emit a comment
202        return rewrite_index_concurrent_in_txn(sql);
203    }
204    rewrite_index_concurrent(sql)
205}
206
207/// Returns `true` if the SQL text contains an explicit transaction block
208/// (`BEGIN`, `START TRANSACTION`) with a corresponding `COMMIT` or `ROLLBACK`.
209fn is_inside_transaction_block(sql: &str) -> bool {
210    let upper = sql.to_uppercase();
211    let has_begin = upper.contains("BEGIN") || upper.contains("START TRANSACTION");
212    let has_commit = upper.contains("COMMIT") || upper.contains("ROLLBACK");
213    has_begin && has_commit
214}
215
216/// When CONCURRENTLY cannot be added (inside a transaction), insert an
217/// explanatory comment before each offending CREATE INDEX statement.
218fn rewrite_index_concurrent_in_txn(sql: &str) -> String {
219    sql.lines()
220        .flat_map(|line| {
221            let upper = line.to_uppercase();
222            if (upper.contains("CREATE INDEX") || upper.contains("CREATE UNIQUE INDEX"))
223                && !upper.contains("CONCURRENTLY")
224            {
225                vec![
226                    "-- ⚠️  schemarisk: Cannot use CONCURRENTLY inside a transaction block."
227                        .to_string(),
228                    "-- Remove BEGIN/COMMIT wrapper and run this statement standalone.".to_string(),
229                    line.to_string(),
230                ]
231            } else {
232                vec![line.to_string()]
233            }
234        })
235        .collect::<Vec<_>>()
236        .join("\n")
237}
238
239// ─────────────────────────────────────────────
240// Rule implementations
241// ─────────────────────────────────────────────
242
243/// R01 — CREATE INDEX without CONCURRENTLY.
244fn rule_r01_index_concurrently(
245    table: &str,
246    columns: &str,
247    unique_kw: &str,
248    name: &str,
249    rows: u64,
250) -> FixSuggestion {
251    let rows_note = if rows > 0 {
252        format!(
253            " The table has approximately {} rows — index build will take roughly {} seconds.",
254            fmt_rows(rows),
255            rows / 500_000 + 1
256        )
257    } else {
258        String::new()
259    };
260    FixSuggestion {
261        rule_id: "R01".to_string(),
262        title: "Use CREATE INDEX CONCURRENTLY to avoid blocking writes".to_string(),
263        explanation: format!(
264            "CREATE INDEX without CONCURRENTLY acquires a SHARE lock that blocks all \
265             INSERT, UPDATE, and DELETE statements for the entire duration of the index \
266             build.{rows_note} Using CONCURRENTLY performs two table scans and allows \
267             DML to continue throughout, at the cost of a longer total build time. \
268             Note: CONCURRENTLY cannot run inside a transaction block."
269        ),
270        fixed_sql: Some(format!(
271            "CREATE {unique_kw}INDEX CONCURRENTLY {name}\n  ON {table}({columns});"
272        )),
273        migration_steps: None,
274        severity: FixSeverity::Blocking,
275        docs_url: Some(
276            "https://www.postgresql.org/docs/current/sql-createindex.html#SQL-CREATEINDEX-CONCURRENTLY"
277                .to_string(),
278        ),
279        auto_fixable: true,
280    }
281}
282
283/// R02 — ADD COLUMN NOT NULL without DEFAULT.
284fn rule_r02_add_not_null(table: &str, column: &str, data_type: &str, rows: u64) -> FixSuggestion {
285    let rows_note = if rows > 0 {
286        format!(" (~{} rows)", fmt_rows(rows))
287    } else {
288        String::new()
289    };
290    FixSuggestion {
291        rule_id: "R02".to_string(),
292        title: format!(
293            "Adding NOT NULL column '{column}' without DEFAULT will fail on non-empty tables"
294        ),
295        explanation: format!(
296            "ALTER TABLE {table}{rows_note} ADD COLUMN {column} {data_type} NOT NULL \
297             fails immediately if the table contains any existing rows — PostgreSQL \
298             cannot assign a value to the new column for those rows. Use the \
299             three-step pattern below for a zero-downtime migration that works on \
300             PostgreSQL 11+ (which stores the DEFAULT without rewriting the table)."
301        ),
302        fixed_sql: None,
303        migration_steps: Some(vec![
304            "-- Step 1: Add the column with a sensible DEFAULT (instant on PG ≥ 11 for constant defaults)".to_string(),
305            format!("ALTER TABLE {table}"),
306            format!("  ADD COLUMN {column} {data_type} NOT NULL DEFAULT 'YOUR_DEFAULT_VALUE';"),
307            String::new(),
308            "-- Step 2: Back-fill outdated rows in batches (avoids long lock)".to_string(),
309            format!("-- Run this in a loop until 0 rows are updated:"),
310            format!("UPDATE {table}"),
311            format!("  SET {column} = 'YOUR_REAL_VALUE'"),
312            format!("  WHERE {column} = 'YOUR_DEFAULT_VALUE'"),
313            format!("  LIMIT 10000;"),
314            String::new(),
315            "-- Step 3: Remove the synthetic default (optional) once all rows are back-filled".to_string(),
316            format!("ALTER TABLE {table} ALTER COLUMN {column} DROP DEFAULT;"),
317        ]),
318        severity: FixSeverity::Blocking,
319        docs_url: Some(
320            "https://www.postgresql.org/docs/current/sql-altertable.html".to_string(),
321        ),
322        auto_fixable: false,
323    }
324}
325
326/// R03 — DROP COLUMN; advise two-phase deployment.
327fn rule_r03_drop_column(table: &str, column: &str, rows: u64) -> FixSuggestion {
328    let rows_note = if rows > 1_000_000 {
329        format!(" The table has ~{} rows.", fmt_rows(rows))
330    } else {
331        String::new()
332    };
333    let severity = if rows > 1_000_000 {
334        FixSeverity::Blocking
335    } else {
336        FixSeverity::Warning
337    };
338    FixSuggestion {
339        rule_id: "R03".to_string(),
340        title: format!("Deploy app changes before dropping column '{column}' from '{table}'"),
341        explanation: format!(
342            "DROP COLUMN is irreversible and holds an ACCESS EXCLUSIVE lock for the \
343             duration of the catalog update.{rows_note}  Any in-flight query or \
344             transaction referencing this column will fail.  Use the two-phase \
345             deployment pattern to ensure zero application downtime."
346        ),
347        fixed_sql: None,
348        migration_steps: Some(vec![
349            format!("-- Phase 1 — Application deploy (no DB change yet)"),
350            format!("-- Remove all code that reads or writes '{column}' from '{table}'"),
351            format!("-- Verify no ORM model, query, or migration references this column"),
352            String::new(),
353            format!("-- Phase 2 — Run this migration AFTER the app is fully deployed"),
354            format!("ALTER TABLE {table} DROP COLUMN IF EXISTS {column};"),
355            String::new(),
356            format!("-- Optional: create a backup before dropping"),
357            format!("-- CREATE TABLE {table}_{column}_backup AS"),
358            format!("--   SELECT id, {column} FROM {table};"),
359        ]),
360        severity,
361        docs_url: Some("https://www.postgresql.org/docs/current/sql-altertable.html".to_string()),
362        auto_fixable: false,
363    }
364}
365
366/// R04 — ADD FOREIGN KEY with no index on the referencing column.
367fn rule_r04_missing_fk_index(table: &str, fk_columns: &[String]) -> FixSuggestion {
368    let col_list = fk_columns.join(", ");
369    let col_snake = fk_columns.join("_");
370    FixSuggestion {
371        rule_id: "R04".to_string(),
372        title: format!("Add an index on FK column(s) ({col_list}) to prevent sequential scans"),
373        explanation: format!(
374            "PostgreSQL does NOT automatically create an index on foreign key columns. \
375             Without an index on '{table}.({col_list})', every DELETE or UPDATE on the \
376             referenced parent table triggers a full sequential scan of '{table}' to \
377             check referential integrity. This is catastrophic on tables larger than \
378             10k rows and grows linearly with table size."
379        ),
380        fixed_sql: Some(format!(
381            "CREATE INDEX CONCURRENTLY idx_{table}_{col_snake}\n  ON {table}({col_list});"
382        )),
383        migration_steps: None,
384        severity: FixSeverity::Warning,
385        docs_url: Some("https://www.postgresql.org/docs/current/indexes-intro.html".to_string()),
386        auto_fixable: false,
387    }
388}
389
390/// R05 — RENAME COLUMN; suggest expand-contract pattern.
391fn rule_r05_rename_column(table: &str, old: &str, new: &str) -> FixSuggestion {
392    FixSuggestion {
393        rule_id: "R05".to_string(),
394        title: format!("Use expand-contract pattern to rename '{old}' → '{new}' without downtime"),
395        explanation: format!(
396            "Renaming column '{old}' in '{table}' is a **breaking change** for every \
397             piece of application code, ORM model, stored procedure, view, and query \
398             that references the old column name. The expand-contract (aka \
399             parallel-change) pattern lets you rename a column while keeping both \
400             names alive during the transition window, giving you a zero-downtime \
401             path."
402        ),
403        fixed_sql: None,
404        migration_steps: Some(vec![
405            format!("-- Migration A: Add new column and sync data"),
406            format!("ALTER TABLE {table} ADD COLUMN {new} <same_type_as_{old}>;"),
407            format!("UPDATE {table} SET {new} = {old};"),
408            String::new(),
409            format!("-- Application deploy: Dual-write to both '{old}' and '{new}'"),
410            format!("--   (reads still come from '{old}')"),
411            String::new(),
412            format!("-- Application deploy: Switch reads to '{new}'"),
413            format!("--   (still write to both)"),
414            String::new(),
415            format!("-- Application deploy: Stop writing to '{old}'"),
416            String::new(),
417            format!("-- Migration B: Drop old column"),
418            format!("ALTER TABLE {table} DROP COLUMN {old};"),
419        ]),
420        severity: FixSeverity::Blocking,
421        docs_url: Some("https://martinfowler.com/bliki/ParallelChange.html".to_string()),
422        auto_fixable: false,
423    }
424}
425
426/// R06 — RENAME TABLE; suggest view-based transition.
427fn rule_r06_rename_table(old: &str, new: &str) -> FixSuggestion {
428    FixSuggestion {
429        rule_id: "R06".to_string(),
430        title: format!("Renaming table '{old}' → '{new}' breaks all downstream code instantly"),
431        explanation: format!(
432            "Renaming table '{old}' invalidates ALL queries, ORM models, foreign key \
433             constraints, views, triggers, and stored procedures that reference the old \
434             name. This is one of the most dangerous DDL operations. Use a transitional \
435             compatibility view (Option A) or the full expand-contract pattern (Option B) \
436             to provide a zero-downtime migration path."
437        ),
438        fixed_sql: None,
439        migration_steps: Some(vec![
440            format!("-- ── Option A: Rename + leave compatibility view ────────────────"),
441            format!("ALTER TABLE {old} RENAME TO {new};"),
442            format!("-- Create a view using the old name so existing queries still work:"),
443            format!("CREATE VIEW {old} AS SELECT * FROM {new};"),
444            format!("-- Remove the view after all app code has been updated to use '{new}'"),
445            String::new(),
446            format!("-- ── Option B: Full expand-contract ────────────────────────────"),
447            format!("-- Step 1: Create new table '{new}' with identical schema"),
448            format!("-- Step 2: Create triggers to sync writes from '{old}' → '{new}'"),
449            format!("-- Step 3: Back-fill '{new}' from '{old}' for historical rows"),
450            format!("-- Step 4: Deploy app to write to '{new}', read from both"),
451            format!("-- Step 5: Deploy app to read only from '{new}'"),
452            format!("-- Step 6: Drop triggers + old table '{old}'"),
453        ]),
454        severity: FixSeverity::Blocking,
455        docs_url: Some(
456            "https://braintreepayments.com/blog/safe-operations-for-high-volume-postgresql/"
457                .to_string(),
458        ),
459        auto_fixable: false,
460    }
461}
462
463/// R07 — ALTER COLUMN TYPE; advise shadow-column pattern.
464fn rule_r07_alter_column_type(
465    table: &str,
466    column: &str,
467    new_type: &str,
468    rows: u64,
469) -> FixSuggestion {
470    let rows_clause = if rows > 0 {
471        format!(" (~{} rows)", fmt_rows(rows))
472    } else {
473        String::new()
474    };
475    FixSuggestion {
476        rule_id: "R07".to_string(),
477        title: format!(
478            "Type change on '{table}.{column}' triggers full table rewrite under ACCESS EXCLUSIVE lock"
479        ),
480        explanation: format!(
481            "Changing the type of column '{column}' in '{table}'{rows_clause} causes \
482             PostgreSQL to rewrite the entire table while holding an ACCESS EXCLUSIVE \
483             lock. All reads and writes are blocked for the entire duration. For \
484             large tables this can mean minutes of downtime. Use the shadow-column \
485             pattern to perform the type change online."
486        ),
487        fixed_sql: None,
488        migration_steps: Some(vec![
489            format!("-- Step 1: Add shadow column with new type"),
490            format!("ALTER TABLE {table} ADD COLUMN {column}_v2 {new_type};"),
491            String::new(),
492            format!("-- Step 2: Back-fill in batches (prevents long lock)"),
493            format!("-- Run in a loop until UPDATE returns 0 rows:"),
494            format!("UPDATE {table}"),
495            format!("  SET {column}_v2 = {column}::{new_type}"),
496            format!("  WHERE {column}_v2 IS NULL"),
497            format!("  LIMIT 10000;"),
498            String::new(),
499            format!("-- Step 3: Deploy app to write to both columns"),
500            String::new(),
501            format!("-- Step 4: Atomically swap column names"),
502            format!("ALTER TABLE {table}"),
503            format!("  RENAME COLUMN {column}    TO {column}_old;"),
504            format!("ALTER TABLE {table}"),
505            format!("  RENAME COLUMN {column}_v2 TO {column};"),
506            String::new(),
507            format!("-- Step 5: Drop old column after verifying app health"),
508            format!("ALTER TABLE {table} DROP COLUMN {column}_old;"),
509        ]),
510        severity: FixSeverity::Blocking,
511        docs_url: Some(
512            "https://www.postgresql.org/docs/current/sql-altertable.html".to_string(),
513        ),
514        auto_fixable: false,
515    }
516}
517
518/// R09 — ADD COLUMN with DEFAULT; PG10 and below rewrites the full table.
519/// On PG11+ this is a metadata-only operation (no table rewrite).
520/// We always emit an info/warning so teams know which PG version changes the behaviour.
521fn rule_r09_add_column_default(
522    table: &str,
523    column: &str,
524    data_type: &str,
525    rows: u64,
526) -> FixSuggestion {
527    let rows_note = if rows > 0 {
528        format!(" (~{} rows)", fmt_rows(rows))
529    } else {
530        String::new()
531    };
532    FixSuggestion {
533        rule_id: "R09".to_string(),
534        title: format!(
535            "ADD COLUMN '{column}' WITH DEFAULT: safe on PG11+, table-rewrite on PG10 and below"
536        ),
537        explanation: format!(
538            "PostgreSQL 11 introduced the ability to add a column with a constant DEFAULT \
539             value without rewriting the table — the default is stored in the system catalog \
540             and applied on-the-fly at query time. On PostgreSQL 10 and below, adding a column \
541             with ANY default requires a full table rewrite{rows_note} holding ACCESS EXCLUSIVE \
542             lock. Always run 'SELECT version()' to confirm your PostgreSQL version. \
543             Pass --pg-version to SchemaRisk to get accurate risk scores."
544        ),
545        fixed_sql: None,
546        migration_steps: Some(vec![
547            format!("-- If you are on PostgreSQL 11+ (recommended):"),
548            format!("-- This is already safe — no changes needed."),
549            format!("ALTER TABLE {table}"),
550            format!("  ADD COLUMN {column} {data_type} DEFAULT <your_value>;"),
551            String::new(),
552            format!("-- ── If you are on PostgreSQL 10 or below ────────────────────"),
553            format!("-- Step 1: Add column as nullable (no default, no rewrite)"),
554            format!("ALTER TABLE {table} ADD COLUMN {column} {data_type};"),
555            String::new(),
556            format!("-- Step 2: Back-fill in batches during low-traffic window"),
557            format!("UPDATE {table}"),
558            format!("  SET {column} = <your_value>"),
559            format!("  WHERE {column} IS NULL"),
560            format!("  LIMIT 10000;"),
561            String::new(),
562            format!("-- Step 3: Set NOT NULL constraint after back-fill is complete"),
563            format!("ALTER TABLE {table} ALTER COLUMN {column} SET NOT NULL;"),
564        ]),
565        severity: FixSeverity::Info,
566        docs_url: Some(
567            "https://www.postgresql.org/docs/11/release-11.html#id-1.11.6.14.4".to_string(),
568        ),
569        auto_fixable: false,
570    }
571}
572
573/// R08 — Long ACCESS EXCLUSIVE lock; suggest lock_timeout + pg_repack.
574fn rule_r08_long_lock(description: &str, est_secs: u64) -> FixSuggestion {
575    FixSuggestion {
576        rule_id: "R08".to_string(),
577        title: format!("ACCESS EXCLUSIVE lock held for ~{est_secs}s — protect with lock_timeout"),
578        explanation: format!(
579            "The operation '{}' acquires ACCESS EXCLUSIVE lock for an estimated \
580             ~{est_secs} seconds. During this window every query, transaction, and \
581             connection waiting to access the table is queued. A single long-running \
582             transaction before the migration can turn a 30-second lock into minutes \
583             of application downtime. Set lock_timeout to prevent lock pile-up.",
584            shorten_desc(description, 80)
585        ),
586        fixed_sql: None,
587        migration_steps: Some(vec![
588            "-- Wrap your migration in a lock_timeout guard:".to_string(),
589            "BEGIN;".to_string(),
590            "  SET lock_timeout = '3s';        -- abort if lock is not acquired in 3s".to_string(),
591            "  SET statement_timeout = '120s'; -- abort if statement runs > 2 min".to_string(),
592            String::new(),
593            "  -- YOUR MIGRATION HERE".to_string(),
594            String::new(),
595            "COMMIT;".to_string(),
596            String::new(),
597            "-- For tables > 1GB, consider pg_repack for zero-downtime online rewrites:"
598                .to_string(),
599            "-- https://github.com/reorg/pg_repack".to_string(),
600            "-- pg_repack --dbname=<db_url> --table=<table_name>".to_string(),
601        ]),
602        severity: FixSeverity::Warning,
603        docs_url: Some("https://github.com/reorg/pg_repack".to_string()),
604        auto_fixable: false,
605    }
606}
607
608// ─────────────────────────────────────────────
609// SQL rewriting
610// ─────────────────────────────────────────────
611
612/// Rewrite raw SQL: insert `CONCURRENTLY` after every `CREATE [UNIQUE] INDEX`
613/// that does not already have it.
614///
615/// This preserves all other SQL untouched and is safe to apply repeatedly.
616pub fn rewrite_index_concurrent(sql: &str) -> String {
617    sql.lines()
618        .map(|line| {
619            let upper = line.to_uppercase();
620            // Already concurrent — leave alone
621            if upper.contains("CONCURRENTLY") {
622                return line.to_string();
623            }
624
625            // CREATE UNIQUE INDEX <name> → CREATE UNIQUE INDEX CONCURRENTLY <name>
626            if let Some(pos) = upper.find("CREATE UNIQUE INDEX") {
627                let after = pos + "CREATE UNIQUE INDEX".len();
628                let prefix = &line[..after];
629                let rest = line[after..].trim_start();
630                return format!("{prefix} CONCURRENTLY {rest}");
631            }
632
633            // CREATE INDEX <name> → CREATE INDEX CONCURRENTLY <name>
634            if let Some(pos) = upper.find("CREATE INDEX") {
635                let after = pos + "CREATE INDEX".len();
636                let prefix = &line[..after];
637                let rest = line[after..].trim_start();
638                return format!("{prefix} CONCURRENTLY {rest}");
639            }
640
641            line.to_string()
642        })
643        .collect::<Vec<_>>()
644        .join("\n")
645}
646
647// ─────────────────────────────────────────────
648// Formatting helpers
649// ─────────────────────────────────────────────
650
651/// Format a row count as a human-readable string (e.g. "187.2M").
652fn fmt_rows(n: u64) -> String {
653    if n >= 1_000_000_000 {
654        format!("{:.1}B", n as f64 / 1_000_000_000.0)
655    } else if n >= 1_000_000 {
656        format!("{:.1}M", n as f64 / 1_000_000.0)
657    } else if n >= 1_000 {
658        format!("{:.0}K", n as f64 / 1_000.0)
659    } else {
660        n.to_string()
661    }
662}
663
664/// Rough estimate: seconds an ACCESS EXCLUSIVE lock will be held, based on
665/// estimated row count at ~500k rows/second.
666fn estimate_lock_secs(rows: u64) -> u64 {
667    if rows < 500_000 {
668        1
669    } else {
670        rows / 500_000
671    }
672}
673
674/// Truncate a description string to `max` chars with an ellipsis.
675fn shorten_desc(s: &str, max: usize) -> &str {
676    if s.len() <= max {
677        s
678    } else {
679        &s[..max]
680    }
681}
682
683// ─────────────────────────────────────────────
684// Unit tests
685// ─────────────────────────────────────────────
686
687#[cfg(test)]
688mod tests {
689    use super::*;
690    use crate::parser;
691
692    #[test]
693    fn test_r01_triggers_for_non_concurrent_index() {
694        let sql = "CREATE INDEX idx_users_email ON users(email);";
695        let stmts = parser::parse(sql).expect("parse");
696        let fixes = suggest_fixes(&stmts, &HashMap::new());
697        assert!(fixes.iter().any(|f| f.rule_id == "R01"));
698        assert!(fixes.iter().any(|f| f.auto_fixable));
699    }
700
701    #[test]
702    fn test_r01_skipped_for_concurrent_index() {
703        let sql = "CREATE INDEX CONCURRENTLY idx_users_email ON users(email);";
704        let stmts = parser::parse(sql).expect("parse");
705        let fixes = suggest_fixes(&stmts, &HashMap::new());
706        assert!(fixes.iter().all(|f| f.rule_id != "R01"));
707    }
708
709    #[test]
710    fn test_r02_triggers_for_not_null_no_default() {
711        let sql = "ALTER TABLE users ADD COLUMN verified BOOLEAN NOT NULL;";
712        let stmts = parser::parse(sql).expect("parse");
713        let fixes = suggest_fixes(&stmts, &HashMap::new());
714        assert!(fixes.iter().any(|f| f.rule_id == "R02"));
715    }
716
717    #[test]
718    fn test_rewrite_concurrent() {
719        let sql = "CREATE INDEX idx_a ON t(col);";
720        let result = rewrite_index_concurrent(sql);
721        assert!(result.contains("CONCURRENTLY"), "got: {result}");
722    }
723
724    #[test]
725    fn test_rewrite_concurrent_idempotent() {
726        let sql = "CREATE INDEX CONCURRENTLY idx_a ON t(col);";
727        let result = rewrite_index_concurrent(sql);
728        // Should not double-insert CONCURRENTLY
729        let count = result.matches("CONCURRENTLY").count();
730        assert_eq!(count, 1, "got: {result}");
731    }
732}
schema_risk/recommendation.rs

schema_risk/
recommendation.rs
