feat(analyser): lock timout warning

Author: psteinroe

crates/pgt_analyse/src/analysed_file_context.rs

@@ -1,12 +1,16 @@
 pub struct AnalysedFileContext<'a> {
     pub stmts: &'a Vec<pgt_query::NodeEnum>,
-
     pos: usize,
+    transaction_state: TransactionState,
 }
 
 impl<'a> AnalysedFileContext<'a> {
     pub fn new(stmts: &'a Vec<pgt_query::NodeEnum>) -> Self {
-        Self { stmts, pos: 0 }
+        Self {
+            stmts,
+            pos: 0,
+            transaction_state: TransactionState::default(),
+        }
     }
 
     pub fn previous_stmts(&self) -> &[pgt_query::NodeEnum] {
@@ -17,7 +21,100 @@ impl<'a> AnalysedFileContext<'a> {
         self.stmts.len()
     }
 
+    /// Move to the next statement and update transaction state with the current statement
     pub fn next(&mut self) {
+        if self.pos < self.stmts.len() {
+            self.transaction_state
+                .update_from_stmt(&self.stmts[self.pos]);
+        }
         self.pos += 1;
     }
+
+    /// Get a reference to the transaction state
+    pub fn transaction_state(&self) -> &TransactionState {
+        &self.transaction_state
+    }
+}
+
+/// Represents the state of a transaction as we analyze statements in a file.
+///
+/// This tracks properties that span multiple statements, such as:
+/// - Whether a lock timeout has been set
+/// - Which objects have been created in this transaction
+#[derive(Debug, Default)]
+pub struct TransactionState {
+    /// Whether `SET lock_timeout` has been called in this transaction
+    lock_timeout_set: bool,
+    /// Objects (schema, name) created in this transaction
+    /// Schema names are normalized: empty string is stored as "public"
+    created_objects: Vec<(String, String)>,
+}
+
+impl TransactionState {
+    /// Returns true if a lock timeout has been set in this transaction
+    pub fn has_lock_timeout(&self) -> bool {
+        self.lock_timeout_set
+    }
+
+    /// Returns true if an object with the given schema and name was created in this transaction
+    pub fn has_created_object(&self, schema: &str, name: &str) -> bool {
+        // Normalize schema: treat empty string as "public"
+        let normalized_schema = if schema.is_empty() { "public" } else { schema };
+
+        self.created_objects
+            .iter()
+            .any(|(s, n)| normalized_schema.eq_ignore_ascii_case(s) && name.eq_ignore_ascii_case(n))
+    }
+
+    /// Record that an object was created, normalizing the schema name
+    fn add_created_object(&mut self, schema: String, name: String) {
+        // Normalize schema: store "public" instead of empty string
+        let normalized_schema = if schema.is_empty() {
+            "public".to_string()
+        } else {
+            schema
+        };
+        self.created_objects.push((normalized_schema, name));
+    }
+
+    /// Update transaction state based on a statement
+    pub(crate) fn update_from_stmt(&mut self, stmt: &pgt_query::NodeEnum) {
+        // Track SET lock_timeout
+        if let pgt_query::NodeEnum::VariableSetStmt(set_stmt) = stmt {
+            if set_stmt.name.eq_ignore_ascii_case("lock_timeout") {
+                self.lock_timeout_set = true;
+            }
+        }
+
+        // Track created objects
+        match stmt {
+            pgt_query::NodeEnum::CreateStmt(create_stmt) => {
+                if let Some(relation) = &create_stmt.relation {
+                    let schema = relation.schemaname.clone();
+                    let name = relation.relname.clone();
+                    self.add_created_object(schema, name);
+                }
+            }
+            pgt_query::NodeEnum::IndexStmt(index_stmt) => {
+                if !index_stmt.idxname.is_empty() {
+                    let schema = index_stmt
+                        .relation
+                        .as_ref()
+                        .map(|r| r.schemaname.clone())
+                        .unwrap_or_default();
+                    self.add_created_object(schema, index_stmt.idxname.clone());
+                }
+            }
+            pgt_query::NodeEnum::CreateTableAsStmt(ctas) => {
+                if let Some(into) = &ctas.into {
+                    if let Some(rel) = &into.rel {
+                        let schema = rel.schemaname.clone();
+                        let name = rel.relname.clone();
+                        self.add_created_object(schema, name);
+                    }
+                }
+            }
+            _ => {}
+        }
+    }
 }

crates/pgt_analyser/src/lint/safety.rs

@@ -18,6 +18,7 @@ pub mod changing_column_type;
 pub mod constraint_missing_not_valid;
 pub mod creating_enum;
 pub mod disallow_unique_constraint;
+pub mod lock_timeout_warning;
 pub mod multiple_alter_table;
 pub mod prefer_big_int;
 pub mod prefer_bigint_over_int;
@@ -32,4 +33,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 :: creating_enum :: CreatingEnum , 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 ,] } }
+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 :: creating_enum :: CreatingEnum , self :: disallow_unique_constraint :: DisallowUniqueConstraint , self :: lock_timeout_warning :: LockTimeoutWarning , 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/lock_timeout_warning.rs

@@ -0,0 +1,124 @@
+use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
+use pgt_console::markup;
+use pgt_diagnostics::Severity;
+
+declare_lint_rule! {
+    /// Taking a dangerous lock without setting a lock timeout can cause indefinite blocking.
+    ///
+    /// When a statement acquires a lock that would block common operations (like SELECT, INSERT, UPDATE, DELETE),
+    /// it can cause the database to become unresponsive if another transaction is holding a conflicting lock
+    /// while idle in transaction or active. This is particularly dangerous for:
+    ///
+    /// - ALTER TABLE statements (acquire ACCESS EXCLUSIVE lock)
+    /// - CREATE INDEX without CONCURRENTLY (acquires SHARE lock)
+    ///
+    /// Setting a lock timeout ensures that if the lock cannot be acquired within a reasonable time,
+    /// the statement will fail rather than blocking indefinitely.
+    ///
+    /// ## Examples
+    ///
+    /// ### Invalid
+    ///
+    /// ```sql,expect_diagnostic
+    /// ALTER TABLE users ADD COLUMN email TEXT;
+    /// ```
+    ///
+    /// ```sql,expect_diagnostic
+    /// CREATE INDEX users_email_idx ON users(email);
+    /// ```
+    ///
+    /// ### Valid
+    ///
+    /// ```sql
+    /// -- Use CONCURRENTLY to avoid taking dangerous locks
+    /// CREATE INDEX CONCURRENTLY users_email_idx ON users(email);
+    /// ```
+    ///
+    pub LockTimeoutWarning {
+        version: "next",
+        name: "lockTimeoutWarning",
+        severity: Severity::Error,
+        recommended: false,
+        sources: &[RuleSource::Eugene("E9")],
+    }
+}
+
+impl Rule for LockTimeoutWarning {
+    type Options = ();
+
+    fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
+        let mut diagnostics = Vec::new();
+
+        // Check if lock timeout has been set in the transaction
+        let tx_state = ctx.file_context().transaction_state();
+        if tx_state.has_lock_timeout() {
+            return diagnostics;
+        }
+
+        // Check if this statement takes a dangerous lock on an existing object
+        match ctx.stmt() {
+            // ALTER TABLE takes ACCESS EXCLUSIVE lock
+            pgt_query::NodeEnum::AlterTableStmt(stmt) => {
+                if let Some(relation) = &stmt.relation {
+                    let schema = if relation.schemaname.is_empty() {
+                        "public"
+                    } else {
+                        &relation.schemaname
+                    };
+                    let table = &relation.relname;
+
+                    // Only warn if the table wasn't created in this transaction
+                    if !tx_state.has_created_object(schema, table) {
+                        let full_name = format!("{}.{}", schema, table);
+                        diagnostics.push(
+                            RuleDiagnostic::new(
+                                rule_category!(),
+                                None,
+                                markup! {
+                                    "Statement takes ACCESS EXCLUSIVE lock on "<Emphasis>{full_name}</Emphasis>" without lock timeout set."
+                                },
+                            )
+                            .detail(None, "This can block all operations on the table indefinitely if another transaction holds a conflicting lock.")
+                            .note("Run 'SET LOCAL lock_timeout = '2s';' before this statement and retry the migration if it times out."),
+                        );
+                    }
+                }
+            }
+
+            // CREATE INDEX without CONCURRENTLY takes SHARE lock
+            pgt_query::NodeEnum::IndexStmt(stmt) => {
+                if !stmt.concurrent {
+                    if let Some(relation) = &stmt.relation {
+                        let schema = if relation.schemaname.is_empty() {
+                            "public"
+                        } else {
+                            &relation.schemaname
+                        };
+                        let table = &relation.relname;
+
+                        // Only warn if the table wasn't created in this transaction
+                        if !tx_state.has_created_object(schema, table) {
+                            let full_name = format!("{}.{}", schema, table);
+                            let index_name = &stmt.idxname;
+                            diagnostics.push(
+                                RuleDiagnostic::new(
+                                    rule_category!(),
+                                    None,
+                                    markup! {
+                                        "Statement takes SHARE lock on "<Emphasis>{full_name}</Emphasis>" while creating index "<Emphasis>{index_name}</Emphasis>" without lock timeout set."
+                                    },
+                                )
+                                .detail(None, "This blocks writes to the table indefinitely if another transaction holds a conflicting lock.")
+                                .note("Run 'SET LOCAL lock_timeout = '2s';' before this statement, or use CREATE INDEX CONCURRENTLY to avoid blocking writes."),
+                            );
+                        }
+                    }
+                }
+            }
+
+            _ => {}
+        }
+
+        diagnostics
+    }
+}

crates/pgt_analyser/src/options.rs

@@ -27,6 +27,8 @@ pub type ChangingColumnType =
 pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as pgt_analyse :: Rule > :: Options ;
 pub type CreatingEnum = <lint::safety::creating_enum::CreatingEnum as pgt_analyse::Rule>::Options;
 pub type DisallowUniqueConstraint = < lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint as pgt_analyse :: Rule > :: Options ;
+pub type LockTimeoutWarning =
+    <lint::safety::lock_timeout_warning::LockTimeoutWarning 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;

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

@@ -0,0 +1,3 @@
+-- expect_only_lint/safety/lockTimeoutWarning
+-- ALTER TABLE without lock timeout should trigger the rule
+ALTER TABLE authors ADD COLUMN email TEXT;

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

@@ -0,0 +1,20 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- expect_only_lint/safety/lockTimeoutWarning
+-- ALTER TABLE without lock timeout should trigger the rule
+ALTER TABLE authors ADD COLUMN email TEXT;
+```
+
+# Diagnostics
+lint/safety/lockTimeoutWarning ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Statement takes ACCESS EXCLUSIVE lock on public.authors without lock timeout set.
+  
+  i This can block all operations on the table indefinitely if another transaction holds a conflicting lock.
+  
+  i Run 'SET LOCAL lock_timeout = '2s';' before this statement and retry the migration if it times out.

crates/pgt_analyser/tests/specs/safety/lockTimeoutWarning/create_index_without_timeout.sql

@@ -0,0 +1,3 @@
+-- expect_only_lint/safety/lockTimeoutWarning
+-- CREATE INDEX without CONCURRENTLY or lock timeout should trigger the rule
+CREATE INDEX books_title_idx ON books(title);

crates/pgt_analyser/tests/specs/safety/lockTimeoutWarning/create_index_without_timeout.sql.snap

@@ -0,0 +1,21 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- expect_only_lint/safety/lockTimeoutWarning
+-- CREATE INDEX without CONCURRENTLY or lock timeout should trigger the rule
+CREATE INDEX books_title_idx ON books(title);
+
+```
+
+# Diagnostics
+lint/safety/lockTimeoutWarning ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Statement takes SHARE lock on public.books while creating index books_title_idx without lock timeout set.
+  
+  i This blocks writes to the table indefinitely if another transaction holds a conflicting lock.
+  
+  i Run 'SET LOCAL lock_timeout = '2s';' before this statement, or use CREATE INDEX CONCURRENTLY to avoid blocking writes.

crates/pgt_analyser/tests/specs/safety/lockTimeoutWarning/multiple_statements_without_timeout.sql

@@ -0,0 +1,6 @@
+-- Both statements should trigger the rule
+-- expect_lint/safety/lockTimeoutWarning
+CREATE INDEX orders_user_idx ON orders(user_id);
+
+-- expect_lint/safety/lockTimeoutWarning
+ALTER TABLE orders ADD COLUMN total DECIMAL(10, 2);

crates/pgt_analyser/tests/specs/safety/lockTimeoutWarning/multiple_statements_without_timeout.sql.snap

@@ -0,0 +1,34 @@
+---
+source: crates/pgt_analyser/tests/rules_tests.rs
+expression: snapshot
+snapshot_kind: text
+---
+# Input
+```
+-- Both statements should trigger the rule
+-- expect_lint/safety/lockTimeoutWarning
+CREATE INDEX orders_user_idx ON orders(user_id);
+
+-- expect_lint/safety/lockTimeoutWarning
+ALTER TABLE orders ADD COLUMN total DECIMAL(10, 2);
+
+```
+
+# Diagnostics
+lint/safety/lockTimeoutWarning ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Statement takes SHARE lock on public.orders while creating index orders_user_idx without lock timeout set.
+  
+  i This blocks writes to the table indefinitely if another transaction holds a conflicting lock.
+  
+  i Run 'SET LOCAL lock_timeout = '2s';' before this statement, or use CREATE INDEX CONCURRENTLY to avoid blocking writes.
+  
+
+
+lint/safety/lockTimeoutWarning ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
+
+  × Statement takes ACCESS EXCLUSIVE lock on public.orders without lock timeout set.
+  
+  i This can block all operations on the table indefinitely if another transaction holds a conflicting lock.
+  
+  i Run 'SET LOCAL lock_timeout = '2s';' before this statement and retry the migration if it times out.