From 7098aa83da19e38e13beaec09ea776a8812b29bd Mon Sep 17 00:00:00 2001 From: Cr4ftx Date: Sat, 22 Nov 2025 14:58:27 +0100 Subject: [PATCH] feat(adding_foreign_key_constraint): allow when create table in transaction #703 --- .../rules/adding_foreign_key_constraint.rs | 115 ++++++++++++------ 1 file changed, 77 insertions(+), 38 deletions(-) diff --git a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs index cb3106fe..bad2c758 100644 --- a/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs +++ b/crates/squawk_linter/src/rules/adding_foreign_key_constraint.rs @@ -1,60 +1,75 @@ use squawk_syntax::{ Parse, SourceFile, ast::{self, AstNode}, + identifier::Identifier, }; -use crate::{Linter, Rule, Violation}; +use crate::{ + Linter, Rule, Violation, rules::constraint_missing_not_valid::tables_created_in_transaction, +}; pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse) { let message = "Adding a foreign key constraint requires a table scan and a `SHARE ROW EXCLUSIVE` lock on both tables, which blocks writes to each table."; let help = "Add `NOT VALID` to the constraint in one transaction and then VALIDATE the constraint in a separate transaction."; let file = parse.tree(); + let tables_created = tables_created_in_transaction(ctx.settings.assume_in_transaction, &file); // TODO: use match_ast! like in #api_walkthrough for stmt in file.stmts() { if let ast::Stmt::AlterTable(alter_table) = stmt { - for action in alter_table.actions() { - match action { - ast::AlterTableAction::AddConstraint(add_constraint) => { - if add_constraint.not_valid().is_some() { - // Adding foreign key is okay when NOT VALID is specified. - continue; - } - if let Some(constraint) = add_constraint.constraint() { - if matches!( - constraint, - ast::Constraint::ForeignKeyConstraint(_) - | ast::Constraint::ReferencesConstraint(_) - ) { - ctx.report( - Violation::for_node( - Rule::AddingForeignKeyConstraint, - message.into(), - constraint.syntax(), + if let Some(table_name) = alter_table + .relation_name() + .and_then(|x| x.path()) + .and_then(|x| x.segment()) + .and_then(|x| x.name_ref()) + { + for action in alter_table.actions() { + match action { + ast::AlterTableAction::AddConstraint(add_constraint) => { + if add_constraint.not_valid().is_some() + || tables_created.contains(&Identifier::new(&table_name.text())) + { + // Adding foreign key is okay when: + // - NOT VALID is specified. + // - The table is created in the same transaction + continue; + } + if let Some(constraint) = add_constraint.constraint() { + if matches!( + constraint, + ast::Constraint::ForeignKeyConstraint(_) + | ast::Constraint::ReferencesConstraint(_) + ) { + ctx.report( + Violation::for_node( + Rule::AddingForeignKeyConstraint, + message.into(), + constraint.syntax(), + ) + .help(help), ) - .help(help), - ) + } } } - } - ast::AlterTableAction::AddColumn(add_column) => { - for constraint in add_column.constraints() { - if matches!( - constraint, - ast::Constraint::ForeignKeyConstraint(_) - | ast::Constraint::ReferencesConstraint(_) - ) { - ctx.report( - Violation::for_node( - Rule::AddingForeignKeyConstraint, - message.into(), - constraint.syntax(), + ast::AlterTableAction::AddColumn(add_column) => { + for constraint in add_column.constraints() { + if matches!( + constraint, + ast::Constraint::ForeignKeyConstraint(_) + | ast::Constraint::ReferencesConstraint(_) + ) { + ctx.report( + Violation::for_node( + Rule::AddingForeignKeyConstraint, + message.into(), + constraint.syntax(), + ) + .help(help), ) - .help(help), - ) + } } } + _ => continue, } - _ => continue, } } } @@ -64,7 +79,7 @@ pub(crate) fn adding_foreign_key_constraint(ctx: &mut Linter, parse: &Parse