From cdb18b3cb293281ec12a6ea5515f3c8fdadf039d Mon Sep 17 00:00:00 2001 From: Adam David Date: Wed, 13 Jul 2022 16:07:52 -0400 Subject: [PATCH 1/4] Add test for adding column with references --- linter/src/rules/adding_foreign_key_constraint.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/linter/src/rules/adding_foreign_key_constraint.rs b/linter/src/rules/adding_foreign_key_constraint.rs index 70874f3b..bf2ab2b4 100644 --- a/linter/src/rules/adding_foreign_key_constraint.rs +++ b/linter/src/rules/adding_foreign_key_constraint.rs @@ -122,6 +122,21 @@ ALTER TABLE "email" ADD CONSTRAINT "fk_user" FOREIGN KEY ("user_id") REFERENCES COMMIT; "#; + let violations = lint_sql(sql); + assert_eq!(violations.len(), 1); + assert_eq!( + violations[0].kind, + RuleViolationKind::AddingForeignKeyConstraint + ); + } + #[test] + fn test_add_column_references_lock() { + let sql = r#" +BEGIN; +ALTER TABLE "emails" ADD COLUMN "user_id" INT REFERENCES "user" ("id"); +COMMIT; + "#; + let violations = lint_sql(sql); assert_eq!(violations.len(), 1); assert_eq!( From 52728d60e47182912f6b6304919fd6766fc9e52b Mon Sep 17 00:00:00 2001 From: Adam David Date: Thu, 14 Jul 2022 14:37:21 -0400 Subject: [PATCH 2/4] Add check in fk constraint for using `references` in add column --- linter/src/rules/adding_foreign_key_constraint.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/linter/src/rules/adding_foreign_key_constraint.rs b/linter/src/rules/adding_foreign_key_constraint.rs index bf2ab2b4..9a577c88 100644 --- a/linter/src/rules/adding_foreign_key_constraint.rs +++ b/linter/src/rules/adding_foreign_key_constraint.rs @@ -4,7 +4,7 @@ use crate::{ }; use squawk_parser::ast::{ - AlterTableCmds, AlterTableDef, AlterTableType, ConstrType, RawStmt, Stmt, TableElt, + AlterTableCmds, AlterTableDef, AlterTableType, ColumnDefConstraint, ConstrType, RawStmt, Stmt, TableElt, }; /// Adding a foreign key constraint requires a table scan and a @@ -52,6 +52,18 @@ pub fn adding_foreign_key_constraint( )); } } + } else if let AlterTableType::AddColumn = command.subtype { + if let Some(AlterTableDef::ColumnDef(column_def)) = &command.def { + for ColumnDefConstraint::Constraint(constraint) in &column_def.constraints { + if !constraint.skip_validation && constraint.contype == ConstrType::Foreign { + errs.push(RuleViolation::new( + RuleViolationKind::AddingForeignKeyConstraint, + raw_stmt.into(), + None, + )); + } + } + } } } } From 08bb33f12d2dbc1a25776cf9d31e9e42f53448d1 Mon Sep 17 00:00:00 2001 From: Adam David Date: Thu, 14 Jul 2022 14:43:37 -0400 Subject: [PATCH 3/4] Fix lint errors --- linter/src/rules/adding_foreign_key_constraint.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/linter/src/rules/adding_foreign_key_constraint.rs b/linter/src/rules/adding_foreign_key_constraint.rs index 9a577c88..648ae5da 100644 --- a/linter/src/rules/adding_foreign_key_constraint.rs +++ b/linter/src/rules/adding_foreign_key_constraint.rs @@ -4,7 +4,8 @@ use crate::{ }; use squawk_parser::ast::{ - AlterTableCmds, AlterTableDef, AlterTableType, ColumnDefConstraint, ConstrType, RawStmt, Stmt, TableElt, + AlterTableCmds, AlterTableDef, AlterTableType, ColumnDefConstraint, ConstrType, RawStmt, Stmt, + TableElt, }; /// Adding a foreign key constraint requires a table scan and a @@ -54,8 +55,12 @@ pub fn adding_foreign_key_constraint( } } else if let AlterTableType::AddColumn = command.subtype { if let Some(AlterTableDef::ColumnDef(column_def)) = &command.def { - for ColumnDefConstraint::Constraint(constraint) in &column_def.constraints { - if !constraint.skip_validation && constraint.contype == ConstrType::Foreign { + for ColumnDefConstraint::Constraint(constraint) in + &column_def.constraints + { + if !constraint.skip_validation + && constraint.contype == ConstrType::Foreign + { errs.push(RuleViolation::new( RuleViolationKind::AddingForeignKeyConstraint, raw_stmt.into(), From 6b2709d7def6dd1e10ca8ff4e10d1ae637ec62dc Mon Sep 17 00:00:00 2001 From: Adam David Date: Thu, 14 Jul 2022 14:45:24 -0400 Subject: [PATCH 4/4] Use simpler equality check in if --- linter/src/rules/adding_foreign_key_constraint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linter/src/rules/adding_foreign_key_constraint.rs b/linter/src/rules/adding_foreign_key_constraint.rs index 648ae5da..0001d7fa 100644 --- a/linter/src/rules/adding_foreign_key_constraint.rs +++ b/linter/src/rules/adding_foreign_key_constraint.rs @@ -53,7 +53,7 @@ pub fn adding_foreign_key_constraint( )); } } - } else if let AlterTableType::AddColumn = command.subtype { + } else if AlterTableType::AddColumn == command.subtype { if let Some(AlterTableDef::ColumnDef(column_def)) = &command.def { for ColumnDefConstraint::Constraint(constraint) in &column_def.constraints