feat(analyser): mutiple alter table

Author: psteinroe

crates/pgt_analyser/src/lint/safety.rs

@@ -17,6 +17,7 @@ pub mod ban_truncate_cascade;
 pub mod changing_column_type;
 pub mod constraint_missing_not_valid;
 pub mod disallow_unique_constraint;
+pub mod multiple_alter_table;
 pub mod prefer_big_int;
 pub mod prefer_bigint_over_int;
 pub mod prefer_bigint_over_smallint;
@@ -30,4 +31,4 @@ pub mod renaming_table;
 pub mod require_concurrent_index_creation;
 pub mod require_concurrent_index_deletion;
 pub mod transaction_nesting;
-declare_lint_group! { pub Safety { name : "safety" , rules : [self :: add_serial_column :: AddSerialColumn , self :: adding_field_with_default :: AddingFieldWithDefault , self :: adding_foreign_key_constraint :: AddingForeignKeyConstraint , self :: adding_not_null_field :: AddingNotNullField , self :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint , self :: adding_required_field :: AddingRequiredField , self :: ban_char_field :: BanCharField , self :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_database :: BanDropDatabase , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable , self :: ban_truncate_cascade :: BanTruncateCascade , self :: changing_column_type :: ChangingColumnType , self :: constraint_missing_not_valid :: ConstraintMissingNotValid , self :: disallow_unique_constraint :: DisallowUniqueConstraint , self :: prefer_big_int :: PreferBigInt , self :: prefer_bigint_over_int :: PreferBigintOverInt , self :: prefer_bigint_over_smallint :: PreferBigintOverSmallint , self :: prefer_identity :: PreferIdentity , self :: prefer_jsonb :: PreferJsonb , self :: prefer_robust_stmts :: PreferRobustStmts , self :: prefer_text_field :: PreferTextField , self :: prefer_timestamptz :: PreferTimestamptz , self :: renaming_column :: RenamingColumn , self :: renaming_table :: RenamingTable , self :: require_concurrent_index_creation :: RequireConcurrentIndexCreation , self :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion , self :: transaction_nesting :: TransactionNesting ,] } }
+declare_lint_group! { pub Safety { name : "safety" , rules : [self :: add_serial_column :: AddSerialColumn , self :: adding_field_with_default :: AddingFieldWithDefault , self :: adding_foreign_key_constraint :: AddingForeignKeyConstraint , self :: adding_not_null_field :: AddingNotNullField , self :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint , self :: adding_required_field :: AddingRequiredField , self :: ban_char_field :: BanCharField , self :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_database :: BanDropDatabase , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_table :: BanDropTable , self :: ban_truncate_cascade :: BanTruncateCascade , self :: changing_column_type :: ChangingColumnType , self :: constraint_missing_not_valid :: ConstraintMissingNotValid , self :: disallow_unique_constraint :: DisallowUniqueConstraint , self :: multiple_alter_table :: MultipleAlterTable , self :: prefer_big_int :: PreferBigInt , self :: prefer_bigint_over_int :: PreferBigintOverInt , self :: prefer_bigint_over_smallint :: PreferBigintOverSmallint , self :: prefer_identity :: PreferIdentity , self :: prefer_jsonb :: PreferJsonb , self :: prefer_robust_stmts :: PreferRobustStmts , self :: prefer_text_field :: PreferTextField , self :: prefer_timestamptz :: PreferTimestamptz , self :: renaming_column :: RenamingColumn , self :: renaming_table :: RenamingTable , self :: require_concurrent_index_creation :: RequireConcurrentIndexCreation , self :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion , self :: transaction_nesting :: TransactionNesting ,] } }

crates/pgt_analyser/src/lint/safety/multiple_alter_table.rs

@@ -0,0 +1,115 @@
+use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
+use pgt_console::markup;
+use pgt_diagnostics::Severity;
+
+declare_lint_rule! {
+    /// Multiple ALTER TABLE statements on the same table should be combined into a single statement.
+    ///
+    /// When you run multiple ALTER TABLE statements on the same table, PostgreSQL must scan and potentially
+    /// rewrite the table multiple times. Each ALTER TABLE command requires acquiring locks and performing
+    /// table operations that can be expensive, especially on large tables.
+    ///
+    /// Combining multiple ALTER TABLE operations into a single statement with comma-separated actions
+    /// allows PostgreSQL to scan and modify the table only once, improving performance and reducing
+    /// the time locks are held.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```sql,expect_diagnostic
+    /// ALTER TABLE authors ALTER COLUMN name SET NOT NULL;
+    /// ALTER TABLE authors ALTER COLUMN email SET NOT NULL;
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```sql
+    /// ALTER TABLE authors
+    ///   ALTER COLUMN name SET NOT NULL,
+    ///   ALTER COLUMN email SET NOT NULL;
+    /// ```
+    ///
+    pub MultipleAlterTable {
+        version: "next",
+        name: "multipleAlterTable",
+        severity: Severity::Warning,
+        recommended: true,
+        sources: &[RuleSource::Eugene("W12")],
+    }
+}
+
+impl Rule for MultipleAlterTable {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = Vec::new();
+
+        // Check if current statement is ALTER TABLE
+        if let pgt_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() {
+            if let Some(relation) = &stmt.relation {
+                let current_schema = &relation.schemaname;
+                let current_table = &relation.relname;
+
+                // Check previous statements for ALTER TABLE on the same table
+                let file_ctx = ctx.file_context();
+
+                // Normalize schema name: treat empty string as "public"
+                let current_schema_normalized = if current_schema.is_empty() {
+                    "public"
+                } else {
+                    current_schema.as_str()
+                };
+
+                let has_previous_alter =
+                    file_ctx
+                        .previous_stmts()
+                        .iter()
+                        .any(|prev_stmt| match prev_stmt {
+                            pgt_query::NodeEnum::AlterTableStmt(prev_alter) => {
+                                if let Some(prev_relation) = &prev_alter.relation {
+                                    let prev_schema_normalized =
+                                        if prev_relation.schemaname.is_empty() {
+                                            "public"
+                                        } else {
+                                            prev_relation.schemaname.as_str()
+                                        };
+
+                                    // Match if same table and schema (treating empty schema as "public")
+                                    prev_relation.relname == *current_table
+                                        && prev_schema_normalized == current_schema_normalized
+                                } else {
+                                    false
+                                }
+                            }
+                            _ => false,
+                        });
+
+                if has_previous_alter {
+                    let schema_display = if current_schema.is_empty() {
+                        "public"
+                    } else {
+                        current_schema
+                    };
+
+                    diagnostics.push(
+                        RuleDiagnostic::new(
+                            rule_category!(),
+                            None,
+                            markup! {
+                                "Multiple "<Emphasis>"ALTER TABLE"</Emphasis>" statements found for table "<Emphasis>{schema_display}"."{{current_table}}</Emphasis>"."
+                            },
+                        )
+                        .detail(
+                            None,
+                            "Multiple ALTER TABLE statements on the same table require scanning and potentially rewriting the table multiple times.",
+                        )
+                        .note("Combine the ALTER TABLE statements into a single statement with comma-separated actions to scan the table only once."),
+                    );
+                }
+            }
+        }
+
+        diagnostics
+    }
+}

crates/pgt_analyser/src/options.rs

@@ -26,6 +26,8 @@ pub type ChangingColumnType =
     <lint::safety::changing_column_type::ChangingColumnType as pgt_analyse::Rule>::Options;
 pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as pgt_analyse :: Rule > :: Options ;
 pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as pgt_analyse :: Rule > :: Options ;
+pub type MultipleAlterTable =
+    <lint::safety::multiple_alter_table::MultipleAlterTable as pgt_analyse::Rule>::Options;
 pub type PreferBigInt = <lint::safety::prefer_big_int::PreferBigInt as pgt_analyse::Rule>::Options;
 pub type PreferBigintOverInt =
     <lint::safety::prefer_bigint_over_int::PreferBigintOverInt as pgt_analyse::Rule>::Options;

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/basic.sql

@@ -0,0 +1,4 @@
+-- expect_lint/safety/multipleAlterTable
+-- Test multiple ALTER TABLE statements on the same table
+ALTER TABLE authors ALTER COLUMN name SET NOT NULL;
+ALTER TABLE authors ALTER COLUMN email SET NOT NULL;

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/basic.sql.snap

@@ -0,0 +1,22 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- expect_lint/safety/multipleAlterTable
+-- Test multiple ALTER TABLE statements on the same table
+ALTER TABLE authors ALTER COLUMN name SET NOT NULL;
+ALTER TABLE authors ALTER COLUMN email SET NOT NULL;
+
+```
+
+# Diagnostics
+lint/safety/multipleAlterTable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Multiple ALTER TABLE statements found for table public.authors.
+  
+  i Multiple ALTER TABLE statements on the same table require scanning and potentially rewriting the table multiple times.
+  
+  i Combine the ALTER TABLE statements into a single statement with comma-separated actions to scan the table only once.

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/mixed_schema_notation.sql

@@ -0,0 +1,4 @@
+-- expect_lint/safety/multipleAlterTable
+-- Test mixing implicit and explicit public schema (should match)
+ALTER TABLE authors ADD COLUMN name text;
+ALTER TABLE public.authors ADD COLUMN email text;

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/mixed_schema_notation.sql.snap

@@ -0,0 +1,22 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- expect_lint/safety/multipleAlterTable
+-- Test mixing implicit and explicit public schema (should match)
+ALTER TABLE authors ADD COLUMN name text;
+ALTER TABLE public.authors ADD COLUMN email text;
+
+```
+
+# Diagnostics
+lint/safety/multipleAlterTable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Multiple ALTER TABLE statements found for table public.authors.
+  
+  i Multiple ALTER TABLE statements on the same table require scanning and potentially rewriting the table multiple times.
+  
+  i Combine the ALTER TABLE statements into a single statement with comma-separated actions to scan the table only once.

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/three_alters.sql

@@ -0,0 +1,5 @@
+-- expect_lint/safety/multipleAlterTable
+-- Test ALTER TABLE after other statements
+CREATE TABLE products (id serial PRIMARY KEY);
+ALTER TABLE products ADD COLUMN description text;
+ALTER TABLE products ADD COLUMN price numeric;

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/three_alters.sql.snap

@@ -0,0 +1,23 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- expect_lint/safety/multipleAlterTable
+-- Test ALTER TABLE after other statements
+CREATE TABLE products (id serial PRIMARY KEY);
+ALTER TABLE products ADD COLUMN description text;
+ALTER TABLE products ADD COLUMN price numeric;
+
+```
+
+# Diagnostics
+lint/safety/multipleAlterTable ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Multiple ALTER TABLE statements found for table public.products.
+  
+  i Multiple ALTER TABLE statements on the same table require scanning and potentially rewriting the table multiple times.
+  
+  i Combine the ALTER TABLE statements into a single statement with comma-separated actions to scan the table only once.

crates/pgt_analyser/tests/specs/safety/multipleAlterTable/valid_combined.sql

@@ -0,0 +1,5 @@
+-- Test single ALTER TABLE with multiple actions (should be safe)
+-- expect_no_diagnostics
+ALTER TABLE authors
+  ALTER COLUMN name SET NOT NULL,
+  ALTER COLUMN email SET NOT NULL;