ban creating domains with constraints

docs/docs/ban-create-domain-with-constraint.md

@@ -0,0 +1,16 @@
+---
+id: ban-create-domain-with-constraint
+title: ban-create-domain-with-constraint
+---
+
+## problem
+
+<!-- TODO -->
+
+## solution
+
+<!-- TODO -->
+
+## links
+
+<!-- TODO -->

docs/sidebars.js

@@ -29,6 +29,7 @@ module.exports = {
       "require-concurrent-index-creation",
       "require-concurrent-index-deletion",
       "transaction-nesting",
+      "ban-create-domain-with-constraint",
       // generator::new-rule-above
     ],
   },

docs/src/pages/index.js

@@ -183,6 +183,11 @@ const rules = [
     tags: ["schema"],
     description: "Prevent forbidden use of transactions during concurrent index creation.",
   },
+  {
+    name: "ban-create-domain-with-constraint",
+    tags: ["TODO"],
+    description: "TODO",
+  },
   // generator::new-rule-above
 ]
 

linter/src/lib.rs

@@ -10,6 +10,7 @@ extern crate lazy_static;
 use crate::errors::CheckSqlError;
 use crate::rules::adding_required_field;
 use crate::rules::ban_concurrent_index_creation_in_transaction;
+use crate::rules::ban_create_domain_with_constraint;
 use crate::rules::ban_drop_not_null;
 use crate::rules::prefer_big_int;
 use crate::rules::prefer_identity;
@@ -119,6 +120,15 @@ lazy_static! {
             ),
         ],
     },
+    SquawkRule {
+        name: RuleViolationKind::BanCreateDomainWithConstraint,
+        func: ban_create_domain_with_constraint,
+        messages: vec![
+            ViolationMessage::Note(
+                "Domains with constraints have poor support for online migrations".into()
+            ),
+        ],
+    },
     SquawkRule {
         name: RuleViolationKind::BanDropColumn,
         func: ban_drop_column,

linter/src/rules/ban_create_domain_with_constraint.rs

@@ -0,0 +1,66 @@
+use crate::{
+    versions::Version,
+    violations::{RuleViolation, RuleViolationKind},
+};
+
+use squawk_parser::ast::{RawStmt, Stmt};
+
+#[must_use]
+pub fn ban_create_domain_with_constraint(
+    tree: &[RawStmt],
+    _pg_version: Option<Version>,
+    _assume_in_transaction: bool,
+) -> Vec<RuleViolation> {
+    let mut errs = vec![];
+    for raw_stmt in tree {
+        match &raw_stmt.stmt {
+            Stmt::CreateDomainStmt(stmt) if stmt.constraints.len() > 0 => {
+                println!("X: {:?}", &raw_stmt.stmt);
+                println!("{}", serde_json::to_string_pretty(&stmt).unwrap());
+                errs.push(RuleViolation::new(
+                    RuleViolationKind::BanCreateDomainWithConstraint,
+                    raw_stmt.into(),
+                    None,
+                ));
+            }
+            _ => continue,
+        }
+    }
+    errs
+}
+
+#[cfg(test)]
+mod test_rules {
+    use crate::{
+        check_sql_with_rule,
+        violations::{RuleViolation, RuleViolationKind},
+    };
+    use insta::assert_debug_snapshot;
+
+    fn lint_sql(sql: &str) -> Vec<RuleViolation> {
+        check_sql_with_rule(
+            sql,
+            &RuleViolationKind::BanCreateDomainWithConstraint,
+            None,
+            false,
+        )
+        .unwrap()
+    }
+
+    #[test]
+    fn ban_create_domain_without_constraint_is_ok() {
+        let sql = r#"
+    CREATE DOMAIN domain_name_1 AS TEXT;
+    CREATE DOMAIN domain_name_2 AS CHARACTER VARYING;
+      "#;
+        assert_eq!(lint_sql(sql), vec![]);
+    }
+
+    #[test]
+    fn ban_create_domain_with_constraint_works() {
+        let sql = r#"
+    CREATE DOMAIN domain_name_3 AS NUMERIC(15,5) CHECK (value > 0);
+      "#;
+        assert_debug_snapshot!(lint_sql(sql));
+    }
+}

linter/src/rules/mod.rs

@@ -52,3 +52,5 @@ pub mod adding_required_field;
 pub use adding_required_field::*;
 pub mod ban_concurrent_index_creation_in_transaction;
 pub use ban_concurrent_index_creation_in_transaction::*;
+pub mod ban_create_domain_with_constraint;
+pub use ban_create_domain_with_constraint::*;

linter/src/rules/snapshots/squawk_linter__rules__ban_create_domain_with_constraint__test_rules__ban_create_domain_with_constraint_works.snap

@@ -0,0 +1,21 @@
+---
+source: linter/src/rules/ban_create_domain_with_constraint.rs
+expression: lint_sql(sql)
+
+---
+[
+    RuleViolation {
+        kind: BanCreateDomainWithConstraint,
+        span: Span {
+            start: 0,
+            len: Some(
+                67,
+            ),
+        },
+        messages: [
+            Note(
+                "Domains with constraints have poor support for online migrations",
+            ),
+        ],
+    },
+]

linter/src/snapshots/squawk_linter__test_rules__rule_names_debug_snap.snap

@@ -1,6 +1,7 @@
 ---
 source: linter/src/lib.rs
 expression: rule_names
+
 ---
 [
     "adding-field-with-default",
@@ -10,6 +11,7 @@ expression: rule_names
     "adding-serial-primary-key-field",
     "ban-char-field",
     "ban-concurrent-index-creation-in-transaction",
+    "ban-create-domain-with-constraint",
     "ban-drop-column",
     "ban-drop-database",
     "ban-drop-not-null",

linter/src/snapshots/squawk_linter__test_rules__rule_names_display_snap.snap

@@ -1,6 +1,7 @@
 ---
 source: linter/src/lib.rs
 expression: "rule_names.join(\"\\n\")"
+
 ---
 adding-field-with-default
 adding-foreign-key-constraint
@@ -9,6 +10,7 @@ adding-required-field
 adding-serial-primary-key-field
 ban-char-field
 ban-concurrent-index-creation-in-transaction
+ban-create-domain-with-constraint
 ban-drop-column
 ban-drop-database
 ban-drop-not-null

linter/src/violations.rs

@@ -61,6 +61,8 @@ pub enum RuleViolationKind {
     AddingRequiredField,
     #[serde(rename = "ban-concurrent-index-creation-in-transaction")]
     BanConcurrentIndexCreationInTransaction,
+    #[serde(rename = "ban-create-domain-with-constraint")]
+    BanCreateDomainWithConstraint,
     // generator::new-rule-above
 }
 

parser/src/ast.rs

@@ -429,6 +429,16 @@ pub struct AlterTableStmt {
     pub missing_ok: bool,
 }
 
+#[derive(Debug, Deserialize, Serialize)]
+pub struct CreateDomainStmt {
+    #[serde(rename = "domainname")]
+    pub domain_name: Vec<QualifiedName>,
+    #[serde(rename = "typeName")]
+    pub typename: Value,
+    #[serde(default)]
+    pub constraints: Vec<Value>,
+}
+
 /// Source: <https://github.com/pganalyze/libpg_query/blob/b2790f8140721ff7f047167ecd7d44267b0a3880/src/pg_query_enum_defs.c#L249-L257>
 #[derive(Serialize, Deserialize, PartialEq, Debug)]
 pub enum DropBehavior {
@@ -928,7 +938,7 @@ pub enum Stmt {
     CreateSeqStmt(Value),
     AlterSeqStmt(Value),
     DefineStmt(Value),
-    CreateDomainStmt(Value),
+    CreateDomainStmt(CreateDomainStmt),
     CreateOpClassStmt(Value),
     CreateOpFamilyStmt(Value),
     AlterOpFamilyStmt(Value),

parser/src/snapshots/squawk_parser__parse__tests__create_domain_stmt.snap

@@ -1,13 +1,38 @@
 ---
 source: parser/src/parse.rs
 expression: res
+
 ---
 Ok(
     [
         RawStmt {
             stmt: CreateDomainStmt(
-                Object({
-                    "constraints": Array([
+                CreateDomainStmt {
+                    domain_name: [
+                        QualifiedName {
+                            string: PGString {
+                                sval: "us_postal_code",
+                            },
+                        },
+                    ],
+                    typename: Object({
+                        "location": Number(
+                            33,
+                        ),
+                        "names": Array([
+                            Object({
+                                "String": Object({
+                                    "sval": String(
+                                        "text",
+                                    ),
+                                }),
+                            }),
+                        ]),
+                        "typemod": Number(
+                            -1,
+                        ),
+                    }),
+                    constraints: [
                         Object({
                             "Constraint": Object({
                                 "contype": String(
@@ -127,34 +152,8 @@ Ok(
                                 }),
                             }),
                         }),
-                    ]),
-                    "domainname": Array([
-                        Object({
-                            "String": Object({
-                                "sval": String(
-                                    "us_postal_code",
-                                ),
-                            }),
-                        }),
-                    ]),
-                    "typeName": Object({
-                        "location": Number(
-                            33,
-                        ),
-                        "names": Array([
-                            Object({
-                                "String": Object({
-                                    "sval": String(
-                                        "text",
-                                    ),
-                                }),
-                            }),
-                        ]),
-                        "typemod": Number(
-                            -1,
-                        ),
-                    }),
-                }),
+                    ],
+                },
             ),
             stmt_location: 0,
             stmt_len: Some(