linter: new rule: require-concurrent-partition-detach

crates/squawk_linter/src/lib.rs

@@ -54,6 +54,7 @@ use rules::renaming_column;
 use rules::renaming_table;
 use rules::require_concurrent_index_creation;
 use rules::require_concurrent_index_deletion;
+use rules::require_concurrent_partition_detach;
 use rules::require_enum_value_ordering;
 use rules::require_table_schema;
 use rules::require_timeout_settings;
@@ -96,6 +97,7 @@ pub enum Rule {
     RequireEnumValueOrdering,
     RequireTableSchema,
     IdentifierTooLong,
+    RequireConcurrentPartitionDetach,
     // xtask:new-rule:error-name
 }
 
@@ -149,6 +151,7 @@ impl TryFrom<&str> for Rule {
             "require-enum-value-ordering" => Ok(Rule::RequireEnumValueOrdering),
             "require-table-schema" => Ok(Rule::RequireTableSchema),
             "identifier-too-long" => Ok(Rule::IdentifierTooLong),
+            "require-concurrent-partition-detach" => Ok(Rule::RequireConcurrentPartitionDetach),
             // xtask:new-rule:str-name
             _ => Err(format!("Unknown violation name: {s}")),
         }
@@ -214,6 +217,7 @@ impl fmt::Display for Rule {
             Rule::RequireEnumValueOrdering => "require-enum-value-ordering",
             Rule::RequireTableSchema => "require-table-schema",
             Rule::IdentifierTooLong => "identifier-too-long",
+            Rule::RequireConcurrentPartitionDetach => "require-concurrent-partition-detach",
             // xtask:new-rule:variant-to-name
         };
         write!(f, "{val}")
@@ -450,6 +454,9 @@ impl Linter {
         if self.rules.contains(&Rule::IdentifierTooLong) {
             identifier_too_long(self, file);
         }
+        if self.rules.contains(&Rule::RequireConcurrentPartitionDetach) {
+            require_concurrent_partition_detach(self, file);
+        }
         // xtask:new-rule:rule-call
 
         // locate any ignores in the file

crates/squawk_linter/src/rules/mod.rs

@@ -27,6 +27,7 @@ pub(crate) mod renaming_column;
 pub(crate) mod renaming_table;
 pub(crate) mod require_concurrent_index_creation;
 pub(crate) mod require_concurrent_index_deletion;
+pub(crate) mod require_concurrent_partition_detach;
 pub(crate) mod require_enum_value_ordering;
 pub(crate) mod require_table_schema;
 pub(crate) mod require_timeout_settings;
@@ -62,6 +63,7 @@ pub(crate) use renaming_column::renaming_column;
 pub(crate) use renaming_table::renaming_table;
 pub(crate) use require_concurrent_index_creation::require_concurrent_index_creation;
 pub(crate) use require_concurrent_index_deletion::require_concurrent_index_deletion;
+pub(crate) use require_concurrent_partition_detach::require_concurrent_partition_detach;
 pub(crate) use require_enum_value_ordering::require_enum_value_ordering;
 pub(crate) use require_table_schema::require_table_schema;
 pub(crate) use require_timeout_settings::require_timeout_settings;

crates/squawk_linter/src/rules/require_concurrent_partition_detach.rs

@@ -0,0 +1,87 @@
+use squawk_syntax::{
+    Parse, SourceFile,
+    ast::{self, AstNode},
+};
+
+use crate::{Edit, Fix, Linter, Rule, Violation};
+
+fn concurrently_fix(detach_partition: &ast::DetachPartition) -> Option<Fix> {
+    let path = detach_partition.path()?;
+    let at = path.syntax().text_range().end();
+    let edit = Edit::insert(" concurrently", at);
+    Some(Fix::new("Add `concurrently`", vec![edit]))
+}
+
+pub(crate) fn require_concurrent_partition_detach(ctx: &mut Linter, parse: &Parse<SourceFile>) {
+    for stmt in parse.tree().stmts() {
+        if let ast::Stmt::AlterTable(alter_table) = stmt {
+            for action in alter_table.actions() {
+                if let ast::AlterTableAction::DetachPartition(detach_partition) = action {
+                    if detach_partition.concurrently_token().is_none()
+                        && detach_partition.finalize_token().is_none()
+                    {
+                        let fix = concurrently_fix(&detach_partition);
+                        ctx.report(
+                            Violation::for_node(
+                                Rule::RequireConcurrentPartitionDetach,
+                                "Detaching a partition requires an `ACCESS EXCLUSIVE` lock, which prevents reads and writes to the table.".into(),
+                                detach_partition.syntax(),
+                            )
+                            .help("Detach the partition `CONCURRENTLY`.")
+                            .fix(fix),
+                        );
+                    }
+                }
+            }
+        }
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use insta::assert_snapshot;
+
+    use crate::{
+        Rule,
+        test_utils::{fix_sql, lint_errors, lint_ok},
+    };
+
+    fn fix(sql: &str) -> String {
+        fix_sql(sql, Rule::RequireConcurrentPartitionDetach)
+    }
+
+    #[test]
+    fn detach_partition_missing_concurrently_err() {
+        let sql = "ALTER TABLE t DETACH PARTITION p;";
+        assert_snapshot!(lint_errors(sql, Rule::RequireConcurrentPartitionDetach), @"
+        warning[require-concurrent-partition-detach]: Detaching a partition requires an `ACCESS EXCLUSIVE` lock, which prevents reads and writes to the table.
+          ╭▸ 
+        1 │ ALTER TABLE t DETACH PARTITION p;
+          │               ━━━━━━━━━━━━━━━━━━
+          │
+          ├ help: Detach the partition `CONCURRENTLY`.
+          ╭╴
+        1 │ ALTER TABLE t DETACH PARTITION p concurrently;
+          ╰╴                                 ++++++++++++
+        ");
+    }
+
+    #[test]
+    fn detach_partition_concurrently_ok() {
+        let sql = "ALTER TABLE t DETACH PARTITION p CONCURRENTLY;";
+        lint_ok(sql, Rule::RequireConcurrentPartitionDetach);
+    }
+
+    #[test]
+    fn detach_partition_finalize_ok() {
+        let sql = "ALTER TABLE t DETACH PARTITION p FINALIZE;";
+        lint_ok(sql, Rule::RequireConcurrentPartitionDetach);
+    }
+
+    #[test]
+    fn fix_add_concurrently() {
+        let sql = "ALTER TABLE t DETACH PARTITION p;";
+        let result = fix(sql);
+        assert_snapshot!(result, @"ALTER TABLE t DETACH PARTITION p concurrently;");
+    }
+}

crates/xtask/src/new_rule.rs

@@ -17,8 +17,7 @@ use squawk_syntax::{{
 use crate::{{Linter, Violation, Rule}};
 
 pub(crate) fn {rule_name_snake}(ctx: &mut Linter, parse: &Parse<SourceFile>) {{
-    let file = parse.tree();
-    for stmt in file.stmts() {{
+    for stmt in parse.tree().stmts() {{
         match stmt {{
             // TODO: update to the stmt you want to check
             ast::Stmt::CreateTable(create_table) => {{

docs/docs/require-concurrent-partition-detach.md

@@ -0,0 +1,31 @@
+---
+id: require-concurrent-partition-detach
+title: require-concurrent-partition-detach
+---
+
+## problem
+
+Detaching a partition via `ALTER TABLE t DETACH PARTITION y;` requires an `ACCESS EXCLUSIVE` lock, which prevents reads and writes to the table.
+
+## solution
+
+Ensure all partition detaches use the `CONCURRENTLY` option.
+
+Instead of:
+
+```sql
+-- blocks reads and writes
+ALTER TABLE t DETACH PARTITION p;
+```
+
+Use:
+
+```sql
+-- allows reads and writes while detaching
+ALTER TABLE t DETACH PARTITION p CONCURRENTLY;
+```
+
+## links
+
+- <https://www.postgresql.org/docs/current/ddl-partitioning.html>
+- <https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DETACH-PARTITION>

docs/sidebars.js

@@ -37,6 +37,7 @@ module.exports = {
       "require-enum-value-ordering",
       "require-table-schema",
       "identifier-too-long",
+      "require-concurrent-partition-detach",
       // xtask:new-rule:error-name
     ],
   },

docs/src/pages/index.js

@@ -215,7 +215,12 @@ const rules = [
   {
     name: "identifier-too-long",
     tags: ["schema"],
-    description: "Warn about implicit truncation for identifiers that are too long.",
+    description: "Prevent implicit truncation for identifiers that are too long.",
+  },
+  {
+    name: "require-concurrent-partition-detach",
+    tags: ["schema", "locking"],
+    description: "Prevent blocking reads/writes to table while partition is detached.",
   },
   // xtask:new-rule:rule-doc-meta
 ]