Skip to content

Commit 3edca2d

Browse files
committed
feat(analyser): access exclusive
1 parent 5939553 commit 3edca2d

27 files changed

+392
-5
lines changed

crates/pgt_analyse/src/analysed_file_context.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,17 @@ impl<'a> AnalysedFileContext<'a> {
4141
/// This tracks properties that span multiple statements, such as:
4242
/// - Whether a lock timeout has been set
4343
/// - Which objects have been created in this transaction
44+
/// - Whether an ACCESS EXCLUSIVE lock is currently being held
4445
#[derive(Debug, Default)]
4546
pub struct TransactionState {
4647
/// Whether `SET lock_timeout` has been called in this transaction
4748
lock_timeout_set: bool,
4849
/// Objects (schema, name) created in this transaction
4950
/// Schema names are normalized: empty string is stored as "public"
5051
created_objects: Vec<(String, String)>,
52+
/// Whether an ACCESS EXCLUSIVE lock is currently being held
53+
/// This is set when an ALTER TABLE is executed on an existing table
54+
holding_access_exclusive: bool,
5155
}
5256

5357
impl TransactionState {
@@ -66,6 +70,11 @@ impl TransactionState {
6670
.any(|(s, n)| normalized_schema.eq_ignore_ascii_case(s) && name.eq_ignore_ascii_case(n))
6771
}
6872

73+
/// Returns true if the transaction is currently holding an ACCESS EXCLUSIVE lock
74+
pub fn is_holding_access_exclusive(&self) -> bool {
75+
self.holding_access_exclusive
76+
}
77+
6978
/// Record that an object was created, normalizing the schema name
7079
fn add_created_object(&mut self, schema: String, name: String) {
7180
// Normalize schema: store "public" instead of empty string
@@ -116,5 +125,18 @@ impl TransactionState {
116125
}
117126
_ => {}
118127
}
128+
129+
// Track ACCESS EXCLUSIVE lock acquisition
130+
// ALTER TABLE on an existing table acquires ACCESS EXCLUSIVE lock
131+
if let pgt_query::NodeEnum::AlterTableStmt(alter_stmt) = stmt {
132+
if let Some(relation) = &alter_stmt.relation {
133+
let schema = &relation.schemaname;
134+
let name = &relation.relname;
135+
// Only set the flag if altering an existing table (not one created in this transaction)
136+
if !self.has_created_object(schema, name) {
137+
self.holding_access_exclusive = true;
138+
}
139+
}
140+
}
119141
}
120142
}

crates/pgt_analyser/src/lint/safety.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,6 @@ pub mod renaming_column;
3232
pub mod renaming_table;
3333
pub mod require_concurrent_index_creation;
3434
pub mod require_concurrent_index_deletion;
35+
pub mod running_statement_while_holding_access_exclusive;
3536
pub mod transaction_nesting;
36-
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 ,] } }
37+
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 :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive , self :: transaction_nesting :: TransactionNesting ,] } }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ declare_lint_rule! {
3838
version: "next",
3939
name: "lockTimeoutWarning",
4040
severity: Severity::Error,
41-
recommended: false,
41+
recommended: true,
4242
sources: &[RuleSource::Eugene("E9")],
4343
}
4444
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
use pgt_analyse::{Rule, RuleDiagnostic, RuleSource, context::RuleContext, declare_lint_rule};
2+
use pgt_console::markup;
3+
use pgt_diagnostics::Severity;
4+
5+
declare_lint_rule! {
6+
/// Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access.
7+
///
8+
/// When a transaction acquires an ACCESS EXCLUSIVE lock (e.g., via ALTER TABLE), it blocks all other
9+
/// operations on that table, including reads. Running additional statements in the same transaction
10+
/// extends the duration the lock is held, potentially blocking all database access to that table.
11+
///
12+
/// This is particularly problematic because:
13+
/// - The lock blocks all SELECT, INSERT, UPDATE, DELETE operations
14+
/// - The lock is held for the entire duration of all subsequent statements
15+
/// - Even simple queries like SELECT COUNT(*) can significantly extend lock time
16+
///
17+
/// To minimize blocking, run the ALTER TABLE in its own transaction and execute
18+
/// other operations in separate transactions.
19+
///
20+
/// ## Examples
21+
///
22+
/// ### Invalid
23+
///
24+
/// ```sql,expect_diagnostic
25+
/// ALTER TABLE authors ADD COLUMN email TEXT;
26+
/// SELECT COUNT(*) FROM authors;
27+
/// ```
28+
///
29+
/// ### Valid
30+
///
31+
/// ```sql
32+
/// -- Run ALTER TABLE alone, other queries in separate transactions
33+
/// ALTER TABLE authors ADD COLUMN email TEXT;
34+
/// ```
35+
///
36+
pub RunningStatementWhileHoldingAccessExclusive {
37+
version: "next",
38+
name: "runningStatementWhileHoldingAccessExclusive",
39+
severity: Severity::Warning,
40+
recommended: true,
41+
sources: &[RuleSource::Eugene("E4")],
42+
}
43+
}
44+
45+
impl Rule for RunningStatementWhileHoldingAccessExclusive {
46+
type Options = ();
47+
48+
fn run(ctx: &RuleContext<Self>) -> Vec<RuleDiagnostic> {
49+
let mut diagnostics = Vec::new();
50+
51+
// Check if we're currently holding an ACCESS EXCLUSIVE lock
52+
let tx_state = ctx.file_context().transaction_state();
53+
if tx_state.is_holding_access_exclusive() {
54+
diagnostics.push(
55+
RuleDiagnostic::new(
56+
rule_category!(),
57+
None,
58+
markup! {
59+
"Running statement while holding ACCESS EXCLUSIVE lock."
60+
},
61+
)
62+
.detail(
63+
None,
64+
"This blocks all access to the table for the duration of this statement.",
65+
)
66+
.note("Run this statement in a separate transaction to minimize lock duration."),
67+
);
68+
}
69+
70+
diagnostics
71+
}
72+
}

crates/pgt_analyser/src/options.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,6 @@ pub type RenamingTable =
5050
<lint::safety::renaming_table::RenamingTable as pgt_analyse::Rule>::Options;
5151
pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as pgt_analyse :: Rule > :: Options ;
5252
pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as pgt_analyse :: Rule > :: Options ;
53+
pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as pgt_analyse :: Rule > :: Options ;
5354
pub type TransactionNesting =
5455
<lint::safety::transaction_nesting::TransactionNesting as pgt_analyse::Rule>::Options;
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- expect_only_lint/safety/runningStatementWhileHoldingAccessExclusive
2+
-- Running SELECT after ALTER TABLE should trigger the rule
3+
ALTER TABLE authors ADD COLUMN email TEXT NOT NULL;
4+
SELECT COUNT(*) FROM authors;
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
source: crates/pgt_analyser/tests/rules_tests.rs
3+
expression: snapshot
4+
snapshot_kind: text
5+
---
6+
# Input
7+
```
8+
-- expect_only_lint/safety/runningStatementWhileHoldingAccessExclusive
9+
-- Running SELECT after ALTER TABLE should trigger the rule
10+
ALTER TABLE authors ADD COLUMN email TEXT NOT NULL;
11+
SELECT COUNT(*) FROM authors;
12+
```
13+
14+
# Diagnostics
15+
lint/safety/runningStatementWhileHoldingAccessExclusive ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
16+
17+
× Running statement while holding ACCESS EXCLUSIVE lock.
18+
19+
i This blocks all access to the table for the duration of this statement.
20+
21+
i Run this statement in a separate transaction to minimize lock duration.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- expect_only_lint/safety/runningStatementWhileHoldingAccessExclusive
2+
-- CREATE INDEX after ALTER TABLE should trigger
3+
ALTER TABLE orders ADD COLUMN total DECIMAL(10, 2);
4+
CREATE INDEX orders_total_idx ON orders(total);
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
source: crates/pgt_analyser/tests/rules_tests.rs
3+
expression: snapshot
4+
snapshot_kind: text
5+
---
6+
# Input
7+
```
8+
-- expect_only_lint/safety/runningStatementWhileHoldingAccessExclusive
9+
-- CREATE INDEX after ALTER TABLE should trigger
10+
ALTER TABLE orders ADD COLUMN total DECIMAL(10, 2);
11+
CREATE INDEX orders_total_idx ON orders(total);
12+
13+
```
14+
15+
# Diagnostics
16+
lint/safety/runningStatementWhileHoldingAccessExclusive ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
17+
18+
× Running statement while holding ACCESS EXCLUSIVE lock.
19+
20+
i This blocks all access to the table for the duration of this statement.
21+
22+
i Run this statement in a separate transaction to minimize lock duration.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-- expect_only_lint/safety/runningStatementWhileHoldingAccessExclusive
2+
-- INSERT after ALTER TABLE should trigger
3+
ALTER TABLE books ADD COLUMN isbn TEXT;
4+
INSERT INTO books (title, isbn) VALUES ('Database Systems', '978-0-1234567-8-9');

0 commit comments

Comments
 (0)