Skip to content

Commit

Permalink
improve unique constraint check (#337)
Browse files Browse the repository at this point in the history
fixes #336
  • Loading branch information
chdsbd authored Jan 13, 2024
1 parent 7de8133 commit 6437c3c
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed

- add exceptions for `ban-concurrent-index-creation-in-transaction` to handle golang-migrate. Thanks @janrueth! (#339)
- improve `disallowed-unique-constraint` to handle `alter table...add column... unique`. (#337)

## v0.27.0 - 2024-01-11

Expand Down
17 changes: 17 additions & 0 deletions docs/docs/disallowed-unique-constraint.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,23 @@ CREATE UNIQUE INDEX CONCURRENTLY dist_id_uniq ON distributors (dist_id);
```


Instead of:

```sql
-- blocks reads and writes to table_name while constraint is validated.
ALTER TABLE distributors
ADD COLUMN dist_id text CONSTRAINT dist_id_uniq UNIQUE (dist_id);
```

Use:

```sql
-- add nullable column separately to prevent blocking read/writes.
ALTER TABLE distributors ADD COLUMN dist_id text;
-- allows reads and writes while index is built
CREATE UNIQUE INDEX CONCURRENTLY dist_id_uniq ON distributors (dist_id);
```

## solution for alembic and sqlalchemy

```python
Expand Down
27 changes: 26 additions & 1 deletion linter/src/rules/disallow_unique_constraint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::versions::Version;
use crate::violations::{RuleViolation, RuleViolationKind};

use squawk_parser::ast::{
AlterTableCmds, AlterTableDef, AlterTableType, ConstrType, RawStmt, Stmt,
AlterTableCmds, AlterTableDef, AlterTableType, ColumnDefConstraint, ConstrType, RawStmt, Stmt,
};

#[must_use]
Expand Down Expand Up @@ -36,6 +36,17 @@ pub fn disallow_unique_constraint(
));
}
}
(Some(AlterTableDef::ColumnDef(col)), AlterTableType::AddColumn) => {
for ColumnDefConstraint::Constraint(constraint) in &col.constraints {
if constraint.contype == ConstrType::Unique {
errs.push(RuleViolation::new(
RuleViolationKind::DisallowedUniqueConstraint,
raw_stmt.into(),
None,
));
}
}
}
_ => continue,
}
}
Expand Down Expand Up @@ -144,4 +155,18 @@ ALTER TABLE products ADD CONSTRAINT sku_constraint UNIQUE (sku);
"#;
assert_eq!(lint_sql_assuming_in_transaction(sql), vec![]);
}
#[test]
fn test_unique_constraint_inline_add_column() {
let sql = r#"
ALTER TABLE foo ADD COLUMN bar text CONSTRAINT foo_bar_unique UNIQUE;
"#;
assert_debug_snapshot!(lint_sql(sql));
}
#[test]
fn test_unique_constraint_inline_add_column_unique() {
let sql = r#"
ALTER TABLE foo ADD COLUMN bar text UNIQUE;
"#;
assert_debug_snapshot!(lint_sql(sql));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: linter/src/rules/disallow_unique_constraint.rs
expression: lint_sql(sql)
---
[
RuleViolation {
kind: DisallowedUniqueConstraint,
span: Span {
start: 0,
len: Some(
69,
),
},
messages: [
Note(
"Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads.",
),
Help(
"Create an index CONCURRENTLY and create the constraint using the index.",
),
],
},
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
source: linter/src/rules/disallow_unique_constraint.rs
expression: lint_sql(sql)
---
[
RuleViolation {
kind: DisallowedUniqueConstraint,
span: Span {
start: 0,
len: Some(
43,
),
},
messages: [
Note(
"Adding a UNIQUE constraint requires an ACCESS EXCLUSIVE lock which blocks reads.",
),
Help(
"Create an index CONCURRENTLY and create the constraint using the index.",
),
],
},
]

0 comments on commit 6437c3c

Please sign in to comment.