Skip to content

Commit a58a323

Browse files
authored
Add Postgres version check for adding field with default (#637)
fix #636
1 parent 9064697 commit a58a323

10 files changed

+54
-15
lines changed

crates/squawk_linter/src/rules/adding_field_with_default.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use squawk_syntax::ast::AstNode;
66
use squawk_syntax::{Parse, SourceFile};
77

88
use crate::identifier::Identifier;
9-
use crate::{Linter, Rule, Violation};
9+
use crate::{Linter, Rule, Version, Violation};
1010

1111
fn is_const_expr(expr: &ast::Expr) -> bool {
1212
match expr {
@@ -55,8 +55,7 @@ fn is_non_volatile(expr: &ast::Expr) -> bool {
5555
const NON_VOLATILE_BUILT_IN_FUNCTIONS: &str = include_str!("non_volatile_built_in_functions.txt");
5656

5757
pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse<SourceFile>) {
58-
let message =
59-
"Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.";
58+
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.";
6059
let help = "Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.";
6160
let file = parse.tree();
6261
// TODO: use match_ast! like in #api_walkthrough
@@ -70,7 +69,9 @@ pub(crate) fn adding_field_with_default(ctx: &mut Linter, parse: &Parse<SourceFi
7069
let Some(expr) = default.expr() else {
7170
continue;
7271
};
73-
if is_const_expr(&expr) || is_non_volatile(&expr) {
72+
if ctx.settings.pg_version > Version::new(11, None, None)
73+
&& (is_const_expr(&expr) || is_non_volatile(&expr))
74+
{
7475
continue;
7576
}
7677
ctx.report(
@@ -106,13 +107,10 @@ mod test {
106107
use insta::assert_debug_snapshot;
107108

108109
use crate::Rule;
109-
use crate::test_utils::lint;
110+
use crate::test_utils::{lint, lint_with_postgres_version};
110111

111112
#[test]
112113
fn docs_example_ok_post_pg_11() {
113-
// TODO: differing from squawk because we aren't checking the postgres
114-
// version, maybe we should be default to a more recent version like 15
115-
// instead of 11?
116114
let sql = r#"
117115
-- instead of
118116
ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
@@ -277,4 +275,16 @@ ADD COLUMN bar numeric GENERATED ALWAYS AS (bar + baz) STORED;
277275
assert!(!errors.is_empty());
278276
assert_debug_snapshot!(errors);
279277
}
278+
279+
#[test]
280+
fn docs_example_error_on_pg_11() {
281+
let sql = r#"
282+
-- instead of
283+
ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10;
284+
"#;
285+
286+
let errors = lint_with_postgres_version(sql, Rule::AddingFieldWithDefault, "11");
287+
assert!(!errors.is_empty());
288+
assert_debug_snapshot!(errors);
289+
}
280290
}

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__add_numbers_ok.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 62..67,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__arbitrary_func_err.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 74..83,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_random_with_args_err.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 80..88,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 60..66,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_uuid_error_multi_stmt.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 56..62,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__default_volatile_func_err.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 76..84,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
---
2+
source: crates/squawk_linter/src/rules/adding_field_with_default.rs
3+
expression: errors
4+
---
5+
[
6+
Violation {
7+
code: AddingFieldWithDefault,
8+
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.",
9+
text_range: 74..76,
10+
help: Some(
11+
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",
12+
),
13+
fix: None,
14+
},
15+
]

crates/squawk_linter/src/rules/snapshots/squawk_linter__rules__adding_field_with_default__test__generated_stored_err.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ expression: errors
55
[
66
Violation {
77
code: AddingFieldWithDefault,
8-
message: "Adding a generated column requires a table rewrite with an `ACCESS EXCLUSIVE` lock.",
8+
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.",
99
text_range: 40..78,
1010
help: Some(
1111
"Add the column as nullable, backfill existing rows, and add a trigger to update the column on write instead.",

crates/squawk_linter/src/test_utils.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,20 @@ pub(crate) fn lint(sql: &str, rule: Rule) -> Vec<Violation> {
77
linter.lint(&file, sql)
88
}
99

10+
pub(crate) fn lint_with_postgres_version(
11+
sql: &str,
12+
rule: Rule,
13+
postgres_version: &str,
14+
) -> Vec<Violation> {
15+
let file = squawk_syntax::SourceFile::parse(sql);
16+
assert_eq!(file.errors().len(), 0);
17+
let mut linter = Linter::from([rule]);
18+
linter.settings.pg_version = postgres_version
19+
.parse()
20+
.expect("Invalid PostgreSQL version");
21+
linter.lint(&file, sql)
22+
}
23+
1024
pub(crate) fn lint_with_assume_in_transaction(sql: &str, rule: Rule) -> Vec<Violation> {
1125
let file = squawk_syntax::SourceFile::parse(sql);
1226
assert_eq!(file.errors().len(), 0);

0 commit comments

Comments
 (0)