From 0420cdd0eb8cfcb21b1d7236509b786e49c55752 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=9E=D0=BB=D0=B5=D0=B3=20=D0=95=D1=80=D0=BC=D0=B0=D0=BA?= =?UTF-8?q?=D0=BE=D0=B2?= Date: Mon, 1 Sep 2025 15:03:20 +0300 Subject: [PATCH] Add Postgres version check for adding field with default --- .../src/rules/adding_field_with_default.rs | 26 +++++++++++++------ ...ld_with_default__test__add_numbers_ok.snap | 2 +- ...ith_default__test__arbitrary_func_err.snap | 2 +- ...t__test__default_random_with_args_err.snap | 2 +- ...ith_default__test__default_uuid_error.snap | 2 +- ...__test__default_uuid_error_multi_stmt.snap | 2 +- ...ault__test__default_volatile_func_err.snap | 2 +- ...lt__test__docs_example_error_on_pg_11.snap | 15 +++++++++++ ...h_default__test__generated_stored_err.snap | 2 +- crates/squawk_linter/src/test_utils.rs | 14 ++++++++++ 10 files changed, 54 insertions(+), 15 deletions(-) create mode 100644 crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__docs_example_error_on_pg_11.snap diff --git a/crates/squawk_linter/src/rules/adding_field_with_default.rs b/crates/squawk_linter/src/rules/adding_field_with_default.rs index 230a3bac..3936ff20 100644 --- a/crates/squawk_linter/src/rules/adding_field_with_default.rs +++ b/crates/squawk_linter/src/rules/adding_field_with_default.rs @@ -6,7 +6,7 @@ use squawk_syntax::ast::AstNode; use squawk_syntax::{Parse, SourceFile}; use crate::identifier::Identifier; -use crate::{Linter, Rule, Violation}; +use crate::{Linter, Rule, Version, Violation}; fn is_const_expr(expr: &ast::Expr) -> bool { match expr { @@ -55,8 +55,7 @@ fn is_non_volatile(expr: &ast::Expr) -> bool { const NON_VOLATILE_BUILT_IN_FUNCTIONS: &str = include_str!("non_volatile_built_in_functions.txt"); pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse) { - let message = - "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock."; + let message = "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite."; let help = "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead."; let file = parse.tree(); // TODO: use match_ast! like in #api_walkthrough @@ -70,7 +69,9 @@ pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse Version::new(11, None, None) + && (is_const_expr(&expr) || is_non_volatile(&expr)) + { continue; } ctx.report( @@ -106,13 +107,10 @@ mod test { use insta::assert_debug_snapshot; use crate::Rule; - use crate::test_utils::lint; + use crate::test_utils::{lint, lint_with_postgres_version}; #[test] fn docs_example_ok_post_pg_11() { - // TODO: differing from squawk because we aren't checking the postgres - // version, maybe we should be default to a more recent version like 15 - // instead of 11? let sql = r#" -- instead of ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10; @@ -277,4 +275,16 @@ ADD COLUMN bar numeric GENERATED ALWAYS AS (bar + baz) STORED; assert!(!errors.is_empty()); assert_debug_snapshot!(errors); } + + #[test] + fn docs_example_error_on_pg_11() { + let sql = r#" +-- instead of +ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10; + "#; + + let errors = lint_with_postgres_version(sql, Rule::AddingFieldWithDefault, "11"); + assert!(!errors.is_empty()); + assert_debug_snapshot!(errors); + } } diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__add_numbers_ok.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__add_numbers_ok.snap index 50764fb9..17c224ea 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__add_numbers_ok.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__add_numbers_ok.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 62..67, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap index 6b05a2ed..f429598f 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 74..83, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap index 06442ff2..4429f769 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 80..88, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap index 9f57f336..1a44702e 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 60..66, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap index 5196c975..46cd598d 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 56..62, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap index 16889061..4f0ce043 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 76..84, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__docs_example_error_on_pg_11.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__docs_example_error_on_pg_11.snap new file mode 100644 index 00000000..33a984ee --- /dev/null +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__docs_example_error_on_pg_11.snap @@ -0,0 +1,15 @@ +--- +source: crates/squawk_linter/src/rules/adding_field_with_default.rs +expression: errors +--- +[ + Violation { + code: AddingFieldWithDefault, + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", + text_range: 74..76, + help: Some( + "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", + ), + fix: None, + }, +] diff --git a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap index ee14c327..2fa8357d 100644 --- a/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap +++ b/crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap @@ -5,7 +5,7 @@ expression: errors [ Violation { code: AddingFieldWithDefault, - message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.", + message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock. In Postgres versions 11+, non-VOLATILE DEFAULTs can be added without a rewrite.", text_range: 40..78, help: Some( "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.", diff --git a/crates/squawk_linter/src/test_utils.rs b/crates/squawk_linter/src/test_utils.rs index 9c8ae315..73452655 100644 --- a/crates/squawk_linter/src/test_utils.rs +++ b/crates/squawk_linter/src/test_utils.rs @@ -7,6 +7,20 @@ pub(crate) fn lint(sql: &str, rule: Rule) -> Vec { linter.lint(&file, sql) } +pub(crate) fn lint_with_postgres_version( + sql: &str, + rule: Rule, + postgres_version: &str, +) -> Vec { + let file = squawk_syntax::SourceFile::parse(sql); + assert_eq!(file.errors().len(), 0); + let mut linter = Linter::from([rule]); + linter.settings.pg_version = postgres_version + .parse() + .expect("Invalid PostgreSQL version"); + linter.lint(&file, sql) +} + pub(crate) fn lint_with_assume_in_transaction(sql: &str, rule: Rule) -> Vec { let file = squawk_syntax::SourceFile::parse(sql); assert_eq!(file.errors().len(), 0);