Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve unique constraint check #337

Merged
merged 7 commits into from
Jan 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.",
),
],
},
]
Loading