Skip to content

Commit 13ee783

Browse files
committed
linter: add ban-truncate-cascade rule
`truncate` with the `cascade` option set caused Linear's January 2024 incident: https://linear.app/blog/linear-incident-on-jan-24th-2024 related: #341
1 parent 7fb4208 commit 13ee783

File tree

9 files changed

+230
-12
lines changed

9 files changed

+230
-12
lines changed

crates/squawk_linter/src/lib.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ use rules::renaming_table;
4848
use rules::require_concurrent_index_creation;
4949
use rules::require_concurrent_index_deletion;
5050
use rules::transaction_nesting;
51+
use rules::ban_truncate_cascade;
5152
// xtask:new-rule:rule-import
5253

5354
#[derive(Debug, PartialEq, Clone, Copy, Serialize, Hash, Eq, Deserialize, Sequence)]
@@ -108,6 +109,8 @@ pub enum Rule {
108109
BanCreateDomainWithConstraint,
109110
#[serde(rename = "ban-alter-domain-with-add-constraint")]
110111
BanAlterDomainWithAddConstraint,
112+
#[serde(rename = "ban-truncate-cascade")]
113+
BanTruncateCascade,
111114
// xtask:new-rule:error-name
112115
}
113116

@@ -145,6 +148,7 @@ impl TryFrom<&str> for Rule {
145148
}
146149
"ban-create-domain-with-constraint" => Ok(Rule::BanCreateDomainWithConstraint),
147150
"ban-alter-domain-with-add-constraint" => Ok(Rule::BanAlterDomainWithAddConstraint),
151+
"ban-truncate-cascade" => Ok(Rule::BanTruncateCascade),
148152
// xtask:new-rule:str-name
149153
_ => Err(format!("Unknown violation name: {}", s)),
150154
}
@@ -202,6 +206,7 @@ impl fmt::Display for Rule {
202206
Rule::BanCreateDomainWithConstraint => "ban-create-domain-with-constraint",
203207
Rule::UnusedIgnore => "unused-ignore",
204208
Rule::BanAlterDomainWithAddConstraint => "ban-alter-domain-with-add-constraint",
209+
Rule::BanTruncateCascade => "ban-truncate-cascade",
205210
// xtask:new-rule:variant-to-name
206211
};
207212
write!(f, "{}", val)
@@ -345,6 +350,9 @@ impl Linter {
345350
if self.rules.contains(&Rule::TransactionNesting) {
346351
transaction_nesting(self, &file);
347352
}
353+
if self.rules.contains(&Rule::BanTruncateCascade) {
354+
ban_truncate_cascade(self, &file);
355+
}
348356
// xtask:new-rule:rule-call
349357

350358
// locate any ignores in the file
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
use squawk_syntax::{
2+
ast::{self, HasModuleItem},
3+
Parse, SourceFile,
4+
};
5+
6+
use crate::{Linter, Rule, Violation};
7+
8+
pub(crate) fn ban_truncate_cascade(ctx: &mut Linter, parse: &Parse<SourceFile>) {
9+
let file = parse.tree();
10+
for item in file.items() {
11+
match item {
12+
ast::Item::Truncate(truncate) => {
13+
if let Some(cascade) = truncate.cascade_token() {
14+
// TODO: if we had knowledge about the entire schema, we
15+
// could be more precise here and actually navigate the
16+
// foreign keys.
17+
ctx.report(Violation::new(
18+
Rule::BanTruncateCascade,
19+
format!("Using `CASCADE` will recursively truncate any tables that foreign key to the referenced tables! So if you had foreign keys setup as `a <- b <- c` and truncated `a`, then `b` & `c` would also be truncated!"),
20+
cascade.text_range(),
21+
"Remove the `CASCADE` and specify exactly which tables you want to truncate.".to_string(),
22+
));
23+
}
24+
}
25+
_ => (),
26+
}
27+
}
28+
}
29+
30+
#[cfg(test)]
31+
mod test {
32+
use insta::assert_debug_snapshot;
33+
34+
use crate::{Linter, Rule};
35+
use squawk_syntax::SourceFile;
36+
37+
#[test]
38+
fn err() {
39+
let sql = r#"
40+
truncate a, b, c cascade;
41+
"#;
42+
let file = SourceFile::parse(sql);
43+
let mut linter = Linter::from([Rule::BanTruncateCascade]);
44+
let errors = linter.lint(file, sql);
45+
assert_ne!(errors.len(), 0);
46+
assert_debug_snapshot!(errors);
47+
}
48+
49+
#[test]
50+
fn ok() {
51+
let sql = r#"
52+
truncate a, b, c;
53+
truncate a;
54+
"#;
55+
let file = SourceFile::parse(sql);
56+
let mut linter = Linter::from([Rule::BanTruncateCascade]);
57+
let errors = linter.lint(file, sql);
58+
assert_eq!(errors.len(), 0);
59+
}
60+
}

crates/squawk_linter/src/rules/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ pub(crate) mod renaming_table;
2525
pub(crate) mod require_concurrent_index_creation;
2626
pub(crate) mod require_concurrent_index_deletion;
2727
pub(crate) mod transaction_nesting;
28+
pub(crate) mod ban_truncate_cascade;
2829
// xtask:new-rule:mod-decl
2930

3031
pub(crate) use adding_field_with_default::adding_field_with_default;
@@ -54,4 +55,5 @@ pub(crate) use renaming_table::renaming_table;
5455
pub(crate) use require_concurrent_index_creation::require_concurrent_index_creation;
5556
pub(crate) use require_concurrent_index_deletion::require_concurrent_index_deletion;
5657
pub(crate) use transaction_nesting::transaction_nesting;
58+
pub(crate) use ban_truncate_cascade::ban_truncate_cascade;
5759
// xtask:new-rule:export

crates/squawk_parser/src/grammar.rs

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9781,11 +9781,7 @@ fn lock_stmt(p: &mut Parser<'_>) -> CompletedMarker {
97819781
p.bump(LOCK_KW);
97829782
// [ TABLE ]
97839783
p.eat(TABLE_KW);
9784-
// [ ONLY ] name [ * ] [, ...]
9785-
relation_name(p);
9786-
while !p.at(EOF) && p.eat(COMMA) {
9787-
relation_name(p);
9788-
}
9784+
table_list(p);
97899785
// [ IN lockmode MODE ]
97909786
if p.eat(IN_KW) {
97919787
match (p.current(), p.nth(1)) {
@@ -9828,6 +9824,16 @@ fn lock_stmt(p: &mut Parser<'_>) -> CompletedMarker {
98289824
m.complete(p, LOCK_STMT)
98299825
}
98309826

9827+
// [ ONLY ] name [ * ] [, ... ]
9828+
fn table_list(p: &mut Parser<'_>) {
9829+
let m = p.start();
9830+
relation_name(p);
9831+
while !p.at(EOF) && p.eat(COMMA) {
9832+
relation_name(p);
9833+
}
9834+
m.complete(p, TABLE_LIST);
9835+
}
9836+
98319837
// [ WITH with_query [, ...] ]
98329838
// MERGE INTO [ ONLY ] target_table_name [ * ] [ [ AS ] target_alias ]
98339839
// USING data_source ON join_condition
@@ -11098,12 +11104,7 @@ fn truncate_stmt(p: &mut Parser<'_>) -> CompletedMarker {
1109811104
let m = p.start();
1109911105
p.bump(TRUNCATE_KW);
1110011106
p.eat(TABLE_KW);
11101-
while !p.at(EOF) {
11102-
relation_name(p);
11103-
if !p.eat(COMMA) {
11104-
break;
11105-
}
11106-
}
11107+
table_list(p);
1110711108
if p.eat(RESTART_KW) {
1110811109
p.expect(IDENTITY_KW);
1110911110
}

crates/squawk_parser/src/syntax_kind.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,7 @@ pub enum SyntaxKind {
14301430
ADD_CONSTRAINT,
14311431
ADD_COLUMN,
14321432
ATTACH_PARTITION,
1433+
TABLE_LIST,
14331434
SET_SCHEMA,
14341435
SET_TABLESPACE,
14351436
SET_WITHOUT_CLUSTER,

crates/squawk_syntax/src/ast/nodes.rs

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3115,6 +3115,65 @@ impl AstNode for Rollback {
31153115
}
31163116
}
31173117

3118+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
3119+
pub struct TableList {
3120+
pub(crate) syntax: SyntaxNode,
3121+
}
3122+
3123+
impl AstNode for TableList {
3124+
#[inline]
3125+
fn can_cast(kind: SyntaxKind) -> bool {
3126+
kind == SyntaxKind::TABLE_LIST
3127+
}
3128+
#[inline]
3129+
fn cast(syntax: SyntaxNode) -> Option<Self> {
3130+
if Self::can_cast(syntax.kind()) {
3131+
Some(Self { syntax })
3132+
} else {
3133+
None
3134+
}
3135+
}
3136+
#[inline]
3137+
fn syntax(&self) -> &SyntaxNode {
3138+
&self.syntax
3139+
}
3140+
}
3141+
3142+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
3143+
pub struct Truncate {
3144+
pub(crate) syntax: SyntaxNode,
3145+
}
3146+
3147+
impl Truncate {
3148+
#[inline]
3149+
pub fn cascade_token(&self) -> Option<SyntaxToken> {
3150+
support::token(&self.syntax, SyntaxKind::CASCADE_KW)
3151+
}
3152+
#[inline]
3153+
pub fn table_list(&self) -> Option<TableList> {
3154+
support::child(&self.syntax)
3155+
}
3156+
}
3157+
3158+
impl AstNode for Truncate {
3159+
#[inline]
3160+
fn can_cast(kind: SyntaxKind) -> bool {
3161+
kind == SyntaxKind::TRUNCATE_STMT
3162+
}
3163+
#[inline]
3164+
fn cast(syntax: SyntaxNode) -> Option<Self> {
3165+
if Self::can_cast(syntax.kind()) {
3166+
Some(Self { syntax })
3167+
} else {
3168+
None
3169+
}
3170+
}
3171+
#[inline]
3172+
fn syntax(&self) -> &SyntaxNode {
3173+
&self.syntax
3174+
}
3175+
}
3176+
31183177
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
31193178
pub enum Item {
31203179
AlterTable(AlterTable),
@@ -3134,6 +3193,7 @@ pub enum Item {
31343193
DropIndex(DropIndex),
31353194
DropType(DropType),
31363195
Select(Select),
3196+
Truncate(Truncate),
31373197
}
31383198
// impl ast::HasAttrs for Item {}
31393199
// impl ast::HasDocComments for Item {}
@@ -3159,7 +3219,8 @@ impl AstNode for Item {
31593219
| SyntaxKind::ALTER_DOMAIN_STMT
31603220
| SyntaxKind::ALTER_AGGREGATE_STMT
31613221
| SyntaxKind::CREATE_AGGREGATE_STMT
3162-
| SyntaxKind::ROLLBACK_KW
3222+
| SyntaxKind::ROLLBACK_STMT
3223+
| SyntaxKind::TRUNCATE_STMT
31633224
)
31643225
}
31653226
#[inline]
@@ -3182,6 +3243,7 @@ impl AstNode for Item {
31823243
SyntaxKind::CREATE_AGGREGATE_STMT => Item::CreateAggregate(CreateAggregate { syntax }),
31833244
SyntaxKind::DROP_AGGREGATE_STMT => Item::DropAggregate(DropAggregate { syntax }),
31843245
SyntaxKind::ROLLBACK_STMT => Item::Rollback(Rollback { syntax }),
3246+
SyntaxKind::TRUNCATE_STMT => Item::Truncate(Truncate { syntax }),
31853247
_ => return None,
31863248
};
31873249
Some(res)
@@ -3206,6 +3268,7 @@ impl AstNode for Item {
32063268
Item::CreateAggregate(it) => &it.syntax,
32073269
Item::DropAggregate(it) => &it.syntax,
32083270
Item::Rollback(it) => &it.syntax,
3271+
Item::Truncate(it) => &it.syntax,
32093272
}
32103273
}
32113274
}

docs/docs/ban-truncate-cascade.md

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
id: ban-truncate-cascade
3+
title: ban-truncate-cascade
4+
---
5+
6+
## problem
7+
8+
Using `TRUNCATE`'s `CASCADE` option will truncate any tables that are also foreign-keyed to the specified tables.
9+
10+
So if you had tables with foreign-keys like:
11+
12+
```
13+
a <- b <- c
14+
```
15+
16+
and ran:
17+
18+
```sql
19+
truncate a cascade;
20+
```
21+
22+
You'd end up with `a`, `b`, & `c` all being truncated!
23+
24+
### runnable example
25+
26+
Setup:
27+
28+
```sql
29+
create table a (
30+
a_id int primary key
31+
);
32+
create table b (
33+
b_id int primary key,
34+
a_id int,
35+
foreign key (a_id) references a(a_id)
36+
);
37+
create table c (
38+
c_id int primary key,
39+
b_id int,
40+
foreign key (b_id) references b(b_id)
41+
);
42+
insert into a (a_id) values (1), (2), (3);
43+
insert into b (b_id, a_id) values (101, 1), (102, 2), (103, 3);
44+
insert into c (c_id, b_id) values (1001, 101), (1002, 102), (1003, 103);
45+
```
46+
47+
Then you run:
48+
49+
```sql
50+
truncate a cascade;
51+
```
52+
53+
Which outputs:
54+
55+
```text
56+
NOTICE: truncate cascades to table "b"
57+
NOTICE: truncate cascades to table "c"
58+
59+
Query 1 OK: TRUNCATE TABLE
60+
```
61+
62+
And now tables `a`, `b`, & `c` are empty!
63+
64+
## solution
65+
66+
Don't use the `CASCADE` option, instead manually specify the tables you want.
67+
68+
So if you just wanted tables `a` and `b` from the example above:
69+
70+
```sql
71+
truncate a, b;
72+
```
73+
74+
## links
75+
76+
- https://www.postgresql.org/docs/current/sql-truncate.html
77+
- `CASCADE`'s recursive nature [caused Linear's 2024-01-24 incident](https://linear.app/blog/linear-incident-on-jan-24th-2024).

docs/sidebars.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ module.exports = {
3030
"transaction-nesting",
3131
"ban-create-domain-with-constraint",
3232
"ban-alter-domain-with-add-constraint",
33+
"ban-truncate-cascade",
3334
// xtask:new-rule:error-name
3435
],
3536
},

docs/src/pages/index.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,11 @@ const rules = [
188188
tags: ["schema", "locking"],
189189
description: "Domains with constraints have poor support for online migrations",
190190
},
191+
{
192+
name: "ban-truncate-cascade",
193+
tags: ["backwards compatibility"],
194+
description: "Truncate cascade will recursively truncate all related tables!",
195+
},
191196
// xtask:new-rule:rule-doc-meta
192197
]
193198

0 commit comments

Comments
 (0)