add rules to ban domains with constraints#418
Conversation
👷 Deploy request for squawkhq pending review.Visit the deploys page to approve it
|
| let mut errs = vec![]; | ||
| for raw_stmt in tree { | ||
| match &raw_stmt.stmt { | ||
| Stmt::AlterDomainStmt(stmt) if stmt.subtype == "C" => { |
There was a problem hiding this comment.
i couldn't find direct documentation, or even a clear place to cite in the libpg_query code, to demonstrate that subtype == "C" means it's the add constraint variation of alter domain. but, i have every other form in the "shouldn't report errors" test (ban_alter_domain_without_add_constraint_is_ok) to validate it
There was a problem hiding this comment.
Yeah the api is pretty wonky at the moment, I'm working on a new parser that should make this easier when it's done!
preview of it at (using wasm):
|
i think the failures in the python jobs are unrelated to these changes and instead related to sccache usage. the final error reported is but it does compile with both |
|
|
||
| ## problem | ||
|
|
||
| Postgres [domains][], which associate a data type with an optional check constraint, have poor support for online migrations |
There was a problem hiding this comment.
Both of these docs are very thorough, thank you!
sbdchd
left a comment
There was a problem hiding this comment.
TIL domains are a thing! These rules are both great, thanks!
Ban creating domains with constraints (
CREATE DOMAIN ... CHECK) or adding constraints to existing domains (ALTER DOMAIN ... ADD CONSTRAINT).Domains with constraints are hard to use/operationalize well with online migrations, have performance problems, and are soft-discourage by Postgres developers as having unfavorable tradeoffs.