diff --git a/PLAN.md b/PLAN.md new file mode 100644 index 000000000..6caaef381 --- /dev/null +++ b/PLAN.md @@ -0,0 +1,103 @@ +# pgfence Gap: Remaining Rules & Infrastructure + +Remaining gaps after the initial 11-rule PR. Based on source review of [pgfence](https://github.com/flvmnt/pgfence). + +## Gaps + +### 1. New Statement Rules (trivial) + +#### 1a. `BanEnableDisableTrigger` +- **Match:** `AlterTableStmt` → `AlterTableCmd` with `subtype()` in `{AtEnableTrig, AtDisableTrig, AtEnableTrigAll, AtDisableTrigAll, AtEnableTrigUser, AtDisableTrigUser}` +- **Severity:** Warning, not recommended (same tier as `banCreateTrigger`) +- **Lock:** SHARE ROW EXCLUSIVE +- **pgfence ref:** `trigger.ts:48-62`, rule `enable-disable-trigger` + +#### 1b. `BanUpdateWithoutWhere` +- **Match:** `UpdateStmt` where `where_clause.is_none()` +- **Severity:** Warning, recommended (same as `banDeleteWithoutWhere`) +- **pgfence ref:** `destructive.ts:83-99`, rule `update-in-migration` + +#### 1c. `BanAddExclusionConstraint` +- **Match:** `AlterTableStmt` → `AlterTableCmd(AtAddConstraint)` → `Constraint` where `contype() == ConstrExclusion` +- **Severity:** Warning, recommended (no concurrent alternative exists — only mitigation is `lock_timeout`) +- **pgfence ref:** `add-constraint.ts:100-116`, rule `add-constraint-exclude` +- **Note:** Also check `CreateStmt` → `constraints` for exclusion constraints in `CREATE TABLE` + +#### 1d. `WarnRefreshMatviewConcurrent` +- **Match:** `RefreshMatViewStmt` where `concurrent == true` +- **Severity:** Warning, not recommended (still takes EXCLUSIVE lock blocking DDL, but reads work) +- **pgfence ref:** `refresh-matview.ts:24-36`, rule `refresh-matview-concurrent` +- **Note:** Informational — concurrent refresh still blocks writes. Separate from `banBlockingRefreshMatview`. + +### 2. Policy / Cross-Statement Rules (medium) + +These require extending `TransactionState` in `linter_context.rs`. + +#### 2a. `RequireStatementTimeout` +- **Condition:** File contains a dangerous lock statement (ALTER TABLE, CREATE INDEX non-concurrent) but no `SET statement_timeout` or `SET LOCAL statement_timeout` before it. +- **Pattern:** Same as existing `lockTimeoutWarning` — extend `TransactionState` with `has_statement_timeout()`. +- **Severity:** Warning, not recommended (opt-in policy) +- **pgfence ref:** `policy.ts:76-94`, rule `missing-statement-timeout` + +#### 2b. `RequireIdleInTransactionTimeout` +- **Condition:** File contains dangerous statements but no `SET idle_in_transaction_session_timeout`. +- **Severity:** Warning, not recommended +- **pgfence ref:** `policy.ts:113-127`, rule `missing-idle-timeout` + +#### 2c. `BanNotValidValidateSameTransaction` +- **Condition:** `ALTER TABLE ... ADD CONSTRAINT ... NOT VALID` followed by `ALTER TABLE ... VALIDATE CONSTRAINT` on the same constraint within the same file/transaction. +- **Detection:** + 1. On `AtAddConstraint` with `Constraint.skip_validation == true` → record constraint name in `TransactionState` + 2. On `AtValidateConstraint` → check if constraint name was recorded → emit diagnostic + 3. Reset on `COMMIT`/`ROLLBACK` (already tracked by `TransactionState`) +- **Severity:** Error, recommended (defeats the purpose of NOT VALID) +- **pgfence ref:** `policy.ts:134-172`, rule `not-valid-validate-same-tx` +- **Impl:** Extend `TransactionState` with `not_valid_constraints: HashSet`. The constraint name comes from `AlterTableCmd.name` for validate, and from `Constraint.conname` for add. + +#### 2d. `WarnWideLockWindow` +- **Condition:** Within a transaction, ACCESS EXCLUSIVE held on table A, then ACCESS EXCLUSIVE acquired on table B. +- **Detection:** Track `access_exclusive_tables: HashSet` in `TransactionState`. On any statement that takes ACCESS EXCLUSIVE, check if set is non-empty with a different table. +- **Severity:** Warning, recommended +- **pgfence ref:** `policy.ts:174-203` + `transaction-state.ts:37-58`, rule `wide-lock-window` +- **Note:** Requires knowing which statements take ACCESS EXCLUSIVE. `AlterTableStmt` and non-concurrent `IndexStmt` are the main ones. + +### 3. Infrastructure Improvements (larger) + +#### 3a. `TransactionState` extensions +File: `crates/pgls_analyser/src/linter_context.rs` + +Add fields: +- `statement_timeout_set: bool` +- `idle_in_transaction_timeout_set: bool` +- `not_valid_constraints: HashSet` — constraint names added with NOT VALID in current tx +- `access_exclusive_tables: HashSet<(String, String)>` — (schema, table) pairs with ACCESS EXCLUSIVE in current tx + +The existing `TransactionState` builder in `FileContext` already processes `VariableSetStmt` for `lock_timeout`. Extend it to also detect `statement_timeout` and `idle_in_transaction_session_timeout`. For NOT VALID tracking, process `AlterTableCmd` nodes as they're visited. + +Check: `linter_context.rs` `FileContext::new()` — this is where `TransactionState` is built from `previous_stmts`. The constraint tracking needs to happen at statement-processing time, not just at context creation. + +#### 3b. ALTER COLUMN TYPE widening (deferred) +pgfence distinguishes safe type changes (e.g., `varchar(N)` → `text`) from dangerous ones. Their approach: classify by **target type only** (source type not in the AST). Target `text` or `varchar` without length = LOW risk. Everything else = HIGH. + +Our `changingColumnType` flags all type changes unconditionally. To match pgfence we'd need to inspect `ColumnDef.type_name` in the `AtAlterColumnType` command. This is doable but lower priority — our current rule is more conservative (fewer false negatives, more false positives). + +**Defer** to a follow-up PR. + +## Implementation Order + +1. **1b, 1c** — `BanUpdateWithoutWhere`, `BanAddExclusionConstraint` (trivial, no infra changes) +2. **1a** — `BanEnableDisableTrigger` (trivial) +3. **3a** — `TransactionState` extensions (needed by 2a-2d) +4. **2c** — `BanNotValidValidateSameTransaction` (highest value cross-statement rule) +5. **2a, 2b** — `RequireStatementTimeout`, `RequireIdleInTransactionTimeout` +6. **2d** — `WarnWideLockWindow` +7. **1d** — `WarnRefreshMatviewConcurrent` (nice-to-have, informational) +8. **3b** — ALTER COLUMN TYPE widening (deferred) + +## Out of Scope + +- **DB stats risk adjustment** — pgfence adjusts risk based on row count from live DB. We do static analysis only. +- **ORM/framework SQL extraction** — pgfence extracts SQL from Prisma/Knex/Drizzle/etc. We handle raw SQL. +- **Plugin system** — pgfence supports custom rule plugins. Not needed. +- **Savepoint-level lock tracking** — pgfence tracks lock snapshots at savepoints. Our `TransactionState` resets on COMMIT/ROLLBACK but doesn't model savepoints. Low priority. +- **Cross-file table tracking** — pgfence tracks `CREATE TABLE` across files in a batch to skip false positives in later files. Our `lockTimeoutWarning` already does per-file tracking. Cross-file is a workspace-level concern, defer. diff --git a/crates/pgls_analyse/src/metadata.rs b/crates/pgls_analyse/src/metadata.rs index a614e400e..f33eb856f 100644 --- a/crates/pgls_analyse/src/metadata.rs +++ b/crates/pgls_analyse/src/metadata.rs @@ -96,6 +96,8 @@ pub enum RuleSource { Squawk(&'static str), /// Rules from [Eugene](https://github.com/kaaveland/eugene) Eugene(&'static str), + /// Rules from [pgfence](https://github.com/flvmnt/pgfence) + Pgfence(&'static str), } impl PartialEq for RuleSource { @@ -109,6 +111,7 @@ impl std::fmt::Display for RuleSource { match self { Self::Squawk(_) => write!(f, "Squawk"), Self::Eugene(_) => write!(f, "Eugene"), + Self::Pgfence(_) => write!(f, "pgfence"), } } } @@ -132,6 +135,7 @@ impl RuleSource { match self { Self::Squawk(rule_name) => rule_name, Self::Eugene(rule_name) => rule_name, + Self::Pgfence(rule_name) => rule_name, } } @@ -139,6 +143,7 @@ impl RuleSource { match self { Self::Squawk(rule_name) => format!("squawk/{rule_name}"), Self::Eugene(rule_name) => format!("eugene/{rule_name}"), + Self::Pgfence(rule_name) => format!("pgfence/{rule_name}"), } } @@ -148,6 +153,7 @@ impl RuleSource { Self::Eugene(rule_name) => { format!("https://kaveland.no/eugene/hints/{rule_name}/index.html") } + Self::Pgfence(_) => "https://github.com/flvmnt/pgfence".to_string(), } } diff --git a/crates/pgls_analyser/src/lint/safety.rs b/crates/pgls_analyser/src/lint/safety.rs index 330b14b1c..84fe80a47 100644 --- a/crates/pgls_analyser/src/lint/safety.rs +++ b/crates/pgls_analyser/src/lint/safety.rs @@ -7,13 +7,26 @@ pub mod adding_foreign_key_constraint; pub mod adding_not_null_field; pub mod adding_primary_key_constraint; pub mod adding_required_field; +pub mod ban_add_exclusion_constraint; +pub mod ban_alter_enum_add_value; +pub mod ban_attach_partition; +pub mod ban_blocking_refresh_matview; pub mod ban_char_field; pub mod ban_concurrent_index_creation_in_transaction; +pub mod ban_create_trigger; +pub mod ban_delete_without_where; pub mod ban_drop_column; pub mod ban_drop_database; pub mod ban_drop_not_null; +pub mod ban_drop_schema; pub mod ban_drop_table; +pub mod ban_drop_trigger; +pub mod ban_enable_disable_trigger; +pub mod ban_not_valid_validate_same_transaction; +pub mod ban_truncate; pub mod ban_truncate_cascade; +pub mod ban_update_without_where; +pub mod ban_vacuum_full; pub mod changing_column_type; pub mod constraint_missing_not_valid; pub mod creating_enum; @@ -30,8 +43,14 @@ pub mod prefer_text_field; pub mod prefer_timestamptz; pub mod renaming_column; pub mod renaming_table; +pub mod require_concurrent_detach_partition; pub mod require_concurrent_index_creation; pub mod require_concurrent_index_deletion; +pub mod require_concurrent_reindex; +pub mod require_idle_in_transaction_timeout; +pub mod require_statement_timeout; pub mod running_statement_while_holding_access_exclusive; 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 :: 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 ,] } } +pub mod warn_refresh_matview_concurrent; +pub mod warn_wide_lock_window; +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_add_exclusion_constraint :: BanAddExclusionConstraint , self :: ban_alter_enum_add_value :: BanAlterEnumAddValue , self :: ban_attach_partition :: BanAttachPartition , self :: ban_blocking_refresh_matview :: BanBlockingRefreshMatview , self :: ban_char_field :: BanCharField , self :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction , self :: ban_create_trigger :: BanCreateTrigger , self :: ban_delete_without_where :: BanDeleteWithoutWhere , self :: ban_drop_column :: BanDropColumn , self :: ban_drop_database :: BanDropDatabase , self :: ban_drop_not_null :: BanDropNotNull , self :: ban_drop_schema :: BanDropSchema , self :: ban_drop_table :: BanDropTable , self :: ban_drop_trigger :: BanDropTrigger , self :: ban_enable_disable_trigger :: BanEnableDisableTrigger , self :: ban_not_valid_validate_same_transaction :: BanNotValidValidateSameTransaction , self :: ban_truncate :: BanTruncate , self :: ban_truncate_cascade :: BanTruncateCascade , self :: ban_update_without_where :: BanUpdateWithoutWhere , self :: ban_vacuum_full :: BanVacuumFull , 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_detach_partition :: RequireConcurrentDetachPartition , self :: require_concurrent_index_creation :: RequireConcurrentIndexCreation , self :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion , self :: require_concurrent_reindex :: RequireConcurrentReindex , self :: require_idle_in_transaction_timeout :: RequireIdleInTransactionTimeout , self :: require_statement_timeout :: RequireStatementTimeout , self :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive , self :: transaction_nesting :: TransactionNesting , self :: warn_refresh_matview_concurrent :: WarnRefreshMatviewConcurrent , self :: warn_wide_lock_window :: WarnWideLockWindow ,] } } diff --git a/crates/pgls_analyser/src/lint/safety/add_serial_column.rs b/crates/pgls_analyser/src/lint/safety/add_serial_column.rs index b99ee075e..a5612736a 100644 --- a/crates/pgls_analyser/src/lint/safety/add_serial_column.rs +++ b/crates/pgls_analyser/src/lint/safety/add_serial_column.rs @@ -7,7 +7,7 @@ declare_lint_rule! { /// Adding a column with a SERIAL type or GENERATED ALWAYS AS ... STORED causes a full table rewrite. /// /// When adding a column with a SERIAL type (serial, bigserial, smallserial) or a GENERATED ALWAYS AS ... STORED column - /// to an existing table, PostgreSQL must rewrite the entire table while holding an ACCESS EXCLUSIVE lock. + /// to an existing table, Postgres must rewrite the entire table while holding an ACCESS EXCLUSIVE lock. /// This blocks all reads and writes to the table for the duration of the rewrite operation. /// /// SERIAL types are implemented using sequences and DEFAULT values, while GENERATED ... STORED columns require diff --git a/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs b/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs index 63e81a17f..a04cd8f58 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_field_with_default.rs @@ -6,13 +6,13 @@ use pgls_diagnostics::Severity; declare_lint_rule! { /// Adding a column with a DEFAULT value may lead to a table rewrite while holding an ACCESS EXCLUSIVE lock. /// - /// In PostgreSQL versions before 11, adding a column with a DEFAULT value causes a full table rewrite, + /// In Postgres versions before 11, adding a column with a DEFAULT value causes a full table rewrite, /// which holds an ACCESS EXCLUSIVE lock on the table and blocks all reads and writes. /// - /// In PostgreSQL 11+, this behavior was optimized for non-volatile defaults. However: + /// In Postgres 11+, this behavior was optimized for non-volatile defaults. However: /// - Volatile default values (like random() or custom functions) still cause table rewrites /// - Generated columns (GENERATED ALWAYS AS) always require table rewrites - /// - Non-volatile defaults are safe in PostgreSQL 11+ + /// - Non-volatile defaults are safe in Postgres 11+ /// /// ## Examples /// @@ -45,7 +45,7 @@ impl LinterRule for AddingFieldWithDefault { fn run(ctx: &LinterRuleContext) -> Vec { let mut diagnostics = Vec::new(); - // Check PostgreSQL version - in 11+, non-volatile defaults are safe + // Check Postgres version - in 11+, non-volatile defaults are safe let pg_version = ctx.schema_cache().and_then(|sc| sc.version.major_version); if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() { @@ -110,7 +110,7 @@ impl LinterRule for AddingFieldWithDefault { "Adding a column with a volatile default value causes a table rewrite." }, ) - .detail(None, "Even in PostgreSQL 11+, volatile default values require a full table rewrite.") + .detail(None, "Even in Postgres 11+, volatile default values require a full table rewrite.") .note("Add the column without a default, then set the default in a separate statement."), ); } diff --git a/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs b/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs index 03606c6b7..0c0f9cd78 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_foreign_key_constraint.rs @@ -7,7 +7,7 @@ declare_lint_rule! { /// Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes. /// /// Adding a foreign key constraint to an existing table can cause downtime by locking both tables while - /// verifying the constraint. PostgreSQL needs to check that all existing values in the referencing + /// verifying the constraint. Postgres needs to check that all existing values in the referencing /// column exist in the referenced table. /// /// Instead, add the constraint as NOT VALID in one transaction, then VALIDATE it in another transaction. @@ -111,7 +111,7 @@ fn check_foreign_key_constraint( } else { ( "Adding a foreign key constraint requires a table scan and locks on both tables.", - "This will block writes to both the referencing and referenced tables while PostgreSQL verifies the constraint.", + "This will block writes to both the referencing and referenced tables while Postgres verifies the constraint.", "Add the constraint as NOT VALID first, then VALIDATE it in a separate transaction.", ) }; diff --git a/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs b/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs index d4e449805..512f1e938 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_not_null_field.rs @@ -6,11 +6,11 @@ use pgls_diagnostics::Severity; declare_lint_rule! { /// Setting a column NOT NULL blocks reads while the table is scanned. /// - /// In PostgreSQL versions before 11, adding a NOT NULL constraint to an existing column requires + /// In Postgres versions before 11, adding a NOT NULL constraint to an existing column requires /// a full table scan to verify that all existing rows satisfy the constraint. This operation /// takes an ACCESS EXCLUSIVE lock, blocking all reads and writes. /// - /// In PostgreSQL 11+, this operation is much faster as it can skip the full table scan for + /// In Postgres 11+, this operation is much faster as it can skip the full table scan for /// newly added columns with default values. /// /// Instead of using SET NOT NULL, consider using a CHECK constraint with NOT VALID, then diff --git a/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs b/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs index ab9932085..f74b39da0 100644 --- a/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs +++ b/crates/pgls_analyser/src/lint/safety/adding_primary_key_constraint.rs @@ -6,7 +6,7 @@ use pgls_diagnostics::Severity; declare_lint_rule! { /// Adding a primary key constraint results in locks and table rewrites. /// - /// When you add a PRIMARY KEY constraint, PostgreSQL needs to scan the entire table + /// When you add a PRIMARY KEY constraint, Postgres needs to scan the entire table /// to verify uniqueness and build the underlying index. This requires an ACCESS EXCLUSIVE /// lock which blocks all reads and writes. /// diff --git a/crates/pgls_analyser/src/lint/safety/ban_add_exclusion_constraint.rs b/crates/pgls_analyser/src/lint/safety/ban_add_exclusion_constraint.rs new file mode 100644 index 000000000..92e74505d --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_add_exclusion_constraint.rs @@ -0,0 +1,93 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Adding an exclusion constraint acquires an `ACCESS EXCLUSIVE` lock. + /// + /// Exclusion constraints require a full table scan to validate and block all reads + /// and writes while held. Unlike other constraints, there is no concurrent alternative. + /// Use `SET lock_timeout` to limit the impact on concurrent operations. + /// + /// This also applies to exclusion constraints defined inline in `CREATE TABLE`. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// alter table my_table add constraint my_excl exclude using gist (col with &&); + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// alter table my_table add constraint my_check check (col > 0) not valid; + /// ``` + /// + pub BanAddExclusionConstraint { + version: "next", + name: "banAddExclusionConstraint", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("add-constraint-exclude")], + } +} + +impl LinterRule for BanAddExclusionConstraint { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + match &ctx.stmt() { + pgls_query::NodeEnum::AlterTableStmt(stmt) => { + for cmd in &stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddConstraint + { + if let Some(pgls_query::NodeEnum::Constraint(constraint)) = + cmd.def.as_ref().and_then(|d| d.node.as_ref()) + { + if constraint.contype() + == pgls_query::protobuf::ConstrType::ConstrExclusion + { + diagnostics.push(exclusion_diagnostic()); + } + } + } + } + } + pgls_query::NodeEnum::CreateStmt(stmt) => { + for constraint_node in &stmt.constraints { + if let Some(pgls_query::NodeEnum::Constraint(constraint)) = + &constraint_node.node + { + if constraint.contype() == pgls_query::protobuf::ConstrType::ConstrExclusion + { + diagnostics.push(exclusion_diagnostic()); + } + } + } + } + _ => {} + } + + diagnostics + } +} + +fn exclusion_diagnostic() -> LinterDiagnostic { + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Adding an exclusion constraint acquires an ""ACCESS EXCLUSIVE"" lock." + }, + ) + .detail( + None, + "There is no concurrent alternative for exclusion constraints. Use SET lock_timeout to limit the impact on concurrent operations.", + ) +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_alter_enum_add_value.rs b/crates/pgls_analyser/src/lint/safety/ban_alter_enum_add_value.rs new file mode 100644 index 000000000..8ff47d65d --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_alter_enum_add_value.rs @@ -0,0 +1,60 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// `ALTER TYPE ... ADD VALUE` cannot run inside a transaction block in older Postgres versions. + /// + /// In Postgres versions before 12, `ALTER TYPE ... ADD VALUE` cannot be executed inside a + /// transaction block at all. On Postgres 12+, the operation is fast (metadata-only), but the + /// new enum value cannot be used in the same transaction until it is committed. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// alter type my_enum add value 'new_value'; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanAlterEnumAddValue { + version: "next", + name: "banAlterEnumAddValue", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("alter-enum-add-value")], + } +} + +impl LinterRule for BanAlterEnumAddValue { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::AlterEnumStmt(_) = &ctx.stmt() { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "ALTER TYPE ... ADD VALUE"" cannot be used in a transaction block before Postgres 12." + }, + ) + .detail( + None, + "On Postgres 12+, the operation is fast but the new value cannot be used in the same transaction until committed.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_attach_partition.rs b/crates/pgls_analyser/src/lint/safety/ban_attach_partition.rs new file mode 100644 index 000000000..40a9ea66d --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_attach_partition.rs @@ -0,0 +1,66 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Attaching a partition acquires an `ACCESS EXCLUSIVE` lock on the parent table. + /// + /// `ALTER TABLE ... ATTACH PARTITION` locks the parent table, blocking all reads and writes. + /// For large tables, this can cause significant downtime. Consider creating the partition + /// with the correct constraints upfront, or use a staging table approach. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// alter table my_table attach partition my_partition for values from ('2024-01-01') to ('2025-01-01'); + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanAttachPartition { + version: "next", + name: "banAttachPartition", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("attach-partition")], + } +} + +impl LinterRule for BanAttachPartition { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() { + for cmd in &stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAttachPartition + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Attaching a partition acquires an ""ACCESS EXCLUSIVE"" lock on the parent table." + }, + ) + .detail( + None, + "This blocks all reads and writes on the parent table. Consider adding a matching CHECK constraint to the child table before attaching to minimize lock duration.", + ), + ); + } + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_blocking_refresh_matview.rs b/crates/pgls_analyser/src/lint/safety/ban_blocking_refresh_matview.rs new file mode 100644 index 000000000..603cff7b3 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_blocking_refresh_matview.rs @@ -0,0 +1,62 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// `REFRESH MATERIALIZED VIEW` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. + /// + /// This blocks all reads on the materialized view until the refresh completes. + /// Use `REFRESH MATERIALIZED VIEW CONCURRENTLY` to allow reads during the refresh. + /// Note: concurrent refresh requires a unique index on the materialized view. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// refresh materialized view my_view; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// refresh materialized view concurrently my_view; + /// ``` + /// + pub BanBlockingRefreshMatview { + version: "next", + name: "banBlockingRefreshMatview", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("refresh-matview-blocking")], + } +} + +impl LinterRule for BanBlockingRefreshMatview { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::RefreshMatViewStmt(stmt) = &ctx.stmt() + && !stmt.concurrent + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "REFRESH MATERIALIZED VIEW"" without ""CONCURRENTLY"" blocks all reads." + }, + ) + .detail( + None, + "Use REFRESH MATERIALIZED VIEW CONCURRENTLY to allow reads during the refresh. This requires a unique index on the view.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_create_trigger.rs b/crates/pgls_analyser/src/lint/safety/ban_create_trigger.rs new file mode 100644 index 000000000..a99a4bab7 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_create_trigger.rs @@ -0,0 +1,60 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Creating a trigger acquires a `SHARE ROW EXCLUSIVE` lock on the table. + /// + /// `CREATE TRIGGER` can block concurrent writes while the lock is held. + /// Triggers also add hidden complexity to write operations on the table, + /// which can cause unexpected performance issues and make debugging harder. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// create trigger my_trigger after insert on my_table for each row execute function my_func(); + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanCreateTrigger { + version: "next", + name: "banCreateTrigger", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("create-trigger")], + } +} + +impl LinterRule for BanCreateTrigger { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::CreateTrigStmt(_) = &ctx.stmt() { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Creating a trigger acquires a ""SHARE ROW EXCLUSIVE"" lock on the table." + }, + ) + .detail( + None, + "Triggers add hidden complexity and can block concurrent writes. Consider using application-level logic instead.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_delete_without_where.rs b/crates/pgls_analyser/src/lint/safety/ban_delete_without_where.rs new file mode 100644 index 000000000..595edb54f --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_delete_without_where.rs @@ -0,0 +1,62 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// A `DELETE` statement without a `WHERE` clause will remove all rows from the table. + /// + /// This is almost always unintentional in a migration or application context. + /// If you truly need to remove all rows, use `TRUNCATE` explicitly (and acknowledge + /// its implications), or add a `WHERE true` to signal intent. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// delete from my_table; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// delete from my_table where expired_at < now(); + /// ``` + /// + pub BanDeleteWithoutWhere { + version: "next", + name: "banDeleteWithoutWhere", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("delete-without-where")], + } +} + +impl LinterRule for BanDeleteWithoutWhere { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::DeleteStmt(stmt) = &ctx.stmt() + && stmt.where_clause.is_none() + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "A ""DELETE"" without a ""WHERE"" clause will remove all rows from the table." + }, + ) + .detail( + None, + "Add a WHERE clause to limit which rows are deleted.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_schema.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_schema.rs new file mode 100644 index 000000000..7f47bd384 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_schema.rs @@ -0,0 +1,62 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Dropping a schema will remove all objects within it and may break existing clients. + /// + /// A `DROP SCHEMA` statement removes the entire schema and all objects it contains. + /// This is a destructive operation that can cause significant data loss and break + /// dependent applications. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// drop schema my_schema; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanDropSchema { + version: "next", + name: "banDropSchema", + severity: Severity::Error, + recommended: true, + sources: &[RuleSource::Pgfence("drop-schema")], + } +} + +impl LinterRule for BanDropSchema { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::DropStmt(stmt) = &ctx.stmt() + && stmt.remove_type() == pgls_query::protobuf::ObjectType::ObjectSchema + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Dropping a schema will remove all objects within it and may break existing clients." + }, + ) + .detail( + None, + "Remove objects individually instead, or ensure all dependent applications have been updated.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_drop_trigger.rs b/crates/pgls_analyser/src/lint/safety/ban_drop_trigger.rs new file mode 100644 index 000000000..2c134d6ee --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_drop_trigger.rs @@ -0,0 +1,61 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Dropping a trigger acquires an `ACCESS EXCLUSIVE` lock on the table. + /// + /// `DROP TRIGGER` blocks all reads and writes on the table while the lock is held. + /// It may also break application logic that depends on the trigger's behavior. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// drop trigger my_trigger on my_table; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanDropTrigger { + version: "next", + name: "banDropTrigger", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("drop-trigger")], + } +} + +impl LinterRule for BanDropTrigger { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::DropStmt(stmt) = &ctx.stmt() + && stmt.remove_type() == pgls_query::protobuf::ObjectType::ObjectTrigger + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Dropping a trigger acquires an ""ACCESS EXCLUSIVE"" lock on the table." + }, + ) + .detail( + None, + "This blocks all reads and writes. Ensure no application logic depends on the trigger before dropping it.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_enable_disable_trigger.rs b/crates/pgls_analyser/src/lint/safety/ban_enable_disable_trigger.rs new file mode 100644 index 000000000..36194b0f8 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_enable_disable_trigger.rs @@ -0,0 +1,74 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Enabling or disabling a trigger acquires a `SHARE ROW EXCLUSIVE` lock. + /// + /// `ALTER TABLE ... ENABLE/DISABLE TRIGGER` blocks concurrent writes while the lock is held. + /// This can cause downtime on busy tables. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// alter table my_table enable trigger my_trigger; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanEnableDisableTrigger { + version: "next", + name: "banEnableDisableTrigger", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("enable-disable-trigger")], + } +} + +const TRIGGER_SUBTYPES: &[pgls_query::protobuf::AlterTableType] = &[ + pgls_query::protobuf::AlterTableType::AtEnableTrig, + pgls_query::protobuf::AlterTableType::AtDisableTrig, + pgls_query::protobuf::AlterTableType::AtEnableTrigAll, + pgls_query::protobuf::AlterTableType::AtDisableTrigAll, + pgls_query::protobuf::AlterTableType::AtEnableTrigUser, + pgls_query::protobuf::AlterTableType::AtDisableTrigUser, +]; + +impl LinterRule for BanEnableDisableTrigger { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() { + for cmd in &stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && TRIGGER_SUBTYPES.contains(&cmd.subtype()) + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Enabling or disabling a trigger acquires a ""SHARE ROW EXCLUSIVE"" lock." + }, + ) + .detail( + None, + "This blocks concurrent writes. Consider the impact on busy tables and use SET lock_timeout.", + ), + ); + } + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_not_valid_validate_same_transaction.rs b/crates/pgls_analyser/src/lint/safety/ban_not_valid_validate_same_transaction.rs new file mode 100644 index 000000000..d85df90c3 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_not_valid_validate_same_transaction.rs @@ -0,0 +1,83 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Validating a constraint in the same transaction it was added as `NOT VALID` defeats the purpose. + /// + /// Adding a constraint with `NOT VALID` avoids a full table scan and lock during creation. + /// But if you immediately `VALIDATE CONSTRAINT` in the same transaction, the validation + /// still holds the lock from the `ADD CONSTRAINT`, blocking reads and writes. + /// + /// Run `VALIDATE CONSTRAINT` in a separate transaction to get the benefit of `NOT VALID`. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// Adding a NOT VALID constraint and validating it in the same transaction: + /// ```sql + /// ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; + /// ALTER TABLE orders VALIDATE CONSTRAINT orders_check; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub BanNotValidValidateSameTransaction { + version: "next", + name: "banNotValidValidateSameTransaction", + severity: Severity::Error, + recommended: true, + sources: &[RuleSource::Pgfence("not-valid-validate-same-tx")], + } +} + +impl LinterRule for BanNotValidValidateSameTransaction { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + let pgls_query::NodeEnum::AlterTableStmt(stmt) = ctx.stmt() else { + return diagnostics; + }; + + let tx_state = ctx.file_context().transaction_state(); + + let (table_schema, table_name) = stmt + .relation + .as_ref() + .map(|r| (r.schemaname.as_str(), r.relname.as_str())) + .unwrap_or_default(); + + for cmd in &stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtValidateConstraint + && !cmd.name.is_empty() + && tx_state.has_not_valid_constraint(table_schema, table_name, &cmd.name) + { + let constraint_name = &cmd.name; + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Constraint "{constraint_name}" was added as NOT VALID and validated in the same transaction." + }, + ) + .detail( + None, + "Run VALIDATE CONSTRAINT in a separate transaction to avoid holding locks during validation.", + ), + ); + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_truncate.rs b/crates/pgls_analyser/src/lint/safety/ban_truncate.rs new file mode 100644 index 000000000..69d8082b7 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_truncate.rs @@ -0,0 +1,60 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Truncating a table removes all rows and can cause data loss in production. + /// + /// `TRUNCATE` is a fast, non-transactional (in terms of row-level locking) way to remove + /// all data from a table. It acquires an `ACCESS EXCLUSIVE` lock and cannot be safely + /// rolled back in all scenarios. In a migration context, this is almost always a mistake. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// truncate my_table; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// delete from my_table where expired_at < now(); + /// ``` + /// + pub BanTruncate { + version: "next", + name: "banTruncate", + severity: Severity::Error, + recommended: true, + sources: &[RuleSource::Pgfence("truncate")], + } +} + +impl LinterRule for BanTruncate { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::TruncateStmt(_) = &ctx.stmt() { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Truncating a table removes all rows and can cause data loss." + }, + ) + .detail( + None, + "Use DELETE with a WHERE clause instead, or ensure this is intentional and not part of a migration.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_update_without_where.rs b/crates/pgls_analyser/src/lint/safety/ban_update_without_where.rs new file mode 100644 index 000000000..6f3170862 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_update_without_where.rs @@ -0,0 +1,61 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// An `UPDATE` statement without a `WHERE` clause will modify all rows in the table. + /// + /// This is almost always unintentional in a migration context and can cause data corruption. + /// If you truly need to update all rows, add a `WHERE true` to signal intent. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// update my_table set col = 'value'; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// update my_table set col = 'value' where id = 1; + /// ``` + /// + pub BanUpdateWithoutWhere { + version: "next", + name: "banUpdateWithoutWhere", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("update-in-migration")], + } +} + +impl LinterRule for BanUpdateWithoutWhere { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::UpdateStmt(stmt) = &ctx.stmt() + && stmt.where_clause.is_none() + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "An ""UPDATE"" without a ""WHERE"" clause will modify all rows in the table." + }, + ) + .detail( + None, + "Add a WHERE clause to limit which rows are updated.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/ban_vacuum_full.rs b/crates/pgls_analyser/src/lint/safety/ban_vacuum_full.rs new file mode 100644 index 000000000..32185e498 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/ban_vacuum_full.rs @@ -0,0 +1,77 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// `VACUUM FULL` rewrites the entire table and acquires an `ACCESS EXCLUSIVE` lock. + /// + /// This blocks all reads and writes for the duration of the operation, which can + /// take a very long time on large tables. Use regular `VACUUM` or `pg_repack` instead + /// for online table maintenance. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// vacuum full my_table; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// vacuum my_table; + /// ``` + /// + pub BanVacuumFull { + version: "next", + name: "banVacuumFull", + severity: Severity::Error, + recommended: true, + sources: &[RuleSource::Pgfence("vacuum-full")], + } +} + +impl LinterRule for BanVacuumFull { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::VacuumStmt(stmt) = &ctx.stmt() { + let is_full = stmt.options.iter().any(|opt| { + if let Some(pgls_query::NodeEnum::DefElem(def)) = &opt.node { + if def.defname == "full" { + return match &def.arg { + Some(arg) => match &arg.node { + Some(pgls_query::NodeEnum::Integer(i)) => i.ival != 0, + _ => true, + }, + None => true, + }; + } + } + false + }); + + if is_full { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "VACUUM FULL"" rewrites the entire table and blocks all access." + }, + ) + .detail( + None, + "Use regular VACUUM or pg_repack for online table maintenance without blocking reads and writes.", + ), + ); + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/changing_column_type.rs b/crates/pgls_analyser/src/lint/safety/changing_column_type.rs index 5f39cefa5..50a2ad58d 100644 --- a/crates/pgls_analyser/src/lint/safety/changing_column_type.rs +++ b/crates/pgls_analyser/src/lint/safety/changing_column_type.rs @@ -4,12 +4,18 @@ use pgls_console::markup; use pgls_diagnostics::Severity; declare_lint_rule! { - /// Changing a column type may break existing clients. + /// Changing a column type may require a table rewrite and break existing clients. /// - /// Changing a column's data type requires an exclusive lock on the table while the entire table is rewritten. - /// This can take a long time for large tables and will block reads and writes. + /// Most column type changes require an exclusive lock on the table while the entire + /// table is rewritten. This can take a long time for large tables and will block + /// reads and writes. /// - /// Instead of changing the type directly, consider creating a new column with the desired type, + /// Some type changes are safe and don't require a table rewrite: + /// - Changing to `text` (binary compatible with varchar/char types) + /// - Changing to `varchar` without a length limit + /// - Dropping a `numeric` precision constraint (e.g., `numeric(10,2)` to `numeric`) + /// + /// For unsafe type changes, consider creating a new column with the desired type, /// migrating the data, and then dropping the old column. /// /// ## Examples @@ -17,7 +23,13 @@ declare_lint_rule! { /// ### Invalid /// /// ```sql,expect_diagnostic - /// ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text USING "edits"::text; + /// ALTER TABLE "core_recipe" ALTER COLUMN "count" TYPE bigint; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text; /// ``` /// pub ChangingColumnType { @@ -40,6 +52,14 @@ impl LinterRule for ChangingColumnType { if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAlterColumnType { + if let Some(pgls_query::NodeEnum::ColumnDef(col_def)) = + cmd.def.as_ref().and_then(|d| d.node.as_ref()) + { + if is_safe_type_widening(col_def) { + continue; + } + } + diagnostics.push(LinterDiagnostic::new( rule_category!(), None, @@ -54,3 +74,37 @@ impl LinterRule for ChangingColumnType { diagnostics } } + +fn is_safe_type_widening(col_def: &pgls_query::protobuf::ColumnDef) -> bool { + let Some(type_name) = &col_def.type_name else { + return false; + }; + + let target_type = type_name + .names + .iter() + .filter_map(|n| { + if let Some(pgls_query::NodeEnum::String(s)) = &n.node { + Some(s.sval.as_str()) + } else { + None + } + }) + .last(); + + let Some(target_type) = target_type else { + return false; + }; + + let has_type_modifier = !type_name.typmods.is_empty(); + + match target_type.to_lowercase().as_str() { + // text is always safe — binary compatible with varchar/char + "text" => true, + // varchar without length is safe (dropping a length constraint) + "varchar" if !has_type_modifier => true, + // numeric without precision is safe (dropping precision constraint) + "numeric" | "decimal" if !has_type_modifier => true, + _ => false, + } +} diff --git a/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs b/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs index 248765101..fb9bce5ad 100644 --- a/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs +++ b/crates/pgls_analyser/src/lint/safety/constraint_missing_not_valid.rs @@ -6,7 +6,7 @@ use pgls_diagnostics::Severity; declare_lint_rule! { /// Adding constraints without NOT VALID blocks all reads and writes. /// - /// When adding a CHECK or FOREIGN KEY constraint, PostgreSQL must validate all existing rows, + /// When adding a CHECK or FOREIGN KEY constraint, Postgres must validate all existing rows, /// which requires a full table scan. This blocks reads and writes for the duration. /// /// Instead, add the constraint with NOT VALID first, then VALIDATE CONSTRAINT in a separate diff --git a/crates/pgls_analyser/src/lint/safety/creating_enum.rs b/crates/pgls_analyser/src/lint/safety/creating_enum.rs index 1791361f8..6e689aeb5 100644 --- a/crates/pgls_analyser/src/lint/safety/creating_enum.rs +++ b/crates/pgls_analyser/src/lint/safety/creating_enum.rs @@ -9,7 +9,7 @@ declare_lint_rule! { /// Enumerated types have several limitations that make them difficult to work with in production: /// /// - Removing values from an enum requires complex migrations and is not supported directly - /// - Adding values to an enum requires an ACCESS EXCLUSIVE lock in some PostgreSQL versions + /// - Adding values to an enum requires an ACCESS EXCLUSIVE lock in some Postgres versions /// - Associating additional data with enum values is impossible without restructuring /// - Renaming enum values requires careful migration planning /// diff --git a/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs b/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs index 08f5f68c0..59f2d6c8e 100644 --- a/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs +++ b/crates/pgls_analyser/src/lint/safety/multiple_alter_table.rs @@ -6,12 +6,12 @@ use pgls_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 + /// When you run multiple ALTER TABLE statements on the same table, Postgres 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 + /// allows Postgres to scan and modify the table only once, improving performance and reducing /// the time locks are held. /// /// ## Examples diff --git a/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs b/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs index cbfd24984..51f01c78d 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_jsonb.rs @@ -6,7 +6,7 @@ use pgls_diagnostics::Severity; declare_lint_rule! { /// Prefer JSONB over JSON types. /// - /// JSONB is the binary JSON data type in PostgreSQL that is more efficient for most use cases. + /// JSONB is the binary JSON data type in Postgres that is more efficient for most use cases. /// Unlike JSON, JSONB stores data in a decomposed binary format which provides several advantages: /// - Significantly faster query performance for operations like indexing and searching /// - Support for indexing (GIN indexes) diff --git a/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs b/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs index ac9a89ceb..a9ede8770 100644 --- a/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs +++ b/crates/pgls_analyser/src/lint/safety/prefer_robust_stmts.rs @@ -22,6 +22,14 @@ declare_lint_rule! { /// DROP INDEX CONCURRENTLY users_email_idx; /// ``` /// + /// ```sql,expect_diagnostic + /// CREATE TABLE users (id int); + /// ``` + /// + /// ```sql,expect_diagnostic + /// DROP TABLE users; + /// ``` + /// /// ### Valid /// /// ```sql @@ -32,6 +40,14 @@ declare_lint_rule! { /// DROP INDEX CONCURRENTLY IF EXISTS users_email_idx; /// ``` /// + /// ```sql + /// CREATE TABLE IF NOT EXISTS users (id int); + /// ``` + /// + /// ```sql + /// DROP TABLE IF EXISTS users; + /// ``` + /// pub PreferRobustStmts { version: "next", name: "preferRobustStmts", @@ -82,7 +98,6 @@ impl LinterRule for PreferRobustStmts { } } pgls_query::NodeEnum::DropStmt(stmt) => { - // Concurrent drop runs outside transaction if stmt.concurrent && !stmt.missing_ok { diagnostics.push( LinterDiagnostic::new( @@ -98,6 +113,40 @@ impl LinterRule for PreferRobustStmts { ), ); } + if stmt.remove_type() == pgls_query::protobuf::ObjectType::ObjectTable + && !stmt.missing_ok + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "DROP TABLE"" should use ""IF EXISTS""." + }, + ) + .detail( + None, + "Add IF EXISTS to make the migration re-runnable if it fails.", + ), + ); + } + } + pgls_query::NodeEnum::CreateStmt(stmt) => { + if !stmt.if_not_exists { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "CREATE TABLE"" should use ""IF NOT EXISTS""." + }, + ) + .detail( + None, + "Add IF NOT EXISTS to make the migration re-runnable if it fails.", + ), + ); + } } _ => {} } diff --git a/crates/pgls_analyser/src/lint/safety/require_concurrent_detach_partition.rs b/crates/pgls_analyser/src/lint/safety/require_concurrent_detach_partition.rs new file mode 100644 index 000000000..c4437e187 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/require_concurrent_detach_partition.rs @@ -0,0 +1,70 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Detaching a partition without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. + /// + /// `ALTER TABLE ... DETACH PARTITION` without `CONCURRENTLY` blocks all reads and writes + /// on the parent table. Use `DETACH PARTITION ... CONCURRENTLY` (Postgres 14+) to + /// avoid blocking concurrent operations. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// alter table my_table detach partition my_partition; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// alter table my_table detach partition my_partition concurrently; + /// ``` + /// + pub RequireConcurrentDetachPartition { + version: "next", + name: "requireConcurrentDetachPartition", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("detach-partition")], + } +} + +impl LinterRule for RequireConcurrentDetachPartition { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::AlterTableStmt(stmt) = &ctx.stmt() { + for cmd in &stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtDetachPartition + && !matches!( + cmd.def.as_ref().and_then(|d| d.node.as_ref()), + Some(pgls_query::NodeEnum::PartitionCmd(p)) if p.concurrent + ) + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Detaching a partition without ""CONCURRENTLY"" blocks all table access." + }, + ) + .detail( + None, + "Use DETACH PARTITION ... CONCURRENTLY (Postgres 14+) to avoid blocking reads and writes.", + ), + ); + } + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/require_concurrent_reindex.rs b/crates/pgls_analyser/src/lint/safety/require_concurrent_reindex.rs new file mode 100644 index 000000000..139504c46 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/require_concurrent_reindex.rs @@ -0,0 +1,68 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// `REINDEX` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock on the table. + /// + /// This blocks all reads and writes until the reindex completes. Use `REINDEX CONCURRENTLY` + /// to rebuild the index without blocking concurrent operations. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// reindex index my_index; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// reindex index concurrently my_index; + /// ``` + /// + pub RequireConcurrentReindex { + version: "next", + name: "requireConcurrentReindex", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("reindex-non-concurrent")], + } +} + +impl LinterRule for RequireConcurrentReindex { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::ReindexStmt(stmt) = &ctx.stmt() { + let is_concurrent = stmt.params.iter().any(|param| { + if let Some(pgls_query::NodeEnum::DefElem(def)) = ¶m.node { + return def.defname == "concurrently"; + } + false + }); + + if !is_concurrent { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "REINDEX"" without ""CONCURRENTLY"" blocks all table access." + }, + ) + .detail( + None, + "Use REINDEX CONCURRENTLY to rebuild the index without blocking reads and writes.", + ), + ); + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/require_idle_in_transaction_timeout.rs b/crates/pgls_analyser/src/lint/safety/require_idle_in_transaction_timeout.rs new file mode 100644 index 000000000..2974ae929 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/require_idle_in_transaction_timeout.rs @@ -0,0 +1,63 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Dangerous lock statements should be preceded by `SET idle_in_transaction_session_timeout`. + /// + /// A transaction holding dangerous locks that goes idle (e.g., due to application errors) + /// will block other operations indefinitely. Setting `idle_in_transaction_session_timeout` + /// ensures the session is terminated if it sits idle too long. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// ALTER TABLE users ADD COLUMN email TEXT; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// CREATE INDEX CONCURRENTLY users_email_idx ON users(email); + /// ``` + /// + pub RequireIdleInTransactionTimeout { + version: "next", + name: "requireIdleInTransactionTimeout", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("missing-idle-timeout")], + } +} + +impl LinterRule for RequireIdleInTransactionTimeout { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let tx_state = ctx.file_context().transaction_state(); + if tx_state.has_idle_in_transaction_timeout() { + return vec![]; + } + + if !tx_state.is_dangerous_lock_stmt(ctx.stmt()) { + return vec![]; + } + + vec![ + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Statement takes a dangerous lock without ""idle_in_transaction_session_timeout"" set." + }, + ) + .detail( + None, + "Run SET idle_in_transaction_session_timeout = '...' before this statement to prevent idle transactions from holding locks.", + ), + ] + } +} diff --git a/crates/pgls_analyser/src/lint/safety/require_statement_timeout.rs b/crates/pgls_analyser/src/lint/safety/require_statement_timeout.rs new file mode 100644 index 000000000..951882e12 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/require_statement_timeout.rs @@ -0,0 +1,62 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Dangerous lock statements should be preceded by `SET statement_timeout`. + /// + /// Long-running statements holding locks can block other operations. Setting a + /// `statement_timeout` ensures the statement fails rather than running indefinitely. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// ALTER TABLE users ADD COLUMN email TEXT; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// CREATE INDEX CONCURRENTLY users_email_idx ON users(email); + /// ``` + /// + pub RequireStatementTimeout { + version: "next", + name: "requireStatementTimeout", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("missing-statement-timeout")], + } +} + +impl LinterRule for RequireStatementTimeout { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let tx_state = ctx.file_context().transaction_state(); + if tx_state.has_statement_timeout() { + return vec![]; + } + + if !tx_state.is_dangerous_lock_stmt(ctx.stmt()) { + return vec![]; + } + + vec![ + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Statement takes a dangerous lock without a ""statement_timeout"" set." + }, + ) + .detail( + None, + "Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely.", + ), + ] + } +} diff --git a/crates/pgls_analyser/src/lint/safety/warn_refresh_matview_concurrent.rs b/crates/pgls_analyser/src/lint/safety/warn_refresh_matview_concurrent.rs new file mode 100644 index 000000000..83fc17287 --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/warn_refresh_matview_concurrent.rs @@ -0,0 +1,62 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// `REFRESH MATERIALIZED VIEW CONCURRENTLY` still acquires an `EXCLUSIVE` lock. + /// + /// While concurrent refresh allows reads during the refresh, it still blocks DDL + /// and other write operations on the materialized view. On large views, this can + /// take a long time. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// ```sql,expect_diagnostic + /// refresh materialized view concurrently my_view; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub WarnRefreshMatviewConcurrent { + version: "next", + name: "warnRefreshMatviewConcurrent", + severity: Severity::Warning, + recommended: false, + sources: &[RuleSource::Pgfence("refresh-matview-concurrent")], + } +} + +impl LinterRule for WarnRefreshMatviewConcurrent { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + if let pgls_query::NodeEnum::RefreshMatViewStmt(stmt) = &ctx.stmt() + && stmt.concurrent + { + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "REFRESH MATERIALIZED VIEW CONCURRENTLY"" still acquires an ""EXCLUSIVE"" lock." + }, + ) + .detail( + None, + "Concurrent refresh allows reads but still blocks DDL and writes. Consider the impact on long-running refreshes.", + ), + ); + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/lint/safety/warn_wide_lock_window.rs b/crates/pgls_analyser/src/lint/safety/warn_wide_lock_window.rs new file mode 100644 index 000000000..791a2f9fb --- /dev/null +++ b/crates/pgls_analyser/src/lint/safety/warn_wide_lock_window.rs @@ -0,0 +1,111 @@ +use crate::{LinterDiagnostic, LinterRule, LinterRuleContext}; +use pgls_analyse::{RuleSource, declare_lint_rule}; +use pgls_console::markup; +use pgls_diagnostics::Severity; + +declare_lint_rule! { + /// Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window. + /// + /// When a transaction holds an ACCESS EXCLUSIVE lock on one table and acquires + /// another on a different table, both locks are held until the transaction commits. + /// This increases the chance of blocking concurrent operations and causing downtime. + /// + /// Split the operations into separate transactions to minimize the lock window. + /// + /// ## Examples + /// + /// ### Invalid + /// + /// Acquiring locks on multiple tables in the same transaction: + /// ```sql + /// ALTER TABLE users ADD COLUMN email TEXT; + /// ALTER TABLE orders ADD COLUMN total NUMERIC; + /// ``` + /// + /// ### Valid + /// + /// ```sql + /// select 1; + /// ``` + /// + pub WarnWideLockWindow { + version: "next", + name: "warnWideLockWindow", + severity: Severity::Warning, + recommended: true, + sources: &[RuleSource::Pgfence("wide-lock-window")], + } +} + +impl LinterRule for WarnWideLockWindow { + type Options = (); + + fn run(ctx: &LinterRuleContext) -> Vec { + let mut diagnostics = vec![]; + + let tx_state = ctx.file_context().transaction_state(); + let existing_tables = tx_state.access_exclusive_tables(); + + if existing_tables.is_empty() { + return diagnostics; + } + + // Only AlterTableStmt with ACCESS EXCLUSIVE subtypes widens the lock window + let current_table = match ctx.stmt() { + pgls_query::NodeEnum::AlterTableStmt(stmt) => { + let has_access_exclusive = stmt.cmds.iter().any(|cmd| { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node { + !matches!( + cmd.subtype(), + pgls_query::protobuf::AlterTableType::AtValidateConstraint + ) + } else { + false + } + }); + if has_access_exclusive { + stmt.relation.as_ref().map(|r| { + let schema = if r.schemaname.is_empty() { + "public".to_string() + } else { + r.schemaname.clone() + }; + (schema, r.relname.clone()) + }) + } else { + None + } + } + _ => None, + }; + + if let Some((schema, name)) = current_table { + if tx_state.has_created_object(&schema, &name) { + return diagnostics; + } + + let is_new_table = !existing_tables + .iter() + .any(|(s, n)| s == &schema && n == &name); + + if is_new_table { + let full_name = format!("{schema}.{name}"); + diagnostics.push( + LinterDiagnostic::new( + rule_category!(), + None, + markup! { + "Acquiring a lock on "{full_name}" while already holding ACCESS EXCLUSIVE locks on other tables." + }, + ) + .detail( + None, + "This widens the lock window. Split into separate transactions to minimize lock duration.", + ), + ); + } + } + + diagnostics + } +} diff --git a/crates/pgls_analyser/src/linter_context.rs b/crates/pgls_analyser/src/linter_context.rs index 36c262ef1..c0436bedb 100644 --- a/crates/pgls_analyser/src/linter_context.rs +++ b/crates/pgls_analyser/src/linter_context.rs @@ -136,16 +136,29 @@ impl<'a> AnalysedFileContext<'a> { /// - Whether a lock timeout has been set /// - Which objects have been created in this transaction /// - Whether an ACCESS EXCLUSIVE lock is currently being held +/// +/// Transaction boundaries (BEGIN/COMMIT/ROLLBACK) reset accumulated state +/// to avoid false positives across separate transactions in the same file. #[derive(Debug, Default)] pub struct TransactionState { /// Whether `SET lock_timeout` has been called in this transaction lock_timeout_set: bool, + /// Whether `SET statement_timeout` has been called in this transaction + statement_timeout_set: bool, + /// Whether `SET idle_in_transaction_session_timeout` has been called in this transaction + idle_in_transaction_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)>, /// Whether an ACCESS EXCLUSIVE lock is currently being held /// This is set when an ALTER TABLE is executed on an existing table holding_access_exclusive: bool, + /// Constraints added with NOT VALID: (schema, table, constraint_name) + not_valid_constraints: Vec<(String, String, String)>, + /// Tables holding ACCESS EXCLUSIVE locks in this transaction (for wide lock window detection) + access_exclusive_tables: Vec<(String, String)>, + /// Transaction nesting depth (0 = not in explicit transaction) + transaction_depth: usize, } impl TransactionState { @@ -154,6 +167,31 @@ impl TransactionState { self.lock_timeout_set } + /// Returns true if a statement timeout has been set in this transaction + pub fn has_statement_timeout(&self) -> bool { + self.statement_timeout_set + } + + /// Returns true if an idle-in-transaction timeout has been set in this transaction + pub fn has_idle_in_transaction_timeout(&self) -> bool { + self.idle_in_transaction_timeout_set + } + + /// Returns true if a constraint with the given name on the given table was added with NOT VALID + pub fn has_not_valid_constraint(&self, schema: &str, table: &str, name: &str) -> bool { + let normalized_schema = if schema.is_empty() { "public" } else { schema }; + self.not_valid_constraints.iter().any(|(s, t, n)| { + normalized_schema.eq_ignore_ascii_case(s) + && table.eq_ignore_ascii_case(t) + && name.eq_ignore_ascii_case(n) + }) + } + + /// Returns the tables currently holding ACCESS EXCLUSIVE locks + pub fn access_exclusive_tables(&self) -> &[(String, String)] { + &self.access_exclusive_tables + } + /// 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" @@ -169,6 +207,48 @@ impl TransactionState { self.holding_access_exclusive } + /// Returns true if the statement takes a dangerous lock on an existing object. + /// + /// Covers: AlterTableStmt, non-concurrent IndexStmt, DropStmt (table/index), + /// TruncateStmt, VacuumStmt, non-concurrent ReindexStmt, RenameStmt (with relation), + /// and non-concurrent RefreshMatViewStmt. + pub fn is_dangerous_lock_stmt(&self, stmt: &pgls_query::NodeEnum) -> bool { + match stmt { + pgls_query::NodeEnum::AlterTableStmt(alter_stmt) => { + if let Some(relation) = &alter_stmt.relation { + !self.has_created_object(&relation.schemaname, &relation.relname) + } else { + true + } + } + pgls_query::NodeEnum::IndexStmt(index_stmt) => { + if index_stmt.concurrent { + return false; + } + if let Some(relation) = &index_stmt.relation { + !self.has_created_object(&relation.schemaname, &relation.relname) + } else { + true + } + } + pgls_query::NodeEnum::DropStmt(drop_stmt) => { + let obj_type = drop_stmt.remove_type(); + matches!( + obj_type, + pgls_query::protobuf::ObjectType::ObjectTable + | pgls_query::protobuf::ObjectType::ObjectIndex + | pgls_query::protobuf::ObjectType::ObjectMatview + ) + } + pgls_query::NodeEnum::TruncateStmt(_) => true, + pgls_query::NodeEnum::VacuumStmt(_) => true, + pgls_query::NodeEnum::ReindexStmt(_) => true, + pgls_query::NodeEnum::RenameStmt(rename_stmt) => rename_stmt.relation.is_some(), + pgls_query::NodeEnum::RefreshMatViewStmt(stmt) => !stmt.concurrent, + _ => false, + } + } + /// 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 @@ -180,13 +260,69 @@ impl TransactionState { self.created_objects.push((normalized_schema, name)); } + /// Reset per-transaction accumulated state. + /// Called on COMMIT/ROLLBACK to avoid false positives across transactions. + fn reset_transaction_state(&mut self) { + self.lock_timeout_set = false; + self.statement_timeout_set = false; + self.idle_in_transaction_timeout_set = false; + self.created_objects.clear(); + self.holding_access_exclusive = false; + self.not_valid_constraints.clear(); + self.access_exclusive_tables.clear(); + } + + /// Returns true if an ALTER TABLE subcommand takes an ACCESS EXCLUSIVE lock. + /// Some subtypes take lighter locks and should not be tracked. + fn is_access_exclusive_subcommand(subtype: pgls_query::protobuf::AlterTableType) -> bool { + use pgls_query::protobuf::AlterTableType; + !matches!( + subtype, + // VALIDATE CONSTRAINT takes SHARE UPDATE EXCLUSIVE + AlterTableType::AtValidateConstraint + ) + } + /// Update transaction state based on a statement pub(crate) fn update_from_stmt(&mut self, stmt: &pgls_query::NodeEnum) { - // Track SET lock_timeout - if let pgls_query::NodeEnum::VariableSetStmt(set_stmt) = stmt - && set_stmt.name.eq_ignore_ascii_case("lock_timeout") - { - self.lock_timeout_set = true; + // Handle transaction boundaries + if let pgls_query::NodeEnum::TransactionStmt(tx_stmt) = stmt { + use pgls_query::protobuf::TransactionStmtKind; + match tx_stmt.kind() { + TransactionStmtKind::TransStmtBegin | TransactionStmtKind::TransStmtStart => { + self.transaction_depth += 1; + } + TransactionStmtKind::TransStmtCommit | TransactionStmtKind::TransStmtRollback => { + if self.transaction_depth > 0 { + self.transaction_depth -= 1; + } + // Reset accumulated state — new transaction starts fresh + self.reset_transaction_state(); + } + TransactionStmtKind::TransStmtSavepoint => { + self.transaction_depth += 1; + } + TransactionStmtKind::TransStmtRelease + | TransactionStmtKind::TransStmtRollbackTo => { + if self.transaction_depth > 0 { + self.transaction_depth -= 1; + } + } + _ => {} + } + return; + } + + // Track SET timeouts + if let pgls_query::NodeEnum::VariableSetStmt(set_stmt) = stmt { + let name = &set_stmt.name; + if name.eq_ignore_ascii_case("lock_timeout") { + self.lock_timeout_set = true; + } else if name.eq_ignore_ascii_case("statement_timeout") { + self.statement_timeout_set = true; + } else if name.eq_ignore_ascii_case("idle_in_transaction_session_timeout") { + self.idle_in_transaction_timeout_set = true; + } } // Track created objects @@ -220,16 +356,72 @@ impl TransactionState { _ => {} } + // Track NOT VALID constraints (table-qualified to avoid false positives) + if let pgls_query::NodeEnum::AlterTableStmt(alter_stmt) = stmt { + let (table_schema, table_name) = alter_stmt + .relation + .as_ref() + .map(|r| { + let s = if r.schemaname.is_empty() { + "public".to_string() + } else { + r.schemaname.clone() + }; + (s, r.relname.clone()) + }) + .unwrap_or_default(); + + for cmd in &alter_stmt.cmds { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node + && cmd.subtype() == pgls_query::protobuf::AlterTableType::AtAddConstraint + { + if let Some(pgls_query::NodeEnum::Constraint(constraint)) = + cmd.def.as_ref().and_then(|d| d.node.as_ref()) + { + if constraint.skip_validation && !constraint.conname.is_empty() { + self.not_valid_constraints.push(( + table_schema.clone(), + table_name.clone(), + constraint.conname.clone(), + )); + } + } + } + } + } + // Track ACCESS EXCLUSIVE lock acquisition - // ALTER TABLE on an existing table acquires ACCESS EXCLUSIVE lock + // Only track ALTER TABLE subtypes that actually take ACCESS EXCLUSIVE locks if let pgls_query::NodeEnum::AlterTableStmt(alter_stmt) = stmt && let Some(relation) = &alter_stmt.relation { - let schema = &relation.schemaname; - let name = &relation.relname; - // Only set the flag if altering an existing table (not one created in this transaction) - if !self.has_created_object(schema, name) { - self.holding_access_exclusive = true; + let has_access_exclusive_cmd = alter_stmt.cmds.iter().any(|cmd| { + if let Some(pgls_query::NodeEnum::AlterTableCmd(cmd)) = &cmd.node { + Self::is_access_exclusive_subcommand(cmd.subtype()) + } else { + false + } + }); + + if has_access_exclusive_cmd { + let schema = &relation.schemaname; + let name = &relation.relname; + if !self.has_created_object(schema, name) { + self.holding_access_exclusive = true; + let normalized_schema = if schema.is_empty() { + "public".to_string() + } else { + schema.clone() + }; + if !self + .access_exclusive_tables + .iter() + .any(|(s, n)| s == &normalized_schema && n == name) + { + self.access_exclusive_tables + .push((normalized_schema, name.clone())); + } + } } } } diff --git a/crates/pgls_analyser/src/options.rs b/crates/pgls_analyser/src/options.rs index c7625a5a3..c5e876807 100644 --- a/crates/pgls_analyser/src/options.rs +++ b/crates/pgls_analyser/src/options.rs @@ -11,17 +11,38 @@ pub type AddingNotNullField = pub type AddingPrimaryKeyConstraint = < lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint as crate :: LinterRule > :: Options ; pub type AddingRequiredField = ::Options; +pub type BanAddExclusionConstraint = < lint :: safety :: ban_add_exclusion_constraint :: BanAddExclusionConstraint as crate :: LinterRule > :: Options ; +pub type BanAlterEnumAddValue = + ::Options; +pub type BanAttachPartition = + ::Options; +pub type BanBlockingRefreshMatview = < lint :: safety :: ban_blocking_refresh_matview :: BanBlockingRefreshMatview as crate :: LinterRule > :: Options ; pub type BanCharField = ::Options; pub type BanConcurrentIndexCreationInTransaction = < lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction as crate :: LinterRule > :: Options ; +pub type BanCreateTrigger = + ::Options; +pub type BanDeleteWithoutWhere = + ::Options; pub type BanDropColumn = ::Options; pub type BanDropDatabase = ::Options; pub type BanDropNotNull = ::Options; +pub type BanDropSchema = + ::Options; pub type BanDropTable = ::Options; +pub type BanDropTrigger = + ::Options; +pub type BanEnableDisableTrigger = < lint :: safety :: ban_enable_disable_trigger :: BanEnableDisableTrigger as crate :: LinterRule > :: Options ; +pub type BanNotValidValidateSameTransaction = < lint :: safety :: ban_not_valid_validate_same_transaction :: BanNotValidValidateSameTransaction as crate :: LinterRule > :: Options ; +pub type BanTruncate = ::Options; pub type BanTruncateCascade = ::Options; +pub type BanUpdateWithoutWhere = + ::Options; +pub type BanVacuumFull = + ::Options; pub type ChangingColumnType = ::Options; pub type ConstraintMissingNotValid = < lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid as crate :: LinterRule > :: Options ; @@ -48,8 +69,15 @@ pub type RenamingColumn = ::Options; pub type RenamingTable = ::Options; +pub type RequireConcurrentDetachPartition = < lint :: safety :: require_concurrent_detach_partition :: RequireConcurrentDetachPartition as crate :: LinterRule > :: Options ; pub type RequireConcurrentIndexCreation = < lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation as crate :: LinterRule > :: Options ; pub type RequireConcurrentIndexDeletion = < lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion as crate :: LinterRule > :: Options ; +pub type RequireConcurrentReindex = < lint :: safety :: require_concurrent_reindex :: RequireConcurrentReindex as crate :: LinterRule > :: Options ; +pub type RequireIdleInTransactionTimeout = < lint :: safety :: require_idle_in_transaction_timeout :: RequireIdleInTransactionTimeout as crate :: LinterRule > :: Options ; +pub type RequireStatementTimeout = < lint :: safety :: require_statement_timeout :: RequireStatementTimeout as crate :: LinterRule > :: Options ; pub type RunningStatementWhileHoldingAccessExclusive = < lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive as crate :: LinterRule > :: Options ; pub type TransactionNesting = ::Options; +pub type WarnRefreshMatviewConcurrent = < lint :: safety :: warn_refresh_matview_concurrent :: WarnRefreshMatviewConcurrent as crate :: LinterRule > :: Options ; +pub type WarnWideLockWindow = + ::Options; diff --git a/crates/pgls_analyser/src/registry.rs b/crates/pgls_analyser/src/registry.rs index 7e3872905..781669acb 100644 --- a/crates/pgls_analyser/src/registry.rs +++ b/crates/pgls_analyser/src/registry.rs @@ -8,5 +8,5 @@ pub fn visit_registry(registry: &mut V) { #[doc = r" Maps rule keys to rule executors (zero-cost abstraction)"] #[doc = r" This function is generated by codegen and includes all linter rules"] pub fn get_linter_rule_executor(key: &RuleKey) -> Option { - match key . rule_name () { "addSerialColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: add_serial_column :: AddSerialColumn > ()) , "addingFieldWithDefault" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_field_with_default :: AddingFieldWithDefault > ()) , "addingForeignKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint > ()) , "addingNotNullField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_not_null_field :: AddingNotNullField > ()) , "addingPrimaryKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint > ()) , "addingRequiredField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_required_field :: AddingRequiredField > ()) , "banCharField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_char_field :: BanCharField > ()) , "banConcurrentIndexCreationInTransaction" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > ()) , "banDropColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_column :: BanDropColumn > ()) , "banDropDatabase" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_database :: BanDropDatabase > ()) , "banDropNotNull" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_not_null :: BanDropNotNull > ()) , "banDropTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_table :: BanDropTable > ()) , "banTruncateCascade" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_truncate_cascade :: BanTruncateCascade > ()) , "changingColumnType" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: changing_column_type :: ChangingColumnType > ()) , "constraintMissingNotValid" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid > ()) , "creatingEnum" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: creating_enum :: CreatingEnum > ()) , "disallowUniqueConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint > ()) , "lockTimeoutWarning" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: lock_timeout_warning :: LockTimeoutWarning > ()) , "multipleAlterTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: multiple_alter_table :: MultipleAlterTable > ()) , "preferBigInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_big_int :: PreferBigInt > ()) , "preferBigintOverInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_int :: PreferBigintOverInt > ()) , "preferBigintOverSmallint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint > ()) , "preferIdentity" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_identity :: PreferIdentity > ()) , "preferJsonb" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_jsonb :: PreferJsonb > ()) , "preferRobustStmts" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_robust_stmts :: PreferRobustStmts > ()) , "preferTextField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_text_field :: PreferTextField > ()) , "preferTimestamptz" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_timestamptz :: PreferTimestamptz > ()) , "renamingColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_column :: RenamingColumn > ()) , "renamingTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_table :: RenamingTable > ()) , "requireConcurrentIndexCreation" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation > ()) , "requireConcurrentIndexDeletion" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > ()) , "runningStatementWhileHoldingAccessExclusive" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > ()) , "transactionNesting" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: transaction_nesting :: TransactionNesting > ()) , _ => None , } + match key . rule_name () { "addSerialColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: add_serial_column :: AddSerialColumn > ()) , "addingFieldWithDefault" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_field_with_default :: AddingFieldWithDefault > ()) , "addingForeignKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_foreign_key_constraint :: AddingForeignKeyConstraint > ()) , "addingNotNullField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_not_null_field :: AddingNotNullField > ()) , "addingPrimaryKeyConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_primary_key_constraint :: AddingPrimaryKeyConstraint > ()) , "addingRequiredField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: adding_required_field :: AddingRequiredField > ()) , "banAddExclusionConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_add_exclusion_constraint :: BanAddExclusionConstraint > ()) , "banAlterEnumAddValue" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_alter_enum_add_value :: BanAlterEnumAddValue > ()) , "banAttachPartition" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_attach_partition :: BanAttachPartition > ()) , "banBlockingRefreshMatview" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_blocking_refresh_matview :: BanBlockingRefreshMatview > ()) , "banCharField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_char_field :: BanCharField > ()) , "banConcurrentIndexCreationInTransaction" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_concurrent_index_creation_in_transaction :: BanConcurrentIndexCreationInTransaction > ()) , "banCreateTrigger" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_create_trigger :: BanCreateTrigger > ()) , "banDeleteWithoutWhere" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_delete_without_where :: BanDeleteWithoutWhere > ()) , "banDropColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_column :: BanDropColumn > ()) , "banDropDatabase" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_database :: BanDropDatabase > ()) , "banDropNotNull" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_not_null :: BanDropNotNull > ()) , "banDropSchema" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_schema :: BanDropSchema > ()) , "banDropTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_table :: BanDropTable > ()) , "banDropTrigger" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_drop_trigger :: BanDropTrigger > ()) , "banEnableDisableTrigger" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_enable_disable_trigger :: BanEnableDisableTrigger > ()) , "banNotValidValidateSameTransaction" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_not_valid_validate_same_transaction :: BanNotValidValidateSameTransaction > ()) , "banTruncate" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_truncate :: BanTruncate > ()) , "banTruncateCascade" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_truncate_cascade :: BanTruncateCascade > ()) , "banUpdateWithoutWhere" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_update_without_where :: BanUpdateWithoutWhere > ()) , "banVacuumFull" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: ban_vacuum_full :: BanVacuumFull > ()) , "changingColumnType" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: changing_column_type :: ChangingColumnType > ()) , "constraintMissingNotValid" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: constraint_missing_not_valid :: ConstraintMissingNotValid > ()) , "creatingEnum" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: creating_enum :: CreatingEnum > ()) , "disallowUniqueConstraint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: disallow_unique_constraint :: DisallowUniqueConstraint > ()) , "lockTimeoutWarning" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: lock_timeout_warning :: LockTimeoutWarning > ()) , "multipleAlterTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: multiple_alter_table :: MultipleAlterTable > ()) , "preferBigInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_big_int :: PreferBigInt > ()) , "preferBigintOverInt" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_int :: PreferBigintOverInt > ()) , "preferBigintOverSmallint" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_bigint_over_smallint :: PreferBigintOverSmallint > ()) , "preferIdentity" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_identity :: PreferIdentity > ()) , "preferJsonb" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_jsonb :: PreferJsonb > ()) , "preferRobustStmts" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_robust_stmts :: PreferRobustStmts > ()) , "preferTextField" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_text_field :: PreferTextField > ()) , "preferTimestamptz" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: prefer_timestamptz :: PreferTimestamptz > ()) , "renamingColumn" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_column :: RenamingColumn > ()) , "renamingTable" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: renaming_table :: RenamingTable > ()) , "requireConcurrentDetachPartition" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_detach_partition :: RequireConcurrentDetachPartition > ()) , "requireConcurrentIndexCreation" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_creation :: RequireConcurrentIndexCreation > ()) , "requireConcurrentIndexDeletion" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_index_deletion :: RequireConcurrentIndexDeletion > ()) , "requireConcurrentReindex" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_concurrent_reindex :: RequireConcurrentReindex > ()) , "requireIdleInTransactionTimeout" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_idle_in_transaction_timeout :: RequireIdleInTransactionTimeout > ()) , "requireStatementTimeout" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: require_statement_timeout :: RequireStatementTimeout > ()) , "runningStatementWhileHoldingAccessExclusive" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: running_statement_while_holding_access_exclusive :: RunningStatementWhileHoldingAccessExclusive > ()) , "transactionNesting" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: transaction_nesting :: TransactionNesting > ()) , "warnRefreshMatviewConcurrent" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: warn_refresh_matview_concurrent :: WarnRefreshMatviewConcurrent > ()) , "warnWideLockWindow" => Some (crate :: linter_registry :: RegistryLinterRule :: new :: < crate :: lint :: safety :: warn_wide_lock_window :: WarnWideLockWindow > ()) , _ => None , } } diff --git a/crates/pgls_analyser/tests/specs/safety/addingForeignKeyConstraint/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/addingForeignKeyConstraint/basic.sql.snap index 3570851b8..12de73422 100644 --- a/crates/pgls_analyser/tests/specs/safety/addingForeignKeyConstraint/basic.sql.snap +++ b/crates/pgls_analyser/tests/specs/safety/addingForeignKeyConstraint/basic.sql.snap @@ -1,7 +1,6 @@ --- -source: crates/pgt_analyser/tests/rules_tests.rs +source: crates/pgls_analyser/tests/rules_tests.rs expression: snapshot -snapshot_kind: text --- # Input ``` @@ -15,6 +14,6 @@ lint/safety/addingForeignKeyConstraint ━━━━━━━━━━━━━ × Adding a foreign key constraint requires a table scan and locks on both tables. - i This will block writes to both the referencing and referenced tables while PostgreSQL verifies the constraint. + i This will block writes to both the referencing and referenced tables while Postgres verifies the constraint. i Add the constraint as NOT VALID first, then VALIDATE it in a separate transaction. diff --git a/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql new file mode 100644 index 000000000..95e18f757 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banAddExclusionConstraint +alter table my_table add constraint my_excl exclude using gist (col with &&); diff --git a/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql.snap new file mode 100644 index 000000000..acc0b6336 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banAddExclusionConstraint +alter table my_table add constraint my_excl exclude using gist (col with &&); + +``` + +# Diagnostics +lint/safety/banAddExclusionConstraint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock. + + i There is no concurrent alternative for exclusion constraints. Use SET lock_timeout to limit the impact on concurrent operations. diff --git a/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql new file mode 100644 index 000000000..ad4cc0f0c --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +alter table my_table add constraint my_check check (col > 0) not valid; diff --git a/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql.snap b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql.snap new file mode 100644 index 000000000..034a53016 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAddExclusionConstraint/valid_non_exclusion.sql.snap @@ -0,0 +1,10 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +alter table my_table add constraint my_check check (col > 0) not valid; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql b/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql new file mode 100644 index 000000000..4096036a2 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banAlterEnumAddValue +alter type my_enum add value 'new_value'; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql.snap new file mode 100644 index 000000000..830fc62e7 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAlterEnumAddValue/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banAlterEnumAddValue +alter type my_enum add value 'new_value'; +``` + +# Diagnostics +lint/safety/banAlterEnumAddValue ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × ALTER TYPE ... ADD VALUE cannot be used in a transaction block before Postgres 12. + + i On Postgres 12+, the operation is fast but the new value cannot be used in the same transaction until committed. diff --git a/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql b/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql new file mode 100644 index 000000000..66327392e --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banAttachPartition +alter table my_table attach partition my_partition for values from ('2024-01-01') to ('2025-01-01'); \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql.snap new file mode 100644 index 000000000..733fbea54 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banAttachPartition/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banAttachPartition +alter table my_table attach partition my_partition for values from ('2024-01-01') to ('2025-01-01'); +``` + +# Diagnostics +lint/safety/banAttachPartition ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table. + + i This blocks all reads and writes on the parent table. Consider adding a matching CHECK constraint to the child table before attaching to minimize lock duration. diff --git a/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql new file mode 100644 index 000000000..7d8abd609 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banBlockingRefreshMatview +refresh materialized view my_view; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql.snap new file mode 100644 index 000000000..f03bec5bf --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banBlockingRefreshMatview +refresh materialized view my_view; +``` + +# Diagnostics +lint/safety/banBlockingRefreshMatview ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × REFRESH MATERIALIZED VIEW without CONCURRENTLY blocks all reads. + + i Use REFRESH MATERIALIZED VIEW CONCURRENTLY to allow reads during the refresh. This requires a unique index on the view. diff --git a/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql new file mode 100644 index 000000000..c12f994df --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +refresh materialized view concurrently my_view; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql.snap b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql.snap new file mode 100644 index 000000000..4f8fe1636 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banBlockingRefreshMatview/valid_concurrent.sql.snap @@ -0,0 +1,9 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +refresh materialized view concurrently my_view; +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql b/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql new file mode 100644 index 000000000..1fd8c0d0c --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banCreateTrigger +create trigger my_trigger after insert on my_table for each row execute function my_func(); \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql.snap new file mode 100644 index 000000000..c93755ddd --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banCreateTrigger/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banCreateTrigger +create trigger my_trigger after insert on my_table for each row execute function my_func(); +``` + +# Diagnostics +lint/safety/banCreateTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table. + + i Triggers add hidden complexity and can block concurrent writes. Consider using application-level logic instead. diff --git a/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql new file mode 100644 index 000000000..0cda88341 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banDeleteWithoutWhere +delete from my_table; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql.snap new file mode 100644 index 000000000..41407fb87 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banDeleteWithoutWhere +delete from my_table; +``` + +# Diagnostics +lint/safety/banDeleteWithoutWhere ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × A DELETE without a WHERE clause will remove all rows from the table. + + i Add a WHERE clause to limit which rows are deleted. diff --git a/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql new file mode 100644 index 000000000..3593f34d0 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +delete from my_table where id = 1; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql.snap b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql.snap new file mode 100644 index 000000000..8d8e7d2b3 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDeleteWithoutWhere/valid_with_where.sql.snap @@ -0,0 +1,9 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +delete from my_table where id = 1; +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql b/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql new file mode 100644 index 000000000..2783fef47 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banDropSchema +drop schema my_schema; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql.snap new file mode 100644 index 000000000..e1852935d --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDropSchema/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banDropSchema +drop schema my_schema; +``` + +# Diagnostics +lint/safety/banDropSchema ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Dropping a schema will remove all objects within it and may break existing clients. + + i Remove objects individually instead, or ensure all dependent applications have been updated. diff --git a/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql b/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql new file mode 100644 index 000000000..5f10ae248 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banDropTrigger +drop trigger my_trigger on my_table; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql.snap new file mode 100644 index 000000000..c2fb90e24 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banDropTrigger/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banDropTrigger +drop trigger my_trigger on my_table; +``` + +# Diagnostics +lint/safety/banDropTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table. + + i This blocks all reads and writes. Ensure no application logic depends on the trigger before dropping it. diff --git a/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql new file mode 100644 index 000000000..76cc11895 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banEnableDisableTrigger +alter table my_table enable trigger my_trigger; diff --git a/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql.snap new file mode 100644 index 000000000..878433522 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banEnableDisableTrigger +alter table my_table enable trigger my_trigger; + +``` + +# Diagnostics +lint/safety/banEnableDisableTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock. + + i This blocks concurrent writes. Consider the impact on busy tables and use SET lock_timeout. diff --git a/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql new file mode 100644 index 000000000..cf1b35d56 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banEnableDisableTrigger +alter table my_table disable trigger my_trigger; diff --git a/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql.snap b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql.snap new file mode 100644 index 000000000..9d0ea5963 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banEnableDisableTrigger/disable.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banEnableDisableTrigger +alter table my_table disable trigger my_trigger; + +``` + +# Diagnostics +lint/safety/banEnableDisableTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock. + + i This blocks concurrent writes. Consider the impact on busy tables and use SET lock_timeout. diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql new file mode 100644 index 000000000..68f511cff --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql @@ -0,0 +1,3 @@ +-- expect_lint/safety/banNotValidValidateSameTransaction +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; +ALTER TABLE orders VALIDATE CONSTRAINT orders_check; diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql.snap new file mode 100644 index 000000000..b621493c1 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/basic.sql.snap @@ -0,0 +1,18 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banNotValidValidateSameTransaction +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; +ALTER TABLE orders VALIDATE CONSTRAINT orders_check; + +``` + +# Diagnostics +lint/safety/banNotValidValidateSameTransaction ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Constraint orders_check was added as NOT VALID and validated in the same transaction. + + i Run VALIDATE CONSTRAINT in a separate transaction to avoid holding locks during validation. diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql new file mode 100644 index 000000000..46f4042b6 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql @@ -0,0 +1,3 @@ +-- expect_no_diagnostics +ALTER TABLE users ADD CONSTRAINT check_positive CHECK (amount > 0) NOT VALID; +ALTER TABLE orders VALIDATE CONSTRAINT check_positive; diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql.snap b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql.snap new file mode 100644 index 000000000..742e14251 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_different_tables.sql.snap @@ -0,0 +1,11 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE users ADD CONSTRAINT check_positive CHECK (amount > 0) NOT VALID; +ALTER TABLE orders VALIDATE CONSTRAINT check_positive; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql new file mode 100644 index 000000000..6d53f1ea3 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql.snap b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql.snap new file mode 100644 index 000000000..cf4bdcc87 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_no_validate.sql.snap @@ -0,0 +1,10 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql new file mode 100644 index 000000000..9eb257e62 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql @@ -0,0 +1,4 @@ +-- expect_no_diagnostics +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; +COMMIT; +ALTER TABLE orders VALIDATE CONSTRAINT orders_check; diff --git a/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql.snap b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql.snap new file mode 100644 index 000000000..559548bd1 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banNotValidValidateSameTransaction/valid_separate_transaction.sql.snap @@ -0,0 +1,12 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; +COMMIT; +ALTER TABLE orders VALIDATE CONSTRAINT orders_check; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql b/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql new file mode 100644 index 000000000..adebe310d --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banTruncate +truncate my_table; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql.snap new file mode 100644 index 000000000..733e9b33f --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banTruncate/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banTruncate +truncate my_table; +``` + +# Diagnostics +lint/safety/banTruncate ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Truncating a table removes all rows and can cause data loss. + + i Use DELETE with a WHERE clause instead, or ensure this is intentional and not part of a migration. diff --git a/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql new file mode 100644 index 000000000..3b232bfc8 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banUpdateWithoutWhere +update my_table set col = 'value'; diff --git a/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql.snap new file mode 100644 index 000000000..112c1c1ec --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banUpdateWithoutWhere +update my_table set col = 'value'; + +``` + +# Diagnostics +lint/safety/banUpdateWithoutWhere ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × An UPDATE without a WHERE clause will modify all rows in the table. + + i Add a WHERE clause to limit which rows are updated. diff --git a/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql new file mode 100644 index 000000000..7549aa8d8 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +update my_table set col = 'value' where id = 1; diff --git a/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql.snap b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql.snap new file mode 100644 index 000000000..66e3fbec2 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banUpdateWithoutWhere/valid_with_where.sql.snap @@ -0,0 +1,10 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +update my_table set col = 'value' where id = 1; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql new file mode 100644 index 000000000..6813d0bdb --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/banVacuumFull +vacuum full my_table; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql.snap new file mode 100644 index 000000000..bf51ab9db --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/banVacuumFull +vacuum full my_table; +``` + +# Diagnostics +lint/safety/banVacuumFull ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × VACUUM FULL rewrites the entire table and blocks all access. + + i Use regular VACUUM or pg_repack for online table maintenance without blocking reads and writes. diff --git a/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql new file mode 100644 index 000000000..46a9a9c27 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +vacuum my_table; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql.snap b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql.snap new file mode 100644 index 000000000..fa03ed133 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/banVacuumFull/valid_regular_vacuum.sql.snap @@ -0,0 +1,9 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +vacuum my_table; +``` diff --git a/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql b/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql index 5e5c2fc38..9ebd690ee 100644 --- a/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql +++ b/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql @@ -1,2 +1,2 @@ -- expect_lint/safety/changingColumnType -ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text USING "edits"::text; \ No newline at end of file +ALTER TABLE "core_recipe" ALTER COLUMN "count" TYPE bigint; diff --git a/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql.snap index 5739621a7..994ae18f6 100644 --- a/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql.snap +++ b/crates/pgls_analyser/tests/specs/safety/changingColumnType/basic.sql.snap @@ -1,12 +1,12 @@ --- -source: crates/pgt_analyser/tests/rules_tests.rs +source: crates/pgls_analyser/tests/rules_tests.rs expression: snapshot -snapshot_kind: text --- # Input ``` -- expect_lint/safety/changingColumnType -ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text USING "edits"::text; +ALTER TABLE "core_recipe" ALTER COLUMN "count" TYPE bigint; + ``` # Diagnostics diff --git a/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql b/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql new file mode 100644 index 000000000..1404d78bc --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql @@ -0,0 +1,4 @@ +-- expect_no_diagnostics +ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text; +ALTER TABLE "core_recipe" ALTER COLUMN "name" TYPE varchar; +ALTER TABLE "core_recipe" ALTER COLUMN "amount" TYPE numeric; diff --git a/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql.snap b/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql.snap new file mode 100644 index 000000000..6a1a9ae1b --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/changingColumnType/safe_widening.sql.snap @@ -0,0 +1,12 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text; +ALTER TABLE "core_recipe" ALTER COLUMN "name" TYPE varchar; +ALTER TABLE "core_recipe" ALTER COLUMN "amount" TYPE numeric; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql new file mode 100644 index 000000000..10899750f --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireConcurrentDetachPartition +alter table my_table detach partition my_partition; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql.snap new file mode 100644 index 000000000..e50a48fc9 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireConcurrentDetachPartition +alter table my_table detach partition my_partition; +``` + +# Diagnostics +lint/safety/requireConcurrentDetachPartition ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Detaching a partition without CONCURRENTLY blocks all table access. + + i Use DETACH PARTITION ... CONCURRENTLY (Postgres 14+) to avoid blocking reads and writes. diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql new file mode 100644 index 000000000..f881cf048 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +alter table my_table detach partition my_partition concurrently; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql.snap new file mode 100644 index 000000000..318ad1a93 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentDetachPartition/valid_concurrent.sql.snap @@ -0,0 +1,9 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +alter table my_table detach partition my_partition concurrently; +``` diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql new file mode 100644 index 000000000..948545da2 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireConcurrentReindex +reindex index my_index; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql.snap new file mode 100644 index 000000000..3406ea1a3 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/basic.sql.snap @@ -0,0 +1,16 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireConcurrentReindex +reindex index my_index; +``` + +# Diagnostics +lint/safety/requireConcurrentReindex ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × REINDEX without CONCURRENTLY blocks all table access. + + i Use REINDEX CONCURRENTLY to rebuild the index without blocking reads and writes. diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql new file mode 100644 index 000000000..d1a759995 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +reindex index concurrently my_index; \ No newline at end of file diff --git a/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql.snap new file mode 100644 index 000000000..d866b79c1 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireConcurrentReindex/valid_concurrent.sql.snap @@ -0,0 +1,9 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +reindex index concurrently my_index; +``` diff --git a/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql new file mode 100644 index 000000000..d70b7d9a7 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireIdleInTransactionTimeout +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql.snap new file mode 100644 index 000000000..22c99a838 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireIdleInTransactionTimeout +ALTER TABLE users ADD COLUMN email TEXT; + +``` + +# Diagnostics +lint/safety/requireIdleInTransactionTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without idle_in_transaction_session_timeout set. + + i Run SET idle_in_transaction_session_timeout = '...' before this statement to prevent idle transactions from holding locks. diff --git a/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql new file mode 100644 index 000000000..61a990b96 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql @@ -0,0 +1,3 @@ +-- expect_no_diagnostics +SET idle_in_transaction_session_timeout = '30s'; +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql.snap new file mode 100644 index 000000000..8b80d1cd9 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireIdleInTransactionTimeout/valid_with_timeout.sql.snap @@ -0,0 +1,11 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +SET idle_in_transaction_session_timeout = '30s'; +ALTER TABLE users ADD COLUMN email TEXT; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql new file mode 100644 index 000000000..da0fd8c00 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireStatementTimeout +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql.snap new file mode 100644 index 000000000..8fbe4bff9 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +ALTER TABLE users ADD COLUMN email TEXT; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql new file mode 100644 index 000000000..95e1a9914 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireStatementTimeout +DROP TABLE users; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql.snap new file mode 100644 index 000000000..c1bdb4859 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/drop_table.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +DROP TABLE users; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql new file mode 100644 index 000000000..c9cd40d9c --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireStatementTimeout +REFRESH MATERIALIZED VIEW my_view; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql.snap new file mode 100644 index 000000000..83b82f878 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/refresh_matview.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +REFRESH MATERIALIZED VIEW my_view; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql new file mode 100644 index 000000000..fba87dc14 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireStatementTimeout +REINDEX INDEX my_index; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql.snap new file mode 100644 index 000000000..1390c2605 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reindex.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +REINDEX INDEX my_index; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql new file mode 100644 index 000000000..477920f49 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql @@ -0,0 +1,4 @@ +-- expect_lint/safety/requireStatementTimeout +SET statement_timeout = '5s'; +COMMIT; +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql.snap new file mode 100644 index 000000000..6d818270d --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/reset_after_commit.sql.snap @@ -0,0 +1,19 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +SET statement_timeout = '5s'; +COMMIT; +ALTER TABLE users ADD COLUMN email TEXT; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql new file mode 100644 index 000000000..e28079461 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/requireStatementTimeout +TRUNCATE my_table; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql.snap new file mode 100644 index 000000000..6b9e59eb6 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/truncate.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/requireStatementTimeout +TRUNCATE my_table; + +``` + +# Diagnostics +lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Statement takes a dangerous lock without a statement_timeout set. + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql new file mode 100644 index 000000000..29ac232c3 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql @@ -0,0 +1,3 @@ +-- expect_no_diagnostics +SET statement_timeout = '5s'; +ALTER TABLE users ADD COLUMN email TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql.snap b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql.snap new file mode 100644 index 000000000..f383902d4 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/requireStatementTimeout/valid_with_timeout.sql.snap @@ -0,0 +1,11 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +SET statement_timeout = '5s'; +ALTER TABLE users ADD COLUMN email TEXT; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql new file mode 100644 index 000000000..27ecb03bf --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql @@ -0,0 +1,2 @@ +-- expect_lint/safety/warnRefreshMatviewConcurrent +refresh materialized view concurrently my_view; diff --git a/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql.snap new file mode 100644 index 000000000..824053e98 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/basic.sql.snap @@ -0,0 +1,17 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/warnRefreshMatviewConcurrent +refresh materialized view concurrently my_view; + +``` + +# Diagnostics +lint/safety/warnRefreshMatviewConcurrent ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock. + + i Concurrent refresh allows reads but still blocks DDL and writes. Consider the impact on long-running refreshes. diff --git a/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql new file mode 100644 index 000000000..7cb4eef12 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql @@ -0,0 +1,2 @@ +-- expect_no_diagnostics +select 1; diff --git a/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql.snap b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql.snap new file mode 100644 index 000000000..3ecfcb613 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnRefreshMatviewConcurrent/valid_non_concurrent.sql.snap @@ -0,0 +1,10 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +select 1; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql new file mode 100644 index 000000000..9165aa62a --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql @@ -0,0 +1,3 @@ +-- expect_lint/safety/warnWideLockWindow +ALTER TABLE users ADD COLUMN email TEXT; +ALTER TABLE orders ADD COLUMN total NUMERIC; diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql.snap b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql.snap new file mode 100644 index 000000000..346d35a17 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/basic.sql.snap @@ -0,0 +1,18 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_lint/safety/warnWideLockWindow +ALTER TABLE users ADD COLUMN email TEXT; +ALTER TABLE orders ADD COLUMN total NUMERIC; + +``` + +# Diagnostics +lint/safety/warnWideLockWindow ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Acquiring a lock on public.orders while already holding ACCESS EXCLUSIVE locks on other tables. + + i This widens the lock window. Split into separate transactions to minimize lock duration. diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql new file mode 100644 index 000000000..085997a68 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql @@ -0,0 +1,3 @@ +-- expect_no_diagnostics +ALTER TABLE users ADD COLUMN email TEXT; +ALTER TABLE users ADD COLUMN name TEXT; diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql.snap b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql.snap new file mode 100644 index 000000000..51051d54a --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_same_table.sql.snap @@ -0,0 +1,11 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE users ADD COLUMN email TEXT; +ALTER TABLE users ADD COLUMN name TEXT; + +``` diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql new file mode 100644 index 000000000..eb54e856a --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql @@ -0,0 +1,4 @@ +-- expect_no_diagnostics +ALTER TABLE users ADD COLUMN email TEXT; +COMMIT; +ALTER TABLE orders ADD COLUMN total NUMERIC; diff --git a/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql.snap b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql.snap new file mode 100644 index 000000000..8c6f61687 --- /dev/null +++ b/crates/pgls_analyser/tests/specs/safety/warnWideLockWindow/valid_separate_transactions.sql.snap @@ -0,0 +1,12 @@ +--- +source: crates/pgls_analyser/tests/rules_tests.rs +expression: snapshot +--- +# Input +``` +-- expect_no_diagnostics +ALTER TABLE users ADD COLUMN email TEXT; +COMMIT; +ALTER TABLE orders ADD COLUMN total NUMERIC; + +``` diff --git a/crates/pgls_configuration/src/linter/rules.rs b/crates/pgls_configuration/src/linter/rules.rs index 8b5fb768b..f2cf44bf9 100644 --- a/crates/pgls_configuration/src/linter/rules.rs +++ b/crates/pgls_configuration/src/linter/rules.rs @@ -167,6 +167,21 @@ pub struct Safety { #[serde(skip_serializing_if = "Option::is_none")] pub adding_required_field: Option>, + #[doc = "Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_add_exclusion_constraint: + Option>, + #[doc = "ALTER TYPE ... ADD VALUE cannot run inside a transaction block in older Postgres versions."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_alter_enum_add_value: + Option>, + #[doc = "Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_attach_partition: Option>, + #[doc = "REFRESH MATERIALIZED VIEW without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_blocking_refresh_matview: + Option>, #[doc = "Using CHAR(n) or CHARACTER(n) types is discouraged."] #[serde(skip_serializing_if = "Option::is_none")] pub ban_char_field: Option>, @@ -174,6 +189,13 @@ pub struct Safety { #[serde(skip_serializing_if = "Option::is_none")] pub ban_concurrent_index_creation_in_transaction: Option>, + #[doc = "Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_create_trigger: Option>, + #[doc = "A DELETE statement without a WHERE clause will remove all rows from the table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_delete_without_where: + Option>, #[doc = "Dropping a column may break existing clients."] #[serde(skip_serializing_if = "Option::is_none")] pub ban_drop_column: Option>, @@ -183,13 +205,37 @@ pub struct Safety { #[doc = "Dropping a NOT NULL constraint may break existing clients."] #[serde(skip_serializing_if = "Option::is_none")] pub ban_drop_not_null: Option>, + #[doc = "Dropping a schema will remove all objects within it and may break existing clients."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_drop_schema: Option>, #[doc = "Dropping a table may break existing clients."] #[serde(skip_serializing_if = "Option::is_none")] pub ban_drop_table: Option>, + #[doc = "Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_drop_trigger: Option>, + #[doc = "Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_enable_disable_trigger: + Option>, + #[doc = "Validating a constraint in the same transaction it was added as NOT VALID defeats the purpose."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_not_valid_validate_same_transaction: + Option>, + #[doc = "Truncating a table removes all rows and can cause data loss in production."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_truncate: Option>, #[doc = "Using TRUNCATE's CASCADE option will truncate any tables that are also foreign-keyed to the specified tables."] #[serde(skip_serializing_if = "Option::is_none")] pub ban_truncate_cascade: Option>, - #[doc = "Changing a column type may break existing clients."] + #[doc = "An UPDATE statement without a WHERE clause will modify all rows in the table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_update_without_where: + Option>, + #[doc = "VACUUM FULL rewrites the entire table and acquires an ACCESS EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub ban_vacuum_full: Option>, + #[doc = "Changing a column type may require a table rewrite and break existing clients."] #[serde(skip_serializing_if = "Option::is_none")] pub changing_column_type: Option>, #[doc = "Adding constraints without NOT VALID blocks all reads and writes."] @@ -241,6 +287,10 @@ pub struct Safety { #[doc = "Renaming tables may break existing queries and application code."] #[serde(skip_serializing_if = "Option::is_none")] pub renaming_table: Option>, + #[doc = "Detaching a partition without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub require_concurrent_detach_partition: + Option>, #[doc = "Creating indexes non-concurrently can lock the table for writes."] #[serde(skip_serializing_if = "Option::is_none")] pub require_concurrent_index_creation: @@ -249,6 +299,18 @@ pub struct Safety { #[serde(skip_serializing_if = "Option::is_none")] pub require_concurrent_index_deletion: Option>, + #[doc = "REINDEX without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock on the table."] + #[serde(skip_serializing_if = "Option::is_none")] + pub require_concurrent_reindex: + Option>, + #[doc = "Dangerous lock statements should be preceded by SET idle_in_transaction_session_timeout."] + #[serde(skip_serializing_if = "Option::is_none")] + pub require_idle_in_transaction_timeout: + Option>, + #[doc = "Dangerous lock statements should be preceded by SET statement_timeout."] + #[serde(skip_serializing_if = "Option::is_none")] + pub require_statement_timeout: + Option>, #[doc = "Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access."] #[serde(skip_serializing_if = "Option::is_none")] pub running_statement_while_holding_access_exclusive: Option< @@ -257,6 +319,14 @@ pub struct Safety { #[doc = "Detects problematic transaction nesting that could lead to unexpected behavior."] #[serde(skip_serializing_if = "Option::is_none")] pub transaction_nesting: Option>, + #[doc = "REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock."] + #[serde(skip_serializing_if = "Option::is_none")] + pub warn_refresh_matview_concurrent: + Option>, + #[doc = "Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window."] + #[serde(skip_serializing_if = "Option::is_none")] + pub warn_wide_lock_window: + Option>, } impl Safety { const GROUP_NAME: &'static str = "safety"; @@ -267,13 +337,26 @@ impl Safety { "addingNotNullField", "addingPrimaryKeyConstraint", "addingRequiredField", + "banAddExclusionConstraint", + "banAlterEnumAddValue", + "banAttachPartition", + "banBlockingRefreshMatview", "banCharField", "banConcurrentIndexCreationInTransaction", + "banCreateTrigger", + "banDeleteWithoutWhere", "banDropColumn", "banDropDatabase", "banDropNotNull", + "banDropSchema", "banDropTable", + "banDropTrigger", + "banEnableDisableTrigger", + "banNotValidValidateSameTransaction", + "banTruncate", "banTruncateCascade", + "banUpdateWithoutWhere", + "banVacuumFull", "changingColumnType", "constraintMissingNotValid", "creatingEnum", @@ -290,10 +373,16 @@ impl Safety { "preferTimestamptz", "renamingColumn", "renamingTable", + "requireConcurrentDetachPartition", "requireConcurrentIndexCreation", "requireConcurrentIndexDeletion", + "requireConcurrentReindex", + "requireIdleInTransactionTimeout", + "requireStatementTimeout", "runningStatementWhileHoldingAccessExclusive", "transactionNesting", + "warnRefreshMatviewConcurrent", + "warnWideLockWindow", ]; const RECOMMENDED_RULES_AS_FILTERS: &'static [RuleFilter<'static>] = &[ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), @@ -301,13 +390,25 @@ impl Safety { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[2]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[3]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[4]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8]), - RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[48]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[51]), ]; const ALL_RULES_AS_FILTERS: &'static [RuleFilter<'static>] = &[ RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[0]), @@ -343,6 +444,25 @@ impl Safety { RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31]), RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[36]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[47]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[48]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[49]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[50]), + RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[51]), ]; #[doc = r" Retrieves the recommended rules"] pub(crate) fn is_recommended_true(&self) -> bool { @@ -389,142 +509,237 @@ impl Safety { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5])); } } - if let Some(rule) = self.ban_char_field.as_ref() { + if let Some(rule) = self.ban_add_exclusion_constraint.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6])); } } - if let Some(rule) = self.ban_concurrent_index_creation_in_transaction.as_ref() { + if let Some(rule) = self.ban_alter_enum_add_value.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.ban_drop_column.as_ref() { + if let Some(rule) = self.ban_attach_partition.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.ban_drop_database.as_ref() { + if let Some(rule) = self.ban_blocking_refresh_matview.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.ban_drop_not_null.as_ref() { + if let Some(rule) = self.ban_char_field.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.ban_drop_table.as_ref() { + if let Some(rule) = self.ban_concurrent_index_creation_in_transaction.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.ban_truncate_cascade.as_ref() { + if let Some(rule) = self.ban_create_trigger.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.changing_column_type.as_ref() { + if let Some(rule) = self.ban_delete_without_where.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.constraint_missing_not_valid.as_ref() { + if let Some(rule) = self.ban_drop_column.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.creating_enum.as_ref() { + if let Some(rule) = self.ban_drop_database.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.disallow_unique_constraint.as_ref() { + if let Some(rule) = self.ban_drop_not_null.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.lock_timeout_warning.as_ref() { + if let Some(rule) = self.ban_drop_schema.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.multiple_alter_table.as_ref() { + if let Some(rule) = self.ban_drop_table.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.prefer_big_int.as_ref() { + if let Some(rule) = self.ban_drop_trigger.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.prefer_bigint_over_int.as_ref() { + if let Some(rule) = self.ban_enable_disable_trigger.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.prefer_bigint_over_smallint.as_ref() { + if let Some(rule) = self.ban_not_valid_validate_same_transaction.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.prefer_identity.as_ref() { + if let Some(rule) = self.ban_truncate.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.prefer_jsonb.as_ref() { + if let Some(rule) = self.ban_truncate_cascade.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.prefer_robust_stmts.as_ref() { + if let Some(rule) = self.ban_update_without_where.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.prefer_text_field.as_ref() { + if let Some(rule) = self.ban_vacuum_full.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.prefer_timestamptz.as_ref() { + if let Some(rule) = self.changing_column_type.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.renaming_column.as_ref() { + if let Some(rule) = self.constraint_missing_not_valid.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.renaming_table.as_ref() { + if let Some(rule) = self.creating_enum.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.require_concurrent_index_creation.as_ref() { + if let Some(rule) = self.disallow_unique_constraint.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.require_concurrent_index_deletion.as_ref() { + if let Some(rule) = self.lock_timeout_warning.as_ref() { if rule.is_enabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } + if let Some(rule) = self.multiple_alter_table.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); + } + } + if let Some(rule) = self.prefer_big_int.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + } + } + if let Some(rule) = self.prefer_bigint_over_int.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); + } + } + if let Some(rule) = self.prefer_bigint_over_smallint.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); + } + } + if let Some(rule) = self.prefer_identity.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35])); + } + } + if let Some(rule) = self.prefer_jsonb.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[36])); + } + } + if let Some(rule) = self.prefer_robust_stmts.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37])); + } + } + if let Some(rule) = self.prefer_text_field.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38])); + } + } + if let Some(rule) = self.prefer_timestamptz.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39])); + } + } + if let Some(rule) = self.renaming_column.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40])); + } + } + if let Some(rule) = self.renaming_table.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41])); + } + } + if let Some(rule) = self.require_concurrent_detach_partition.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42])); + } + } + if let Some(rule) = self.require_concurrent_index_creation.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43])); + } + } + if let Some(rule) = self.require_concurrent_index_deletion.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44])); + } + } + if let Some(rule) = self.require_concurrent_reindex.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45])); + } + } + if let Some(rule) = self.require_idle_in_transaction_timeout.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46])); + } + } + if let Some(rule) = self.require_statement_timeout.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[47])); + } + } if let Some(rule) = self .running_statement_while_holding_access_exclusive .as_ref() { if rule.is_enabled() { - index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[48])); } } if let Some(rule) = self.transaction_nesting.as_ref() { if rule.is_enabled() { - index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[49])); + } + } + if let Some(rule) = self.warn_refresh_matview_concurrent.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[50])); + } + } + if let Some(rule) = self.warn_wide_lock_window.as_ref() { + if rule.is_enabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[51])); } } index_set @@ -561,142 +776,237 @@ impl Safety { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[5])); } } - if let Some(rule) = self.ban_char_field.as_ref() { + if let Some(rule) = self.ban_add_exclusion_constraint.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[6])); } } - if let Some(rule) = self.ban_concurrent_index_creation_in_transaction.as_ref() { + if let Some(rule) = self.ban_alter_enum_add_value.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[7])); } } - if let Some(rule) = self.ban_drop_column.as_ref() { + if let Some(rule) = self.ban_attach_partition.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[8])); } } - if let Some(rule) = self.ban_drop_database.as_ref() { + if let Some(rule) = self.ban_blocking_refresh_matview.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[9])); } } - if let Some(rule) = self.ban_drop_not_null.as_ref() { + if let Some(rule) = self.ban_char_field.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[10])); } } - if let Some(rule) = self.ban_drop_table.as_ref() { + if let Some(rule) = self.ban_concurrent_index_creation_in_transaction.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[11])); } } - if let Some(rule) = self.ban_truncate_cascade.as_ref() { + if let Some(rule) = self.ban_create_trigger.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[12])); } } - if let Some(rule) = self.changing_column_type.as_ref() { + if let Some(rule) = self.ban_delete_without_where.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[13])); } } - if let Some(rule) = self.constraint_missing_not_valid.as_ref() { + if let Some(rule) = self.ban_drop_column.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[14])); } } - if let Some(rule) = self.creating_enum.as_ref() { + if let Some(rule) = self.ban_drop_database.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[15])); } } - if let Some(rule) = self.disallow_unique_constraint.as_ref() { + if let Some(rule) = self.ban_drop_not_null.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[16])); } } - if let Some(rule) = self.lock_timeout_warning.as_ref() { + if let Some(rule) = self.ban_drop_schema.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[17])); } } - if let Some(rule) = self.multiple_alter_table.as_ref() { + if let Some(rule) = self.ban_drop_table.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[18])); } } - if let Some(rule) = self.prefer_big_int.as_ref() { + if let Some(rule) = self.ban_drop_trigger.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[19])); } } - if let Some(rule) = self.prefer_bigint_over_int.as_ref() { + if let Some(rule) = self.ban_enable_disable_trigger.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[20])); } } - if let Some(rule) = self.prefer_bigint_over_smallint.as_ref() { + if let Some(rule) = self.ban_not_valid_validate_same_transaction.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[21])); } } - if let Some(rule) = self.prefer_identity.as_ref() { + if let Some(rule) = self.ban_truncate.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[22])); } } - if let Some(rule) = self.prefer_jsonb.as_ref() { + if let Some(rule) = self.ban_truncate_cascade.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[23])); } } - if let Some(rule) = self.prefer_robust_stmts.as_ref() { + if let Some(rule) = self.ban_update_without_where.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[24])); } } - if let Some(rule) = self.prefer_text_field.as_ref() { + if let Some(rule) = self.ban_vacuum_full.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[25])); } } - if let Some(rule) = self.prefer_timestamptz.as_ref() { + if let Some(rule) = self.changing_column_type.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[26])); } } - if let Some(rule) = self.renaming_column.as_ref() { + if let Some(rule) = self.constraint_missing_not_valid.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[27])); } } - if let Some(rule) = self.renaming_table.as_ref() { + if let Some(rule) = self.creating_enum.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[28])); } } - if let Some(rule) = self.require_concurrent_index_creation.as_ref() { + if let Some(rule) = self.disallow_unique_constraint.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[29])); } } - if let Some(rule) = self.require_concurrent_index_deletion.as_ref() { + if let Some(rule) = self.lock_timeout_warning.as_ref() { if rule.is_disabled() { index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[30])); } } + if let Some(rule) = self.multiple_alter_table.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); + } + } + if let Some(rule) = self.prefer_big_int.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + } + } + if let Some(rule) = self.prefer_bigint_over_int.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[33])); + } + } + if let Some(rule) = self.prefer_bigint_over_smallint.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[34])); + } + } + if let Some(rule) = self.prefer_identity.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[35])); + } + } + if let Some(rule) = self.prefer_jsonb.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[36])); + } + } + if let Some(rule) = self.prefer_robust_stmts.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[37])); + } + } + if let Some(rule) = self.prefer_text_field.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[38])); + } + } + if let Some(rule) = self.prefer_timestamptz.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[39])); + } + } + if let Some(rule) = self.renaming_column.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[40])); + } + } + if let Some(rule) = self.renaming_table.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[41])); + } + } + if let Some(rule) = self.require_concurrent_detach_partition.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[42])); + } + } + if let Some(rule) = self.require_concurrent_index_creation.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[43])); + } + } + if let Some(rule) = self.require_concurrent_index_deletion.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[44])); + } + } + if let Some(rule) = self.require_concurrent_reindex.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[45])); + } + } + if let Some(rule) = self.require_idle_in_transaction_timeout.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[46])); + } + } + if let Some(rule) = self.require_statement_timeout.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[47])); + } + } if let Some(rule) = self .running_statement_while_holding_access_exclusive .as_ref() { if rule.is_disabled() { - index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[31])); + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[48])); } } if let Some(rule) = self.transaction_nesting.as_ref() { if rule.is_disabled() { - index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[32])); + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[49])); + } + } + if let Some(rule) = self.warn_refresh_matview_concurrent.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[50])); + } + } + if let Some(rule) = self.warn_wide_lock_window.as_ref() { + if rule.is_disabled() { + index_set.insert(RuleFilter::Rule(Self::GROUP_NAME, Self::GROUP_RULES[51])); } } index_set @@ -734,13 +1044,26 @@ impl Safety { "addingNotNullField" => Severity::Warning, "addingPrimaryKeyConstraint" => Severity::Warning, "addingRequiredField" => Severity::Error, + "banAddExclusionConstraint" => Severity::Warning, + "banAlterEnumAddValue" => Severity::Warning, + "banAttachPartition" => Severity::Warning, + "banBlockingRefreshMatview" => Severity::Warning, "banCharField" => Severity::Warning, "banConcurrentIndexCreationInTransaction" => Severity::Error, + "banCreateTrigger" => Severity::Warning, + "banDeleteWithoutWhere" => Severity::Warning, "banDropColumn" => Severity::Warning, "banDropDatabase" => Severity::Warning, "banDropNotNull" => Severity::Warning, + "banDropSchema" => Severity::Error, "banDropTable" => Severity::Warning, + "banDropTrigger" => Severity::Warning, + "banEnableDisableTrigger" => Severity::Warning, + "banNotValidValidateSameTransaction" => Severity::Error, + "banTruncate" => Severity::Error, "banTruncateCascade" => Severity::Error, + "banUpdateWithoutWhere" => Severity::Warning, + "banVacuumFull" => Severity::Error, "changingColumnType" => Severity::Warning, "constraintMissingNotValid" => Severity::Warning, "creatingEnum" => Severity::Warning, @@ -757,10 +1080,16 @@ impl Safety { "preferTimestamptz" => Severity::Warning, "renamingColumn" => Severity::Warning, "renamingTable" => Severity::Warning, + "requireConcurrentDetachPartition" => Severity::Warning, "requireConcurrentIndexCreation" => Severity::Warning, "requireConcurrentIndexDeletion" => Severity::Warning, + "requireConcurrentReindex" => Severity::Warning, + "requireIdleInTransactionTimeout" => Severity::Warning, + "requireStatementTimeout" => Severity::Warning, "runningStatementWhileHoldingAccessExclusive" => Severity::Warning, "transactionNesting" => Severity::Warning, + "warnRefreshMatviewConcurrent" => Severity::Warning, + "warnWideLockWindow" => Severity::Warning, _ => unreachable!(), } } @@ -793,6 +1122,22 @@ impl Safety { .adding_required_field .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "banAddExclusionConstraint" => self + .ban_add_exclusion_constraint + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banAlterEnumAddValue" => self + .ban_alter_enum_add_value + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banAttachPartition" => self + .ban_attach_partition + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banBlockingRefreshMatview" => self + .ban_blocking_refresh_matview + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "banCharField" => self .ban_char_field .as_ref() @@ -801,6 +1146,14 @@ impl Safety { .ban_concurrent_index_creation_in_transaction .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "banCreateTrigger" => self + .ban_create_trigger + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banDeleteWithoutWhere" => self + .ban_delete_without_where + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "banDropColumn" => self .ban_drop_column .as_ref() @@ -813,14 +1166,42 @@ impl Safety { .ban_drop_not_null .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "banDropSchema" => self + .ban_drop_schema + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "banDropTable" => self .ban_drop_table .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "banDropTrigger" => self + .ban_drop_trigger + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banEnableDisableTrigger" => self + .ban_enable_disable_trigger + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banNotValidValidateSameTransaction" => self + .ban_not_valid_validate_same_transaction + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banTruncate" => self + .ban_truncate + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "banTruncateCascade" => self .ban_truncate_cascade .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "banUpdateWithoutWhere" => self + .ban_update_without_where + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "banVacuumFull" => self + .ban_vacuum_full + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "changingColumnType" => self .changing_column_type .as_ref() @@ -885,6 +1266,10 @@ impl Safety { .renaming_table .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "requireConcurrentDetachPartition" => self + .require_concurrent_detach_partition + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "requireConcurrentIndexCreation" => self .require_concurrent_index_creation .as_ref() @@ -893,6 +1278,18 @@ impl Safety { .require_concurrent_index_deletion .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "requireConcurrentReindex" => self + .require_concurrent_reindex + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "requireIdleInTransactionTimeout" => self + .require_idle_in_transaction_timeout + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "requireStatementTimeout" => self + .require_statement_timeout + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), "runningStatementWhileHoldingAccessExclusive" => self .running_statement_while_holding_access_exclusive .as_ref() @@ -901,6 +1298,14 @@ impl Safety { .transaction_nesting .as_ref() .map(|conf| (conf.level(), conf.get_options())), + "warnRefreshMatviewConcurrent" => self + .warn_refresh_matview_concurrent + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), + "warnWideLockWindow" => self + .warn_wide_lock_window + .as_ref() + .map(|conf| (conf.level(), conf.get_options())), _ => None, } } diff --git a/crates/pgls_diagnostics_categories/src/categories.rs b/crates/pgls_diagnostics_categories/src/categories.rs index b7f3b573d..1e7e3f0e0 100644 --- a/crates/pgls_diagnostics_categories/src/categories.rs +++ b/crates/pgls_diagnostics_categories/src/categories.rs @@ -19,13 +19,26 @@ define_categories! { "lint/safety/addingNotNullField": "https://pg-language-server.com/latest/reference/rules/adding-not-null-field/", "lint/safety/addingPrimaryKeyConstraint": "https://pg-language-server.com/latest/reference/rules/adding-primary-key-constraint/", "lint/safety/addingRequiredField": "https://pg-language-server.com/latest/reference/rules/adding-required-field/", + "lint/safety/banAddExclusionConstraint": "https://pg-language-server.com/latest/reference/rules/ban-add-exclusion-constraint/", + "lint/safety/banAlterEnumAddValue": "https://pg-language-server.com/latest/reference/rules/ban-alter-enum-add-value/", + "lint/safety/banAttachPartition": "https://pg-language-server.com/latest/reference/rules/ban-attach-partition/", + "lint/safety/banBlockingRefreshMatview": "https://pg-language-server.com/latest/reference/rules/ban-blocking-refresh-matview/", "lint/safety/banCharField": "https://pg-language-server.com/latest/reference/rules/ban-char-field/", "lint/safety/banConcurrentIndexCreationInTransaction": "https://pg-language-server.com/latest/reference/rules/ban-concurrent-index-creation-in-transaction/", + "lint/safety/banCreateTrigger": "https://pg-language-server.com/latest/reference/rules/ban-create-trigger/", + "lint/safety/banDeleteWithoutWhere": "https://pg-language-server.com/latest/reference/rules/ban-delete-without-where/", "lint/safety/banDropColumn": "https://pg-language-server.com/latest/reference/rules/ban-drop-column/", + "lint/safety/banEnableDisableTrigger": "https://pg-language-server.com/latest/reference/rules/ban-enable-disable-trigger/", + "lint/safety/banNotValidValidateSameTransaction": "https://pg-language-server.com/latest/reference/rules/ban-not-valid-validate-same-transaction/", "lint/safety/banDropDatabase": "https://pg-language-server.com/latest/reference/rules/ban-drop-database/", "lint/safety/banDropNotNull": "https://pg-language-server.com/latest/reference/rules/ban-drop-not-null/", + "lint/safety/banDropSchema": "https://pg-language-server.com/latest/reference/rules/ban-drop-schema/", "lint/safety/banDropTable": "https://pg-language-server.com/latest/reference/rules/ban-drop-table/", + "lint/safety/banDropTrigger": "https://pg-language-server.com/latest/reference/rules/ban-drop-trigger/", + "lint/safety/banTruncate": "https://pg-language-server.com/latest/reference/rules/ban-truncate/", "lint/safety/banTruncateCascade": "https://pg-language-server.com/latest/reference/rules/ban-truncate-cascade/", + "lint/safety/banUpdateWithoutWhere": "https://pg-language-server.com/latest/reference/rules/ban-update-without-where/", + "lint/safety/banVacuumFull": "https://pg-language-server.com/latest/reference/rules/ban-vacuum-full/", "lint/safety/changingColumnType": "https://pg-language-server.com/latest/reference/rules/changing-column-type/", "lint/safety/constraintMissingNotValid": "https://pg-language-server.com/latest/reference/rules/constraint-missing-not-valid/", "lint/safety/creatingEnum": "https://pg-language-server.com/latest/reference/rules/creating-enum/", @@ -42,10 +55,16 @@ define_categories! { "lint/safety/preferTimestamptz": "https://pg-language-server.com/latest/reference/rules/prefer-timestamptz/", "lint/safety/renamingColumn": "https://pg-language-server.com/latest/reference/rules/renaming-column/", "lint/safety/renamingTable": "https://pg-language-server.com/latest/reference/rules/renaming-table/", + "lint/safety/requireConcurrentDetachPartition": "https://pg-language-server.com/latest/reference/rules/require-concurrent-detach-partition/", + "lint/safety/requireIdleInTransactionTimeout": "https://pg-language-server.com/latest/reference/rules/require-idle-in-transaction-timeout/", "lint/safety/requireConcurrentIndexCreation": "https://pg-language-server.com/latest/reference/rules/require-concurrent-index-creation/", "lint/safety/requireConcurrentIndexDeletion": "https://pg-language-server.com/latest/reference/rules/require-concurrent-index-deletion/", + "lint/safety/requireConcurrentReindex": "https://pg-language-server.com/latest/reference/rules/require-concurrent-reindex/", + "lint/safety/requireStatementTimeout": "https://pg-language-server.com/latest/reference/rules/require-statement-timeout/", "lint/safety/runningStatementWhileHoldingAccessExclusive": "https://pg-language-server.com/latest/reference/rules/running-statement-while-holding-access-exclusive/", "lint/safety/transactionNesting": "https://pg-language-server.com/latest/reference/rules/transaction-nesting/", + "lint/safety/warnRefreshMatviewConcurrent": "https://pg-language-server.com/latest/reference/rules/warn-refresh-matview-concurrent/", + "lint/safety/warnWideLockWindow": "https://pg-language-server.com/latest/reference/rules/warn-wide-lock-window/", // end lint rules // pglinter rules start // Meta diagnostics diff --git a/docs/reference/rule_sources.md b/docs/reference/rule_sources.md index 5828da7f6..e8f1e663f 100644 --- a/docs/reference/rule_sources.md +++ b/docs/reference/rule_sources.md @@ -49,3 +49,27 @@ _No exclusive rules available._ | [require-concurrent-index-creation](https://squawkhq.com/docs/require-concurrent-index-creation) |[requireConcurrentIndexCreation](./rules/require-concurrent-index-creation.md) | | [require-concurrent-index-deletion](https://squawkhq.com/docs/require-concurrent-index-deletion) |[requireConcurrentIndexDeletion](./rules/require-concurrent-index-deletion.md) | | [transaction-nesting](https://squawkhq.com/docs/transaction-nesting) |[transactionNesting](./rules/transaction-nesting.md) | + +### pgfence + +| pgfence Rule Name | Rule Name | +| ---- | ---- | +| [add-constraint-exclude](https://github.com/flvmnt/pgfence) |[banAddExclusionConstraint](./rules/ban-add-exclusion-constraint.md) | +| [alter-enum-add-value](https://github.com/flvmnt/pgfence) |[banAlterEnumAddValue](./rules/ban-alter-enum-add-value.md) | +| [attach-partition](https://github.com/flvmnt/pgfence) |[banAttachPartition](./rules/ban-attach-partition.md) | +| [create-trigger](https://github.com/flvmnt/pgfence) |[banCreateTrigger](./rules/ban-create-trigger.md) | +| [delete-without-where](https://github.com/flvmnt/pgfence) |[banDeleteWithoutWhere](./rules/ban-delete-without-where.md) | +| [detach-partition](https://github.com/flvmnt/pgfence) |[requireConcurrentDetachPartition](./rules/require-concurrent-detach-partition.md) | +| [drop-schema](https://github.com/flvmnt/pgfence) |[banDropSchema](./rules/ban-drop-schema.md) | +| [drop-trigger](https://github.com/flvmnt/pgfence) |[banDropTrigger](./rules/ban-drop-trigger.md) | +| [enable-disable-trigger](https://github.com/flvmnt/pgfence) |[banEnableDisableTrigger](./rules/ban-enable-disable-trigger.md) | +| [missing-idle-timeout](https://github.com/flvmnt/pgfence) |[requireIdleInTransactionTimeout](./rules/require-idle-in-transaction-timeout.md) | +| [missing-statement-timeout](https://github.com/flvmnt/pgfence) |[requireStatementTimeout](./rules/require-statement-timeout.md) | +| [not-valid-validate-same-tx](https://github.com/flvmnt/pgfence) |[banNotValidValidateSameTransaction](./rules/ban-not-valid-validate-same-transaction.md) | +| [refresh-matview-blocking](https://github.com/flvmnt/pgfence) |[banBlockingRefreshMatview](./rules/ban-blocking-refresh-matview.md) | +| [refresh-matview-concurrent](https://github.com/flvmnt/pgfence) |[warnRefreshMatviewConcurrent](./rules/warn-refresh-matview-concurrent.md) | +| [reindex-non-concurrent](https://github.com/flvmnt/pgfence) |[requireConcurrentReindex](./rules/require-concurrent-reindex.md) | +| [truncate](https://github.com/flvmnt/pgfence) |[banTruncate](./rules/ban-truncate.md) | +| [update-in-migration](https://github.com/flvmnt/pgfence) |[banUpdateWithoutWhere](./rules/ban-update-without-where.md) | +| [vacuum-full](https://github.com/flvmnt/pgfence) |[banVacuumFull](./rules/ban-vacuum-full.md) | +| [wide-lock-window](https://github.com/flvmnt/pgfence) |[warnWideLockWindow](./rules/warn-wide-lock-window.md) | diff --git a/docs/reference/rules.md b/docs/reference/rules.md index 411e6427e..5224a6c3f 100644 --- a/docs/reference/rules.md +++ b/docs/reference/rules.md @@ -18,14 +18,27 @@ Rules that detect potential safety issues in your code. | [addingNotNullField](./rules/adding-not-null-field.md) | Setting a column NOT NULL blocks reads while the table is scanned. | ✅ | | [addingPrimaryKeyConstraint](./rules/adding-primary-key-constraint.md) | Adding a primary key constraint results in locks and table rewrites. | ✅ | | [addingRequiredField](./rules/adding-required-field.md) | Adding a new column that is NOT NULL and has no default value to an existing table effectively makes it required. | | +| [banAddExclusionConstraint](./rules/ban-add-exclusion-constraint.md) | Adding an exclusion constraint acquires an `ACCESS EXCLUSIVE` lock. | ✅ | +| [banAlterEnumAddValue](./rules/ban-alter-enum-add-value.md) | `ALTER TYPE ... ADD VALUE` cannot run inside a transaction block in older Postgres versions. | | +| [banAttachPartition](./rules/ban-attach-partition.md) | Attaching a partition acquires an `ACCESS EXCLUSIVE` lock on the parent table. | ✅ | +| [banBlockingRefreshMatview](./rules/ban-blocking-refresh-matview.md) | `REFRESH MATERIALIZED VIEW` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. | ✅ | | [banCharField](./rules/ban-char-field.md) | Using CHAR(n) or CHARACTER(n) types is discouraged. | | | [banConcurrentIndexCreationInTransaction](./rules/ban-concurrent-index-creation-in-transaction.md) | Concurrent index creation is not allowed within a transaction. | ✅ | +| [banCreateTrigger](./rules/ban-create-trigger.md) | Creating a trigger acquires a `SHARE ROW EXCLUSIVE` lock on the table. | | +| [banDeleteWithoutWhere](./rules/ban-delete-without-where.md) | A `DELETE` statement without a `WHERE` clause will remove all rows from the table. | ✅ | | [banDropColumn](./rules/ban-drop-column.md) | Dropping a column may break existing clients. | ✅ | | [banDropDatabase](./rules/ban-drop-database.md) | Dropping a database may break existing clients (and everything else, really). | | | [banDropNotNull](./rules/ban-drop-not-null.md) | Dropping a NOT NULL constraint may break existing clients. | ✅ | +| [banDropSchema](./rules/ban-drop-schema.md) | Dropping a schema will remove all objects within it and may break existing clients. | ✅ | | [banDropTable](./rules/ban-drop-table.md) | Dropping a table may break existing clients. | ✅ | +| [banDropTrigger](./rules/ban-drop-trigger.md) | Dropping a trigger acquires an `ACCESS EXCLUSIVE` lock on the table. | | +| [banEnableDisableTrigger](./rules/ban-enable-disable-trigger.md) | Enabling or disabling a trigger acquires a `SHARE ROW EXCLUSIVE` lock. | | +| [banNotValidValidateSameTransaction](./rules/ban-not-valid-validate-same-transaction.md) | Validating a constraint in the same transaction it was added as `NOT VALID` defeats the purpose. | ✅ | +| [banTruncate](./rules/ban-truncate.md) | Truncating a table removes all rows and can cause data loss in production. | ✅ | | [banTruncateCascade](./rules/ban-truncate-cascade.md) | Using `TRUNCATE`'s `CASCADE` option will truncate any tables that are also foreign-keyed to the specified tables. | | -| [changingColumnType](./rules/changing-column-type.md) | Changing a column type may break existing clients. | | +| [banUpdateWithoutWhere](./rules/ban-update-without-where.md) | An `UPDATE` statement without a `WHERE` clause will modify all rows in the table. | ✅ | +| [banVacuumFull](./rules/ban-vacuum-full.md) | `VACUUM FULL` rewrites the entire table and acquires an `ACCESS EXCLUSIVE` lock. | ✅ | +| [changingColumnType](./rules/changing-column-type.md) | Changing a column type may require a table rewrite and break existing clients. | | | [constraintMissingNotValid](./rules/constraint-missing-not-valid.md) | Adding constraints without NOT VALID blocks all reads and writes. | | | [creatingEnum](./rules/creating-enum.md) | Creating enum types is not recommended for new applications. | | | [disallowUniqueConstraint](./rules/disallow-unique-constraint.md) | Disallow adding a UNIQUE constraint without using an existing index. | | @@ -41,10 +54,16 @@ Rules that detect potential safety issues in your code. | [preferTimestamptz](./rules/prefer-timestamptz.md) | Prefer TIMESTAMPTZ over TIMESTAMP types. | | | [renamingColumn](./rules/renaming-column.md) | Renaming columns may break existing queries and application code. | | | [renamingTable](./rules/renaming-table.md) | Renaming tables may break existing queries and application code. | | +| [requireConcurrentDetachPartition](./rules/require-concurrent-detach-partition.md) | Detaching a partition without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. | ✅ | | [requireConcurrentIndexCreation](./rules/require-concurrent-index-creation.md) | Creating indexes non-concurrently can lock the table for writes. | | | [requireConcurrentIndexDeletion](./rules/require-concurrent-index-deletion.md) | Dropping indexes non-concurrently can lock the table for reads. | | +| [requireConcurrentReindex](./rules/require-concurrent-reindex.md) | `REINDEX` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock on the table. | ✅ | +| [requireIdleInTransactionTimeout](./rules/require-idle-in-transaction-timeout.md) | Dangerous lock statements should be preceded by `SET idle_in_transaction_session_timeout`. | | +| [requireStatementTimeout](./rules/require-statement-timeout.md) | Dangerous lock statements should be preceded by `SET statement_timeout`. | | | [runningStatementWhileHoldingAccessExclusive](./rules/running-statement-while-holding-access-exclusive.md) | Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access. | ✅ | | [transactionNesting](./rules/transaction-nesting.md) | Detects problematic transaction nesting that could lead to unexpected behavior. | | +| [warnRefreshMatviewConcurrent](./rules/warn-refresh-matview-concurrent.md) | `REFRESH MATERIALIZED VIEW CONCURRENTLY` still acquires an `EXCLUSIVE` lock. | | +| [warnWideLockWindow](./rules/warn-wide-lock-window.md) | Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window. | ✅ | [//]: # (END RULES_INDEX) diff --git a/docs/reference/rules/add-serial-column.md b/docs/reference/rules/add-serial-column.md index 6047cb43e..6d335271d 100644 --- a/docs/reference/rules/add-serial-column.md +++ b/docs/reference/rules/add-serial-column.md @@ -13,7 +13,7 @@ Adding a column with a SERIAL type or GENERATED ALWAYS AS ... STORED causes a full table rewrite. When adding a column with a SERIAL type (serial, bigserial, smallserial) or a GENERATED ALWAYS AS ... STORED column -to an existing table, PostgreSQL must rewrite the entire table while holding an ACCESS EXCLUSIVE lock. +to an existing table, Postgres must rewrite the entire table while holding an ACCESS EXCLUSIVE lock. This blocks all reads and writes to the table for the duration of the rewrite operation. SERIAL types are implemented using sequences and DEFAULT values, while GENERATED ... STORED columns require diff --git a/docs/reference/rules/adding-field-with-default.md b/docs/reference/rules/adding-field-with-default.md index 644ac91fc..3f0cfb37f 100644 --- a/docs/reference/rules/adding-field-with-default.md +++ b/docs/reference/rules/adding-field-with-default.md @@ -12,14 +12,14 @@ ## Description Adding a column with a DEFAULT value may lead to a table rewrite while holding an ACCESS EXCLUSIVE lock. -In PostgreSQL versions before 11, adding a column with a DEFAULT value causes a full table rewrite, +In Postgres versions before 11, adding a column with a DEFAULT value causes a full table rewrite, which holds an ACCESS EXCLUSIVE lock on the table and blocks all reads and writes. -In PostgreSQL 11+, this behavior was optimized for non-volatile defaults. However: +In Postgres 11+, this behavior was optimized for non-volatile defaults. However: - Volatile default values (like random() or custom functions) still cause table rewrites - Generated columns (GENERATED ALWAYS AS) always require table rewrites -- Non-volatile defaults are safe in PostgreSQL 11+ +- Non-volatile defaults are safe in Postgres 11+ ## Examples diff --git a/docs/reference/rules/adding-foreign-key-constraint.md b/docs/reference/rules/adding-foreign-key-constraint.md index 19971ae3e..7f717a7b4 100644 --- a/docs/reference/rules/adding-foreign-key-constraint.md +++ b/docs/reference/rules/adding-foreign-key-constraint.md @@ -13,7 +13,7 @@ Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes. Adding a foreign key constraint to an existing table can cause downtime by locking both tables while -verifying the constraint. PostgreSQL needs to check that all existing values in the referencing +verifying the constraint. Postgres needs to check that all existing values in the referencing column exist in the referenced table. Instead, add the constraint as NOT VALID in one transaction, then VALIDATE it in another transaction. @@ -36,7 +36,7 @@ code-block.sql:1:1 lint/safety/addingForeignKeyConstraint ━━━━━━━ │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 2 │ - i This will block writes to both the referencing and referenced tables while PostgreSQL verifies the constraint. + i This will block writes to both the referencing and referenced tables while Postgres verifies the constraint. i Add the constraint as NOT VALID first, then VALIDATE it in a separate transaction. diff --git a/docs/reference/rules/adding-not-null-field.md b/docs/reference/rules/adding-not-null-field.md index f4da097c9..da8306717 100644 --- a/docs/reference/rules/adding-not-null-field.md +++ b/docs/reference/rules/adding-not-null-field.md @@ -12,11 +12,11 @@ ## Description Setting a column NOT NULL blocks reads while the table is scanned. -In PostgreSQL versions before 11, adding a NOT NULL constraint to an existing column requires +In Postgres versions before 11, adding a NOT NULL constraint to an existing column requires a full table scan to verify that all existing rows satisfy the constraint. This operation takes an ACCESS EXCLUSIVE lock, blocking all reads and writes. -In PostgreSQL 11+, this operation is much faster as it can skip the full table scan for +In Postgres 11+, this operation is much faster as it can skip the full table scan for newly added columns with default values. Instead of using SET NOT NULL, consider using a CHECK constraint with NOT VALID, then diff --git a/docs/reference/rules/adding-primary-key-constraint.md b/docs/reference/rules/adding-primary-key-constraint.md index 644788807..48e3aa843 100644 --- a/docs/reference/rules/adding-primary-key-constraint.md +++ b/docs/reference/rules/adding-primary-key-constraint.md @@ -12,7 +12,7 @@ ## Description Adding a primary key constraint results in locks and table rewrites. -When you add a PRIMARY KEY constraint, PostgreSQL needs to scan the entire table +When you add a PRIMARY KEY constraint, Postgres needs to scan the entire table to verify uniqueness and build the underlying index. This requires an ACCESS EXCLUSIVE lock which blocks all reads and writes. diff --git a/docs/reference/rules/ban-add-exclusion-constraint.md b/docs/reference/rules/ban-add-exclusion-constraint.md new file mode 100644 index 000000000..39d860bef --- /dev/null +++ b/docs/reference/rules/ban-add-exclusion-constraint.md @@ -0,0 +1,62 @@ +# banAddExclusionConstraint +**Diagnostic Category: `lint/safety/banAddExclusionConstraint`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/add-constraint-exclude + +## Description +Adding an exclusion constraint acquires an `ACCESS EXCLUSIVE` lock. + +Exclusion constraints require a full table scan to validate and block all reads +and writes while held. Unlike other constraints, there is no concurrent alternative. +Use `SET lock_timeout` to limit the impact on concurrent operations. + +This also applies to exclusion constraints defined inline in `CREATE TABLE`. + +## Examples + +### Invalid + +```sql +alter table my_table add constraint my_excl exclude using gist (col with &&); +``` + +```sh +code-block.sql:1:1 lint/safety/banAddExclusionConstraint ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock. + + > 1 │ alter table my_table add constraint my_excl exclude using gist (col with &&); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i There is no concurrent alternative for exclusion constraints. Use SET lock_timeout to limit the impact on concurrent operations. + + +``` + +### Valid + +```sql +alter table my_table add constraint my_check check (col > 0) not valid; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banAddExclusionConstraint": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-alter-enum-add-value.md b/docs/reference/rules/ban-alter-enum-add-value.md new file mode 100644 index 000000000..b4713f16b --- /dev/null +++ b/docs/reference/rules/ban-alter-enum-add-value.md @@ -0,0 +1,58 @@ +# banAlterEnumAddValue +**Diagnostic Category: `lint/safety/banAlterEnumAddValue`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/alter-enum-add-value + +## Description +`ALTER TYPE ... ADD VALUE` cannot run inside a transaction block in older Postgres versions. + +In Postgres versions before 12, `ALTER TYPE ... ADD VALUE` cannot be executed inside a +transaction block at all. On Postgres 12+, the operation is fast (metadata-only), but the +new enum value cannot be used in the same transaction until it is committed. + +## Examples + +### Invalid + +```sql +alter type my_enum add value 'new_value'; +``` + +```sh +code-block.sql:1:1 lint/safety/banAlterEnumAddValue ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! ALTER TYPE ... ADD VALUE cannot be used in a transaction block before Postgres 12. + + > 1 │ alter type my_enum add value 'new_value'; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i On Postgres 12+, the operation is fast but the new value cannot be used in the same transaction until committed. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banAlterEnumAddValue": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-attach-partition.md b/docs/reference/rules/ban-attach-partition.md new file mode 100644 index 000000000..bf7f34070 --- /dev/null +++ b/docs/reference/rules/ban-attach-partition.md @@ -0,0 +1,60 @@ +# banAttachPartition +**Diagnostic Category: `lint/safety/banAttachPartition`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/attach-partition + +## Description +Attaching a partition acquires an `ACCESS EXCLUSIVE` lock on the parent table. + +`ALTER TABLE ... ATTACH PARTITION` locks the parent table, blocking all reads and writes. +For large tables, this can cause significant downtime. Consider creating the partition +with the correct constraints upfront, or use a staging table approach. + +## Examples + +### Invalid + +```sql +alter table my_table attach partition my_partition for values from ('2024-01-01') to ('2025-01-01'); +``` + +```sh +code-block.sql:1:1 lint/safety/banAttachPartition ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table. + + > 1 │ alter table my_table attach partition my_partition for values from ('2024-01-01') to ('2025-01-01'); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i This blocks all reads and writes on the parent table. Consider adding a matching CHECK constraint to the child table before attaching to minimize lock duration. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banAttachPartition": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-blocking-refresh-matview.md b/docs/reference/rules/ban-blocking-refresh-matview.md new file mode 100644 index 000000000..9c1c8cc75 --- /dev/null +++ b/docs/reference/rules/ban-blocking-refresh-matview.md @@ -0,0 +1,60 @@ +# banBlockingRefreshMatview +**Diagnostic Category: `lint/safety/banBlockingRefreshMatview`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/refresh-matview-blocking + +## Description +`REFRESH MATERIALIZED VIEW` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. + +This blocks all reads on the materialized view until the refresh completes. +Use `REFRESH MATERIALIZED VIEW CONCURRENTLY` to allow reads during the refresh. +Note: concurrent refresh requires a unique index on the materialized view. + +## Examples + +### Invalid + +```sql +refresh materialized view my_view; +``` + +```sh +code-block.sql:1:1 lint/safety/banBlockingRefreshMatview ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! REFRESH MATERIALIZED VIEW without CONCURRENTLY blocks all reads. + + > 1 │ refresh materialized view my_view; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Use REFRESH MATERIALIZED VIEW CONCURRENTLY to allow reads during the refresh. This requires a unique index on the view. + + +``` + +### Valid + +```sql +refresh materialized view concurrently my_view; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banBlockingRefreshMatview": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-create-trigger.md b/docs/reference/rules/ban-create-trigger.md new file mode 100644 index 000000000..aac1a41bc --- /dev/null +++ b/docs/reference/rules/ban-create-trigger.md @@ -0,0 +1,58 @@ +# banCreateTrigger +**Diagnostic Category: `lint/safety/banCreateTrigger`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/create-trigger + +## Description +Creating a trigger acquires a `SHARE ROW EXCLUSIVE` lock on the table. + +`CREATE TRIGGER` can block concurrent writes while the lock is held. +Triggers also add hidden complexity to write operations on the table, +which can cause unexpected performance issues and make debugging harder. + +## Examples + +### Invalid + +```sql +create trigger my_trigger after insert on my_table for each row execute function my_func(); +``` + +```sh +code-block.sql:1:1 lint/safety/banCreateTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table. + + > 1 │ create trigger my_trigger after insert on my_table for each row execute function my_func(); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Triggers add hidden complexity and can block concurrent writes. Consider using application-level logic instead. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banCreateTrigger": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-delete-without-where.md b/docs/reference/rules/ban-delete-without-where.md new file mode 100644 index 000000000..fe71e3bd3 --- /dev/null +++ b/docs/reference/rules/ban-delete-without-where.md @@ -0,0 +1,60 @@ +# banDeleteWithoutWhere +**Diagnostic Category: `lint/safety/banDeleteWithoutWhere`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/delete-without-where + +## Description +A `DELETE` statement without a `WHERE` clause will remove all rows from the table. + +This is almost always unintentional in a migration or application context. +If you truly need to remove all rows, use `TRUNCATE` explicitly (and acknowledge +its implications), or add a `WHERE true` to signal intent. + +## Examples + +### Invalid + +```sql +delete from my_table; +``` + +```sh +code-block.sql:1:1 lint/safety/banDeleteWithoutWhere ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! A DELETE without a WHERE clause will remove all rows from the table. + + > 1 │ delete from my_table; + │ ^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Add a WHERE clause to limit which rows are deleted. + + +``` + +### Valid + +```sql +delete from my_table where expired_at < now(); +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banDeleteWithoutWhere": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-drop-schema.md b/docs/reference/rules/ban-drop-schema.md new file mode 100644 index 000000000..0541a7b84 --- /dev/null +++ b/docs/reference/rules/ban-drop-schema.md @@ -0,0 +1,60 @@ +# banDropSchema +**Diagnostic Category: `lint/safety/banDropSchema`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/drop-schema + +## Description +Dropping a schema will remove all objects within it and may break existing clients. + +A `DROP SCHEMA` statement removes the entire schema and all objects it contains. +This is a destructive operation that can cause significant data loss and break +dependent applications. + +## Examples + +### Invalid + +```sql +drop schema my_schema; +``` + +```sh +code-block.sql:1:1 lint/safety/banDropSchema ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Dropping a schema will remove all objects within it and may break existing clients. + + > 1 │ drop schema my_schema; + │ ^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Remove objects individually instead, or ensure all dependent applications have been updated. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banDropSchema": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-drop-trigger.md b/docs/reference/rules/ban-drop-trigger.md new file mode 100644 index 000000000..35944feb4 --- /dev/null +++ b/docs/reference/rules/ban-drop-trigger.md @@ -0,0 +1,57 @@ +# banDropTrigger +**Diagnostic Category: `lint/safety/banDropTrigger`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/drop-trigger + +## Description +Dropping a trigger acquires an `ACCESS EXCLUSIVE` lock on the table. + +`DROP TRIGGER` blocks all reads and writes on the table while the lock is held. +It may also break application logic that depends on the trigger's behavior. + +## Examples + +### Invalid + +```sql +drop trigger my_trigger on my_table; +``` + +```sh +code-block.sql:1:1 lint/safety/banDropTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table. + + > 1 │ drop trigger my_trigger on my_table; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i This blocks all reads and writes. Ensure no application logic depends on the trigger before dropping it. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banDropTrigger": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-enable-disable-trigger.md b/docs/reference/rules/ban-enable-disable-trigger.md new file mode 100644 index 000000000..350a9fb17 --- /dev/null +++ b/docs/reference/rules/ban-enable-disable-trigger.md @@ -0,0 +1,57 @@ +# banEnableDisableTrigger +**Diagnostic Category: `lint/safety/banEnableDisableTrigger`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/enable-disable-trigger + +## Description +Enabling or disabling a trigger acquires a `SHARE ROW EXCLUSIVE` lock. + +`ALTER TABLE ... ENABLE/DISABLE TRIGGER` blocks concurrent writes while the lock is held. +This can cause downtime on busy tables. + +## Examples + +### Invalid + +```sql +alter table my_table enable trigger my_trigger; +``` + +```sh +code-block.sql:1:1 lint/safety/banEnableDisableTrigger ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock. + + > 1 │ alter table my_table enable trigger my_trigger; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i This blocks concurrent writes. Consider the impact on busy tables and use SET lock_timeout. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banEnableDisableTrigger": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-not-valid-validate-same-transaction.md b/docs/reference/rules/ban-not-valid-validate-same-transaction.md new file mode 100644 index 000000000..2cb158050 --- /dev/null +++ b/docs/reference/rules/ban-not-valid-validate-same-transaction.md @@ -0,0 +1,51 @@ +# banNotValidValidateSameTransaction +**Diagnostic Category: `lint/safety/banNotValidValidateSameTransaction`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/not-valid-validate-same-tx + +## Description +Validating a constraint in the same transaction it was added as `NOT VALID` defeats the purpose. + +Adding a constraint with `NOT VALID` avoids a full table scan and lock during creation. +But if you immediately `VALIDATE CONSTRAINT` in the same transaction, the validation +still holds the lock from the `ADD CONSTRAINT`, blocking reads and writes. + +Run `VALIDATE CONSTRAINT` in a separate transaction to get the benefit of `NOT VALID`. + +## Examples + +### Invalid + +Adding a NOT VALID constraint and validating it in the same transaction: + +```sql +ALTER TABLE orders ADD CONSTRAINT orders_check CHECK (total > 0) NOT VALID; +ALTER TABLE orders VALIDATE CONSTRAINT orders_check; +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banNotValidValidateSameTransaction": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-truncate.md b/docs/reference/rules/ban-truncate.md new file mode 100644 index 000000000..ea49161c8 --- /dev/null +++ b/docs/reference/rules/ban-truncate.md @@ -0,0 +1,60 @@ +# banTruncate +**Diagnostic Category: `lint/safety/banTruncate`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/truncate + +## Description +Truncating a table removes all rows and can cause data loss in production. + +`TRUNCATE` is a fast, non-transactional (in terms of row-level locking) way to remove +all data from a table. It acquires an `ACCESS EXCLUSIVE` lock and cannot be safely +rolled back in all scenarios. In a migration context, this is almost always a mistake. + +## Examples + +### Invalid + +```sql +truncate my_table; +``` + +```sh +code-block.sql:1:1 lint/safety/banTruncate ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × Truncating a table removes all rows and can cause data loss. + + > 1 │ truncate my_table; + │ ^^^^^^^^^^^^^^^^^^ + 2 │ + + i Use DELETE with a WHERE clause instead, or ensure this is intentional and not part of a migration. + + +``` + +### Valid + +```sql +delete from my_table where expired_at < now(); +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banTruncate": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-update-without-where.md b/docs/reference/rules/ban-update-without-where.md new file mode 100644 index 000000000..7368c3402 --- /dev/null +++ b/docs/reference/rules/ban-update-without-where.md @@ -0,0 +1,59 @@ +# banUpdateWithoutWhere +**Diagnostic Category: `lint/safety/banUpdateWithoutWhere`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/update-in-migration + +## Description +An `UPDATE` statement without a `WHERE` clause will modify all rows in the table. + +This is almost always unintentional in a migration context and can cause data corruption. +If you truly need to update all rows, add a `WHERE true` to signal intent. + +## Examples + +### Invalid + +```sql +update my_table set col = 'value'; +``` + +```sh +code-block.sql:1:1 lint/safety/banUpdateWithoutWhere ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! An UPDATE without a WHERE clause will modify all rows in the table. + + > 1 │ update my_table set col = 'value'; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Add a WHERE clause to limit which rows are updated. + + +``` + +### Valid + +```sql +update my_table set col = 'value' where id = 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banUpdateWithoutWhere": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/ban-vacuum-full.md b/docs/reference/rules/ban-vacuum-full.md new file mode 100644 index 000000000..b40a63737 --- /dev/null +++ b/docs/reference/rules/ban-vacuum-full.md @@ -0,0 +1,60 @@ +# banVacuumFull +**Diagnostic Category: `lint/safety/banVacuumFull`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/vacuum-full + +## Description +`VACUUM FULL` rewrites the entire table and acquires an `ACCESS EXCLUSIVE` lock. + +This blocks all reads and writes for the duration of the operation, which can +take a very long time on large tables. Use regular `VACUUM` or `pg_repack` instead +for online table maintenance. + +## Examples + +### Invalid + +```sql +vacuum full my_table; +``` + +```sh +code-block.sql:1:1 lint/safety/banVacuumFull ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + × VACUUM FULL rewrites the entire table and blocks all access. + + > 1 │ vacuum full my_table; + │ ^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Use regular VACUUM or pg_repack for online table maintenance without blocking reads and writes. + + +``` + +### Valid + +```sql +vacuum my_table; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "banVacuumFull": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/changing-column-type.md b/docs/reference/rules/changing-column-type.md index ac5d2cd84..34b68a773 100644 --- a/docs/reference/rules/changing-column-type.md +++ b/docs/reference/rules/changing-column-type.md @@ -8,12 +8,19 @@ - Inspired from: squawk/changing-column-type ## Description -Changing a column type may break existing clients. +Changing a column type may require a table rewrite and break existing clients. -Changing a column's data type requires an exclusive lock on the table while the entire table is rewritten. -This can take a long time for large tables and will block reads and writes. +Most column type changes require an exclusive lock on the table while the entire +table is rewritten. This can take a long time for large tables and will block +reads and writes. -Instead of changing the type directly, consider creating a new column with the desired type, +Some type changes are safe and don't require a table rewrite: + +- Changing to `text` (binary compatible with varchar/char types) +- Changing to `varchar` without a length limit +- Dropping a `numeric` precision constraint (e.g., `numeric(10,2)` to `numeric`) + +For unsafe type changes, consider creating a new column with the desired type, migrating the data, and then dropping the old column. ## Examples @@ -21,7 +28,7 @@ migrating the data, and then dropping the old column. ### Invalid ```sql -ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text USING "edits"::text; +ALTER TABLE "core_recipe" ALTER COLUMN "count" TYPE bigint; ``` ```sh @@ -29,8 +36,8 @@ code-block.sql:1:1 lint/safety/changingColumnType ━━━━━━━━━━ ! Changing a column type requires a table rewrite and blocks reads and writes. - > 1 │ ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text USING "edits"::text; - │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + > 1 │ ALTER TABLE "core_recipe" ALTER COLUMN "count" TYPE bigint; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 2 │ i Consider creating a new column with the desired type, migrating data, and then dropping the old column. @@ -38,6 +45,12 @@ code-block.sql:1:1 lint/safety/changingColumnType ━━━━━━━━━━ ``` +### Valid + +```sql +ALTER TABLE "core_recipe" ALTER COLUMN "edits" TYPE text; +``` + ## How to configure ```json diff --git a/docs/reference/rules/constraint-missing-not-valid.md b/docs/reference/rules/constraint-missing-not-valid.md index a22ed85a2..3ca55ecae 100644 --- a/docs/reference/rules/constraint-missing-not-valid.md +++ b/docs/reference/rules/constraint-missing-not-valid.md @@ -10,7 +10,7 @@ ## Description Adding constraints without NOT VALID blocks all reads and writes. -When adding a CHECK or FOREIGN KEY constraint, PostgreSQL must validate all existing rows, +When adding a CHECK or FOREIGN KEY constraint, Postgres must validate all existing rows, which requires a full table scan. This blocks reads and writes for the duration. Instead, add the constraint with NOT VALID first, then VALIDATE CONSTRAINT in a separate diff --git a/docs/reference/rules/creating-enum.md b/docs/reference/rules/creating-enum.md index 54c45494e..8bf459f52 100644 --- a/docs/reference/rules/creating-enum.md +++ b/docs/reference/rules/creating-enum.md @@ -13,7 +13,7 @@ Creating enum types is not recommended for new applications. Enumerated types have several limitations that make them difficult to work with in production: - Removing values from an enum requires complex migrations and is not supported directly -- Adding values to an enum requires an ACCESS EXCLUSIVE lock in some PostgreSQL versions +- Adding values to an enum requires an ACCESS EXCLUSIVE lock in some Postgres versions - Associating additional data with enum values is impossible without restructuring - Renaming enum values requires careful migration planning diff --git a/docs/reference/rules/multiple-alter-table.md b/docs/reference/rules/multiple-alter-table.md index e5b1e49f2..f9bf8574c 100644 --- a/docs/reference/rules/multiple-alter-table.md +++ b/docs/reference/rules/multiple-alter-table.md @@ -12,12 +12,12 @@ ## Description 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 +When you run multiple ALTER TABLE statements on the same table, Postgres 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 +allows Postgres to scan and modify the table only once, improving performance and reducing the time locks are held. ## Examples diff --git a/docs/reference/rules/prefer-jsonb.md b/docs/reference/rules/prefer-jsonb.md index 7a7bda21c..a23c436e6 100644 --- a/docs/reference/rules/prefer-jsonb.md +++ b/docs/reference/rules/prefer-jsonb.md @@ -10,7 +10,7 @@ ## Description Prefer JSONB over JSON types. -JSONB is the binary JSON data type in PostgreSQL that is more efficient for most use cases. +JSONB is the binary JSON data type in Postgres that is more efficient for most use cases. Unlike JSON, JSONB stores data in a decomposed binary format which provides several advantages: - Significantly faster query performance for operations like indexing and searching diff --git a/docs/reference/rules/prefer-robust-stmts.md b/docs/reference/rules/prefer-robust-stmts.md index 059176fd4..aa8e0c99c 100644 --- a/docs/reference/rules/prefer-robust-stmts.md +++ b/docs/reference/rules/prefer-robust-stmts.md @@ -52,6 +52,42 @@ code-block.sql:1:1 lint/safety/preferRobustStmts ━━━━━━━━━━ i Add IF EXISTS to make the migration re-runnable if it fails. +``` + +```sql +CREATE TABLE users (id int); +``` + +```sh +code-block.sql:1:1 lint/safety/preferRobustStmts ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! CREATE TABLE should use IF NOT EXISTS. + + > 1 │ CREATE TABLE users (id int); + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Add IF NOT EXISTS to make the migration re-runnable if it fails. + + +``` + +```sql +DROP TABLE users; +``` + +```sh +code-block.sql:1:1 lint/safety/preferRobustStmts ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! DROP TABLE should use IF EXISTS. + + > 1 │ DROP TABLE users; + │ ^^^^^^^^^^^^^^^^^ + 2 │ + + i Add IF EXISTS to make the migration re-runnable if it fails. + + ``` ### Valid @@ -64,6 +100,14 @@ CREATE INDEX CONCURRENTLY IF NOT EXISTS users_email_idx ON users (email); DROP INDEX CONCURRENTLY IF EXISTS users_email_idx; ``` +```sql +CREATE TABLE IF NOT EXISTS users (id int); +``` + +```sql +DROP TABLE IF EXISTS users; +``` + ## How to configure ```json diff --git a/docs/reference/rules/require-concurrent-detach-partition.md b/docs/reference/rules/require-concurrent-detach-partition.md new file mode 100644 index 000000000..04737f176 --- /dev/null +++ b/docs/reference/rules/require-concurrent-detach-partition.md @@ -0,0 +1,60 @@ +# requireConcurrentDetachPartition +**Diagnostic Category: `lint/safety/requireConcurrentDetachPartition`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/detach-partition + +## Description +Detaching a partition without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock. + +`ALTER TABLE ... DETACH PARTITION` without `CONCURRENTLY` blocks all reads and writes +on the parent table. Use `DETACH PARTITION ... CONCURRENTLY` (Postgres 14+) to +avoid blocking concurrent operations. + +## Examples + +### Invalid + +```sql +alter table my_table detach partition my_partition; +``` + +```sh +code-block.sql:1:1 lint/safety/requireConcurrentDetachPartition ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Detaching a partition without CONCURRENTLY blocks all table access. + + > 1 │ alter table my_table detach partition my_partition; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Use DETACH PARTITION ... CONCURRENTLY (Postgres 14+) to avoid blocking reads and writes. + + +``` + +### Valid + +```sql +alter table my_table detach partition my_partition concurrently; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "requireConcurrentDetachPartition": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/require-concurrent-reindex.md b/docs/reference/rules/require-concurrent-reindex.md new file mode 100644 index 000000000..33c21f0aa --- /dev/null +++ b/docs/reference/rules/require-concurrent-reindex.md @@ -0,0 +1,59 @@ +# requireConcurrentReindex +**Diagnostic Category: `lint/safety/requireConcurrentReindex`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/reindex-non-concurrent + +## Description +`REINDEX` without `CONCURRENTLY` acquires an `ACCESS EXCLUSIVE` lock on the table. + +This blocks all reads and writes until the reindex completes. Use `REINDEX CONCURRENTLY` +to rebuild the index without blocking concurrent operations. + +## Examples + +### Invalid + +```sql +reindex index my_index; +``` + +```sh +code-block.sql:1:1 lint/safety/requireConcurrentReindex ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! REINDEX without CONCURRENTLY blocks all table access. + + > 1 │ reindex index my_index; + │ ^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Use REINDEX CONCURRENTLY to rebuild the index without blocking reads and writes. + + +``` + +### Valid + +```sql +reindex index concurrently my_index; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "requireConcurrentReindex": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/require-idle-in-transaction-timeout.md b/docs/reference/rules/require-idle-in-transaction-timeout.md new file mode 100644 index 000000000..a27fc6124 --- /dev/null +++ b/docs/reference/rules/require-idle-in-transaction-timeout.md @@ -0,0 +1,58 @@ +# requireIdleInTransactionTimeout +**Diagnostic Category: `lint/safety/requireIdleInTransactionTimeout`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/missing-idle-timeout + +## Description +Dangerous lock statements should be preceded by `SET idle_in_transaction_session_timeout`. + +A transaction holding dangerous locks that goes idle (e.g., due to application errors) +will block other operations indefinitely. Setting `idle_in_transaction_session_timeout` +ensures the session is terminated if it sits idle too long. + +## Examples + +### Invalid + +```sql +ALTER TABLE users ADD COLUMN email TEXT; +``` + +```sh +code-block.sql:1:1 lint/safety/requireIdleInTransactionTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Statement takes a dangerous lock without idle_in_transaction_session_timeout set. + + > 1 │ ALTER TABLE users ADD COLUMN email TEXT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Run SET idle_in_transaction_session_timeout = '...' before this statement to prevent idle transactions from holding locks. + + +``` + +### Valid + +```sql +CREATE INDEX CONCURRENTLY users_email_idx ON users(email); +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "requireIdleInTransactionTimeout": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/require-statement-timeout.md b/docs/reference/rules/require-statement-timeout.md new file mode 100644 index 000000000..762a3bcd3 --- /dev/null +++ b/docs/reference/rules/require-statement-timeout.md @@ -0,0 +1,57 @@ +# requireStatementTimeout +**Diagnostic Category: `lint/safety/requireStatementTimeout`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/missing-statement-timeout + +## Description +Dangerous lock statements should be preceded by `SET statement_timeout`. + +Long-running statements holding locks can block other operations. Setting a +`statement_timeout` ensures the statement fails rather than running indefinitely. + +## Examples + +### Invalid + +```sql +ALTER TABLE users ADD COLUMN email TEXT; +``` + +```sh +code-block.sql:1:1 lint/safety/requireStatementTimeout ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! Statement takes a dangerous lock without a statement_timeout set. + + > 1 │ ALTER TABLE users ADD COLUMN email TEXT; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Run SET statement_timeout = '...' before this statement to prevent it from running indefinitely. + + +``` + +### Valid + +```sql +CREATE INDEX CONCURRENTLY users_email_idx ON users(email); +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "requireStatementTimeout": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/warn-refresh-matview-concurrent.md b/docs/reference/rules/warn-refresh-matview-concurrent.md new file mode 100644 index 000000000..fcc7e8808 --- /dev/null +++ b/docs/reference/rules/warn-refresh-matview-concurrent.md @@ -0,0 +1,58 @@ +# warnRefreshMatviewConcurrent +**Diagnostic Category: `lint/safety/warnRefreshMatviewConcurrent`** + +**Since**: `vnext` + + +**Sources**: +- Inspired from: pgfence/refresh-matview-concurrent + +## Description +`REFRESH MATERIALIZED VIEW CONCURRENTLY` still acquires an `EXCLUSIVE` lock. + +While concurrent refresh allows reads during the refresh, it still blocks DDL +and other write operations on the materialized view. On large views, this can +take a long time. + +## Examples + +### Invalid + +```sql +refresh materialized view concurrently my_view; +``` + +```sh +code-block.sql:1:1 lint/safety/warnRefreshMatviewConcurrent ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + ! REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock. + + > 1 │ refresh materialized view concurrently my_view; + │ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + 2 │ + + i Concurrent refresh allows reads but still blocks DDL and writes. Consider the impact on long-running refreshes. + + +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "warnRefreshMatviewConcurrent": "error" + } + } + } +} + +``` diff --git a/docs/reference/rules/warn-wide-lock-window.md b/docs/reference/rules/warn-wide-lock-window.md new file mode 100644 index 000000000..5bd95bcc3 --- /dev/null +++ b/docs/reference/rules/warn-wide-lock-window.md @@ -0,0 +1,51 @@ +# warnWideLockWindow +**Diagnostic Category: `lint/safety/warnWideLockWindow`** + +**Since**: `vnext` + +> [!NOTE] +> This rule is recommended. A diagnostic error will appear when linting your code. + +**Sources**: +- Inspired from: pgfence/wide-lock-window + +## Description +Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window. + +When a transaction holds an ACCESS EXCLUSIVE lock on one table and acquires +another on a different table, both locks are held until the transaction commits. +This increases the chance of blocking concurrent operations and causing downtime. + +Split the operations into separate transactions to minimize the lock window. + +## Examples + +### Invalid + +Acquiring locks on multiple tables in the same transaction: + +```sql +ALTER TABLE users ADD COLUMN email TEXT; +ALTER TABLE orders ADD COLUMN total NUMERIC; +``` + +### Valid + +```sql +select 1; +``` + +## How to configure +```json + +{ + "linter": { + "rules": { + "safety": { + "warnWideLockWindow": "error" + } + } + } +} + +``` diff --git a/docs/schema.json b/docs/schema.json index eed4379cc..feec96f77 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -1017,6 +1017,50 @@ "null" ] }, + "banAddExclusionConstraint": { + "description": "Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banAlterEnumAddValue": { + "description": "ALTER TYPE ... ADD VALUE cannot run inside a transaction block in older Postgres versions.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banAttachPartition": { + "description": "Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banBlockingRefreshMatview": { + "description": "REFRESH MATERIALIZED VIEW without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "banCharField": { "description": "Using CHAR(n) or CHARACTER(n) types is discouraged.", "anyOf": [ @@ -1039,6 +1083,28 @@ } ] }, + "banCreateTrigger": { + "description": "Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banDeleteWithoutWhere": { + "description": "A DELETE statement without a WHERE clause will remove all rows from the table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "banDropColumn": { "description": "Dropping a column may break existing clients.", "anyOf": [ @@ -1072,6 +1138,17 @@ } ] }, + "banDropSchema": { + "description": "Dropping a schema will remove all objects within it and may break existing clients.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "banDropTable": { "description": "Dropping a table may break existing clients.", "anyOf": [ @@ -1083,6 +1160,50 @@ } ] }, + "banDropTrigger": { + "description": "Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banEnableDisableTrigger": { + "description": "Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banNotValidValidateSameTransaction": { + "description": "Validating a constraint in the same transaction it was added as NOT VALID defeats the purpose.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banTruncate": { + "description": "Truncating a table removes all rows and can cause data loss in production.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "banTruncateCascade": { "description": "Using TRUNCATE's CASCADE option will truncate any tables that are also foreign-keyed to the specified tables.", "anyOf": [ @@ -1094,8 +1215,30 @@ } ] }, + "banUpdateWithoutWhere": { + "description": "An UPDATE statement without a WHERE clause will modify all rows in the table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "banVacuumFull": { + "description": "VACUUM FULL rewrites the entire table and acquires an ACCESS EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "changingColumnType": { - "description": "Changing a column type may break existing clients.", + "description": "Changing a column type may require a table rewrite and break existing clients.", "anyOf": [ { "$ref": "#/definitions/RuleConfiguration" @@ -1277,6 +1420,17 @@ } ] }, + "requireConcurrentDetachPartition": { + "description": "Detaching a partition without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "requireConcurrentIndexCreation": { "description": "Creating indexes non-concurrently can lock the table for writes.", "anyOf": [ @@ -1299,6 +1453,39 @@ } ] }, + "requireConcurrentReindex": { + "description": "REINDEX without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock on the table.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "requireIdleInTransactionTimeout": { + "description": "Dangerous lock statements should be preceded by SET idle_in_transaction_session_timeout.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "requireStatementTimeout": { + "description": "Dangerous lock statements should be preceded by SET statement_timeout.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, "runningStatementWhileHoldingAccessExclusive": { "description": "Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access.", "anyOf": [ @@ -1320,6 +1507,28 @@ "type": "null" } ] + }, + "warnRefreshMatviewConcurrent": { + "description": "REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] + }, + "warnWideLockWindow": { + "description": "Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window.", + "anyOf": [ + { + "$ref": "#/definitions/RuleConfiguration" + }, + { + "type": "null" + } + ] } }, "additionalProperties": false diff --git a/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts b/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts index 3d4383427..95bbb5609 100644 --- a/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts +++ b/packages/@postgres-language-server/backend-jsonrpc/src/workspace.ts @@ -68,13 +68,26 @@ export type Category = | "lint/safety/addingNotNullField" | "lint/safety/addingPrimaryKeyConstraint" | "lint/safety/addingRequiredField" + | "lint/safety/banAddExclusionConstraint" + | "lint/safety/banAlterEnumAddValue" + | "lint/safety/banAttachPartition" + | "lint/safety/banBlockingRefreshMatview" | "lint/safety/banCharField" | "lint/safety/banConcurrentIndexCreationInTransaction" + | "lint/safety/banCreateTrigger" + | "lint/safety/banDeleteWithoutWhere" | "lint/safety/banDropColumn" + | "lint/safety/banEnableDisableTrigger" + | "lint/safety/banNotValidValidateSameTransaction" | "lint/safety/banDropDatabase" | "lint/safety/banDropNotNull" + | "lint/safety/banDropSchema" | "lint/safety/banDropTable" + | "lint/safety/banDropTrigger" + | "lint/safety/banTruncate" | "lint/safety/banTruncateCascade" + | "lint/safety/banUpdateWithoutWhere" + | "lint/safety/banVacuumFull" | "lint/safety/changingColumnType" | "lint/safety/constraintMissingNotValid" | "lint/safety/creatingEnum" @@ -91,10 +104,16 @@ export type Category = | "lint/safety/preferTimestamptz" | "lint/safety/renamingColumn" | "lint/safety/renamingTable" + | "lint/safety/requireConcurrentDetachPartition" + | "lint/safety/requireIdleInTransactionTimeout" | "lint/safety/requireConcurrentIndexCreation" | "lint/safety/requireConcurrentIndexDeletion" + | "lint/safety/requireConcurrentReindex" + | "lint/safety/requireStatementTimeout" | "lint/safety/runningStatementWhileHoldingAccessExclusive" | "lint/safety/transactionNesting" + | "lint/safety/warnRefreshMatviewConcurrent" + | "lint/safety/warnWideLockWindow" | "pglinter/extensionNotInstalled" | "pglinter/ruleDisabledInExtension" | "pglinter/base/compositePrimaryKeyTooManyColumns" @@ -638,6 +657,22 @@ export interface Safety { * It enables ALL rules for this group. */ all?: boolean; + /** + * Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock. + */ + banAddExclusionConstraint?: RuleConfiguration_for_Null; + /** + * ALTER TYPE ... ADD VALUE cannot run inside a transaction block in older Postgres versions. + */ + banAlterEnumAddValue?: RuleConfiguration_for_Null; + /** + * Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table. + */ + banAttachPartition?: RuleConfiguration_for_Null; + /** + * REFRESH MATERIALIZED VIEW without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock. + */ + banBlockingRefreshMatview?: RuleConfiguration_for_Null; /** * Using CHAR(n) or CHARACTER(n) types is discouraged. */ @@ -646,6 +681,14 @@ export interface Safety { * Concurrent index creation is not allowed within a transaction. */ banConcurrentIndexCreationInTransaction?: RuleConfiguration_for_Null; + /** + * Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table. + */ + banCreateTrigger?: RuleConfiguration_for_Null; + /** + * A DELETE statement without a WHERE clause will remove all rows from the table. + */ + banDeleteWithoutWhere?: RuleConfiguration_for_Null; /** * Dropping a column may break existing clients. */ @@ -658,16 +701,44 @@ export interface Safety { * Dropping a NOT NULL constraint may break existing clients. */ banDropNotNull?: RuleConfiguration_for_Null; + /** + * Dropping a schema will remove all objects within it and may break existing clients. + */ + banDropSchema?: RuleConfiguration_for_Null; /** * Dropping a table may break existing clients. */ banDropTable?: RuleConfiguration_for_Null; + /** + * Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table. + */ + banDropTrigger?: RuleConfiguration_for_Null; + /** + * Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock. + */ + banEnableDisableTrigger?: RuleConfiguration_for_Null; + /** + * Validating a constraint in the same transaction it was added as NOT VALID defeats the purpose. + */ + banNotValidValidateSameTransaction?: RuleConfiguration_for_Null; + /** + * Truncating a table removes all rows and can cause data loss in production. + */ + banTruncate?: RuleConfiguration_for_Null; /** * Using TRUNCATE's CASCADE option will truncate any tables that are also foreign-keyed to the specified tables. */ banTruncateCascade?: RuleConfiguration_for_Null; /** - * Changing a column type may break existing clients. + * An UPDATE statement without a WHERE clause will modify all rows in the table. + */ + banUpdateWithoutWhere?: RuleConfiguration_for_Null; + /** + * VACUUM FULL rewrites the entire table and acquires an ACCESS EXCLUSIVE lock. + */ + banVacuumFull?: RuleConfiguration_for_Null; + /** + * Changing a column type may require a table rewrite and break existing clients. */ changingColumnType?: RuleConfiguration_for_Null; /** @@ -734,6 +805,10 @@ export interface Safety { * Renaming tables may break existing queries and application code. */ renamingTable?: RuleConfiguration_for_Null; + /** + * Detaching a partition without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock. + */ + requireConcurrentDetachPartition?: RuleConfiguration_for_Null; /** * Creating indexes non-concurrently can lock the table for writes. */ @@ -742,6 +817,18 @@ export interface Safety { * Dropping indexes non-concurrently can lock the table for reads. */ requireConcurrentIndexDeletion?: RuleConfiguration_for_Null; + /** + * REINDEX without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock on the table. + */ + requireConcurrentReindex?: RuleConfiguration_for_Null; + /** + * Dangerous lock statements should be preceded by SET idle_in_transaction_session_timeout. + */ + requireIdleInTransactionTimeout?: RuleConfiguration_for_Null; + /** + * Dangerous lock statements should be preceded by SET statement_timeout. + */ + requireStatementTimeout?: RuleConfiguration_for_Null; /** * Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access. */ @@ -750,6 +837,14 @@ export interface Safety { * Detects problematic transaction nesting that could lead to unexpected behavior. */ transactionNesting?: RuleConfiguration_for_Null; + /** + * REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock. + */ + warnRefreshMatviewConcurrent?: RuleConfiguration_for_Null; + /** + * Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window. + */ + warnWideLockWindow?: RuleConfiguration_for_Null; } /** * A list of rules that belong to this group diff --git a/packages/@postgrestools/backend-jsonrpc/src/workspace.ts b/packages/@postgrestools/backend-jsonrpc/src/workspace.ts index 3d4383427..95bbb5609 100644 --- a/packages/@postgrestools/backend-jsonrpc/src/workspace.ts +++ b/packages/@postgrestools/backend-jsonrpc/src/workspace.ts @@ -68,13 +68,26 @@ export type Category = | "lint/safety/addingNotNullField" | "lint/safety/addingPrimaryKeyConstraint" | "lint/safety/addingRequiredField" + | "lint/safety/banAddExclusionConstraint" + | "lint/safety/banAlterEnumAddValue" + | "lint/safety/banAttachPartition" + | "lint/safety/banBlockingRefreshMatview" | "lint/safety/banCharField" | "lint/safety/banConcurrentIndexCreationInTransaction" + | "lint/safety/banCreateTrigger" + | "lint/safety/banDeleteWithoutWhere" | "lint/safety/banDropColumn" + | "lint/safety/banEnableDisableTrigger" + | "lint/safety/banNotValidValidateSameTransaction" | "lint/safety/banDropDatabase" | "lint/safety/banDropNotNull" + | "lint/safety/banDropSchema" | "lint/safety/banDropTable" + | "lint/safety/banDropTrigger" + | "lint/safety/banTruncate" | "lint/safety/banTruncateCascade" + | "lint/safety/banUpdateWithoutWhere" + | "lint/safety/banVacuumFull" | "lint/safety/changingColumnType" | "lint/safety/constraintMissingNotValid" | "lint/safety/creatingEnum" @@ -91,10 +104,16 @@ export type Category = | "lint/safety/preferTimestamptz" | "lint/safety/renamingColumn" | "lint/safety/renamingTable" + | "lint/safety/requireConcurrentDetachPartition" + | "lint/safety/requireIdleInTransactionTimeout" | "lint/safety/requireConcurrentIndexCreation" | "lint/safety/requireConcurrentIndexDeletion" + | "lint/safety/requireConcurrentReindex" + | "lint/safety/requireStatementTimeout" | "lint/safety/runningStatementWhileHoldingAccessExclusive" | "lint/safety/transactionNesting" + | "lint/safety/warnRefreshMatviewConcurrent" + | "lint/safety/warnWideLockWindow" | "pglinter/extensionNotInstalled" | "pglinter/ruleDisabledInExtension" | "pglinter/base/compositePrimaryKeyTooManyColumns" @@ -638,6 +657,22 @@ export interface Safety { * It enables ALL rules for this group. */ all?: boolean; + /** + * Adding an exclusion constraint acquires an ACCESS EXCLUSIVE lock. + */ + banAddExclusionConstraint?: RuleConfiguration_for_Null; + /** + * ALTER TYPE ... ADD VALUE cannot run inside a transaction block in older Postgres versions. + */ + banAlterEnumAddValue?: RuleConfiguration_for_Null; + /** + * Attaching a partition acquires an ACCESS EXCLUSIVE lock on the parent table. + */ + banAttachPartition?: RuleConfiguration_for_Null; + /** + * REFRESH MATERIALIZED VIEW without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock. + */ + banBlockingRefreshMatview?: RuleConfiguration_for_Null; /** * Using CHAR(n) or CHARACTER(n) types is discouraged. */ @@ -646,6 +681,14 @@ export interface Safety { * Concurrent index creation is not allowed within a transaction. */ banConcurrentIndexCreationInTransaction?: RuleConfiguration_for_Null; + /** + * Creating a trigger acquires a SHARE ROW EXCLUSIVE lock on the table. + */ + banCreateTrigger?: RuleConfiguration_for_Null; + /** + * A DELETE statement without a WHERE clause will remove all rows from the table. + */ + banDeleteWithoutWhere?: RuleConfiguration_for_Null; /** * Dropping a column may break existing clients. */ @@ -658,16 +701,44 @@ export interface Safety { * Dropping a NOT NULL constraint may break existing clients. */ banDropNotNull?: RuleConfiguration_for_Null; + /** + * Dropping a schema will remove all objects within it and may break existing clients. + */ + banDropSchema?: RuleConfiguration_for_Null; /** * Dropping a table may break existing clients. */ banDropTable?: RuleConfiguration_for_Null; + /** + * Dropping a trigger acquires an ACCESS EXCLUSIVE lock on the table. + */ + banDropTrigger?: RuleConfiguration_for_Null; + /** + * Enabling or disabling a trigger acquires a SHARE ROW EXCLUSIVE lock. + */ + banEnableDisableTrigger?: RuleConfiguration_for_Null; + /** + * Validating a constraint in the same transaction it was added as NOT VALID defeats the purpose. + */ + banNotValidValidateSameTransaction?: RuleConfiguration_for_Null; + /** + * Truncating a table removes all rows and can cause data loss in production. + */ + banTruncate?: RuleConfiguration_for_Null; /** * Using TRUNCATE's CASCADE option will truncate any tables that are also foreign-keyed to the specified tables. */ banTruncateCascade?: RuleConfiguration_for_Null; /** - * Changing a column type may break existing clients. + * An UPDATE statement without a WHERE clause will modify all rows in the table. + */ + banUpdateWithoutWhere?: RuleConfiguration_for_Null; + /** + * VACUUM FULL rewrites the entire table and acquires an ACCESS EXCLUSIVE lock. + */ + banVacuumFull?: RuleConfiguration_for_Null; + /** + * Changing a column type may require a table rewrite and break existing clients. */ changingColumnType?: RuleConfiguration_for_Null; /** @@ -734,6 +805,10 @@ export interface Safety { * Renaming tables may break existing queries and application code. */ renamingTable?: RuleConfiguration_for_Null; + /** + * Detaching a partition without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock. + */ + requireConcurrentDetachPartition?: RuleConfiguration_for_Null; /** * Creating indexes non-concurrently can lock the table for writes. */ @@ -742,6 +817,18 @@ export interface Safety { * Dropping indexes non-concurrently can lock the table for reads. */ requireConcurrentIndexDeletion?: RuleConfiguration_for_Null; + /** + * REINDEX without CONCURRENTLY acquires an ACCESS EXCLUSIVE lock on the table. + */ + requireConcurrentReindex?: RuleConfiguration_for_Null; + /** + * Dangerous lock statements should be preceded by SET idle_in_transaction_session_timeout. + */ + requireIdleInTransactionTimeout?: RuleConfiguration_for_Null; + /** + * Dangerous lock statements should be preceded by SET statement_timeout. + */ + requireStatementTimeout?: RuleConfiguration_for_Null; /** * Running additional statements while holding an ACCESS EXCLUSIVE lock blocks all table access. */ @@ -750,6 +837,14 @@ export interface Safety { * Detects problematic transaction nesting that could lead to unexpected behavior. */ transactionNesting?: RuleConfiguration_for_Null; + /** + * REFRESH MATERIALIZED VIEW CONCURRENTLY still acquires an EXCLUSIVE lock. + */ + warnRefreshMatviewConcurrent?: RuleConfiguration_for_Null; + /** + * Acquiring ACCESS EXCLUSIVE locks on multiple tables widens the lock window. + */ + warnWideLockWindow?: RuleConfiguration_for_Null; } /** * A list of rules that belong to this group