diff --git a/crates/squawk_linter/src/lib.rs b/crates/squawk_linter/src/lib.rs index f321bc94..7dca6d6f 100644 --- a/crates/squawk_linter/src/lib.rs +++ b/crates/squawk_linter/src/lib.rs @@ -55,6 +55,7 @@ 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_concurrent_reindex; use rules::require_enum_value_ordering; use rules::require_table_schema; use rules::require_timeout_settings; @@ -98,6 +99,7 @@ pub enum Rule { RequireTableSchema, IdentifierTooLong, RequireConcurrentPartitionDetach, + RequireConcurrentReindex, // xtask:new-rule:error-name } @@ -152,6 +154,7 @@ impl TryFrom<&str> for Rule { "require-table-schema" => Ok(Rule::RequireTableSchema), "identifier-too-long" => Ok(Rule::IdentifierTooLong), "require-concurrent-partition-detach" => Ok(Rule::RequireConcurrentPartitionDetach), + "require-concurrent-reindex" => Ok(Rule::RequireConcurrentReindex), // xtask:new-rule:str-name _ => Err(format!("Unknown violation name: {s}")), } @@ -218,6 +221,7 @@ impl fmt::Display for Rule { Rule::RequireTableSchema => "require-table-schema", Rule::IdentifierTooLong => "identifier-too-long", Rule::RequireConcurrentPartitionDetach => "require-concurrent-partition-detach", + Rule::RequireConcurrentReindex => "require-concurrent-reindex", // xtask:new-rule:variant-to-name }; write!(f, "{val}") @@ -457,6 +461,9 @@ impl Linter { if self.rules.contains(&Rule::RequireConcurrentPartitionDetach) { require_concurrent_partition_detach(self, file); } + if self.rules.contains(&Rule::RequireConcurrentReindex) { + require_concurrent_reindex(self, file); + } // xtask:new-rule:rule-call // locate any ignores in the file diff --git a/crates/squawk_linter/src/rules/mod.rs b/crates/squawk_linter/src/rules/mod.rs index c563d28a..922c3bc2 100644 --- a/crates/squawk_linter/src/rules/mod.rs +++ b/crates/squawk_linter/src/rules/mod.rs @@ -28,6 +28,7 @@ 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_concurrent_reindex; pub(crate) mod require_enum_value_ordering; pub(crate) mod require_table_schema; pub(crate) mod require_timeout_settings; @@ -64,6 +65,7 @@ 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_concurrent_reindex::require_concurrent_reindex; 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; diff --git a/crates/squawk_linter/src/rules/require_concurrent_reindex.rs b/crates/squawk_linter/src/rules/require_concurrent_reindex.rs new file mode 100644 index 00000000..cb35eafe --- /dev/null +++ b/crates/squawk_linter/src/rules/require_concurrent_reindex.rs @@ -0,0 +1,155 @@ +use squawk_syntax::{ + Parse, SourceFile, + ast::{self, AstNode}, +}; + +use crate::{Edit, Fix, Linter, Rule, Violation}; + +fn concurrently_fix(reindex: &ast::Reindex) -> Option { + let kw_token = reindex + .table_token() + .or_else(|| reindex.index_token()) + .or_else(|| reindex.schema_token()) + .or_else(|| reindex.database_token())?; + let at = kw_token.text_range().end(); + let edit = Edit::insert(" concurrently", at); + Some(Fix::new("Add `concurrently`", vec![edit])) +} + +pub(crate) fn require_concurrent_reindex(ctx: &mut Linter, parse: &Parse) { + for stmt in parse.tree().stmts() { + if let ast::Stmt::Reindex(reindex) = stmt { + // REINDEX SYSTEM does not support CONCURRENTLY + if reindex.system_token().is_some() { + continue; + } + if reindex.concurrently_token().is_none() { + let fix = concurrently_fix(&reindex); + ctx.report( + Violation::for_node( + Rule::RequireConcurrentReindex, + "Reindexing a table or index without `concurrently` blocks reads and writes." + .into(), + reindex.syntax(), + ) + .help("Use `concurrently` to avoid blocking reads and writes.") + .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::RequireConcurrentReindex) + } + + #[test] + fn reindex_table_no_concurrently_err() { + let sql = "REINDEX TABLE foo;"; + assert_snapshot!(lint_errors(sql, Rule::RequireConcurrentReindex), @" + warning[require-concurrent-reindex]: Reindexing a table or index without `concurrently` blocks reads and writes. + ╭▸ + 1 │ REINDEX TABLE foo; + │ ━━━━━━━━━━━━━━━━━ + │ + ├ help: Use `concurrently` to avoid blocking reads and writes. + ╭╴ + 1 │ REINDEX TABLE concurrently foo; + ╰╴ ++++++++++++ + "); + } + + #[test] + fn reindex_index_no_concurrently_err() { + let sql = "REINDEX INDEX foo;"; + assert_snapshot!(lint_errors(sql, Rule::RequireConcurrentReindex), @" + warning[require-concurrent-reindex]: Reindexing a table or index without `concurrently` blocks reads and writes. + ╭▸ + 1 │ REINDEX INDEX foo; + │ ━━━━━━━━━━━━━━━━━ + │ + ├ help: Use `concurrently` to avoid blocking reads and writes. + ╭╴ + 1 │ REINDEX INDEX concurrently foo; + ╰╴ ++++++++++++ + "); + } + + #[test] + fn reindex_schema_no_concurrently_err() { + let sql = "REINDEX SCHEMA foo;"; + assert_snapshot!(lint_errors(sql, Rule::RequireConcurrentReindex), @" + warning[require-concurrent-reindex]: Reindexing a table or index without `concurrently` blocks reads and writes. + ╭▸ + 1 │ REINDEX SCHEMA foo; + │ ━━━━━━━━━━━━━━━━━━ + │ + ├ help: Use `concurrently` to avoid blocking reads and writes. + ╭╴ + 1 │ REINDEX SCHEMA concurrently foo; + ╰╴ ++++++++++++ + "); + } + + #[test] + fn reindex_database_no_concurrently_err() { + let sql = "REINDEX DATABASE foo;"; + assert_snapshot!(lint_errors(sql, Rule::RequireConcurrentReindex), @" + warning[require-concurrent-reindex]: Reindexing a table or index without `concurrently` blocks reads and writes. + ╭▸ + 1 │ REINDEX DATABASE foo; + │ ━━━━━━━━━━━━━━━━━━━━ + │ + ├ help: Use `concurrently` to avoid blocking reads and writes. + ╭╴ + 1 │ REINDEX DATABASE concurrently foo; + ╰╴ ++++++++++++ + "); + } + + #[test] + fn reindex_table_concurrently_ok() { + let sql = "REINDEX TABLE CONCURRENTLY foo;"; + lint_ok(sql, Rule::RequireConcurrentReindex); + } + + #[test] + fn reindex_index_concurrently_ok() { + let sql = "REINDEX INDEX CONCURRENTLY foo;"; + lint_ok(sql, Rule::RequireConcurrentReindex); + } + + #[test] + fn reindex_options_concurrently_ok() { + let sql = "REINDEX (CONCURRENTLY) TABLE foo;"; + lint_ok(sql, Rule::RequireConcurrentReindex); + } + + #[test] + fn reindex_system_ok() { + let sql = "REINDEX SYSTEM mydb;"; + lint_ok(sql, Rule::RequireConcurrentReindex); + } + + #[test] + fn fix_reindex_table() { + let sql = "REINDEX TABLE foo;"; + assert_snapshot!(fix(sql), @"REINDEX TABLE concurrently foo;"); + } + + #[test] + fn fix_reindex_index() { + let sql = "REINDEX INDEX foo;"; + assert_snapshot!(fix(sql), @"REINDEX INDEX concurrently foo;"); + } +} diff --git a/crates/squawk_syntax/src/ast/generated/nodes.rs b/crates/squawk_syntax/src/ast/generated/nodes.rs index fa1877b8..4befec30 100644 --- a/crates/squawk_syntax/src/ast/generated/nodes.rs +++ b/crates/squawk_syntax/src/ast/generated/nodes.rs @@ -14181,6 +14181,10 @@ impl Reindex { support::child(&self.syntax) } #[inline] + pub fn concurrently_token(&self) -> Option { + support::token(&self.syntax, SyntaxKind::CONCURRENTLY_KW) + } + #[inline] pub fn database_token(&self) -> Option { support::token(&self.syntax, SyntaxKind::DATABASE_KW) } diff --git a/crates/squawk_syntax/src/postgresql.ungram b/crates/squawk_syntax/src/postgresql.ungram index 40a802ce..3a2b7ee9 100644 --- a/crates/squawk_syntax/src/postgresql.ungram +++ b/crates/squawk_syntax/src/postgresql.ungram @@ -2953,7 +2953,7 @@ SetTransaction = ) Reindex = - 'reindex' ('table' | 'index' | 'schema' | 'database' | 'system')? Path? + 'reindex' ('table' | 'index' | 'schema' | 'database' | 'system')? 'concurrently'? Path? CreateView = 'create' OrReplace? Persistence? 'recursive'? 'view' Path ColumnList? diff --git a/crates/xtask/src/new_rule.rs b/crates/xtask/src/new_rule.rs index d5b6e373..b5b6f79d 100644 --- a/crates/xtask/src/new_rule.rs +++ b/crates/xtask/src/new_rule.rs @@ -126,7 +126,7 @@ fn update_lib(name: &str) -> Result<()> { "// xtask:new-rule:rule-call", format!( r#"if self.rules.contains(&Rule::{name_pascal}) {{ - {name_snake}(self, &file); + {name_snake}(self, file); }} "# ), diff --git a/docs/docs/require-concurrent-reindex.md b/docs/docs/require-concurrent-reindex.md new file mode 100644 index 00000000..541e10c3 --- /dev/null +++ b/docs/docs/require-concurrent-reindex.md @@ -0,0 +1,32 @@ +--- +id: require-concurrent-reindex +title: require-concurrent-reindex +--- + +## problem + +`REINDEX` without `CONCURRENTLY` requires an `ACCESS EXCLUSIVE` lock on the index's table, blocking reads and writes. + +## solution + +Use `REINDEX ... CONCURRENTLY`. + +Instead of: + +```sql +-- blocks reads and writes +REINDEX TABLE foo; +REINDEX INDEX foo_idx; +``` + +Use: + +```sql +-- avoids blocking reads and writes +REINDEX TABLE CONCURRENTLY foo; +REINDEX INDEX CONCURRENTLY foo_idx; +``` + +## links + +- diff --git a/docs/sidebars.js b/docs/sidebars.js index 2f498cda..5d7b1328 100644 --- a/docs/sidebars.js +++ b/docs/sidebars.js @@ -38,6 +38,7 @@ module.exports = { "require-table-schema", "identifier-too-long", "require-concurrent-partition-detach", + "require-concurrent-reindex", // xtask:new-rule:error-name ], }, diff --git a/docs/src/pages/index.js b/docs/src/pages/index.js index b0004b24..5f48116c 100644 --- a/docs/src/pages/index.js +++ b/docs/src/pages/index.js @@ -222,6 +222,11 @@ const rules = [ tags: ["schema", "locking"], description: "Prevent blocking reads/writes to table while partition is detached.", }, + { + name: "require-concurrent-reindex", + tags: ["locking"], + description: "Prevent blocking reads/writes to table while index is reindexed.", + }, // xtask:new-rule:rule-doc-meta ]