Ignore postgres version in adding-not-nullable-field

docs/docs/adding-field-with-default.md

@@ -7,7 +7,11 @@ title: adding-field-with-default
 
 Adding a field with a default can cause table rewrites, which will take an [`ACCESS EXCLUSIVE`](https://www.postgresql.org/docs/10/sql-altertable.html#SQL-ALTERTABLE-NOTES) lock on the table, blocking reads / writes while the statement is running.
 
+:::note Postgres Version
+
 In Postgres version 11 and later, adding a field with a non-`VOLATILE` `DEFAULT` will not require a table rewrite. Adding a field with a [`VOLATILE` `DEFAULT` will cause a table rewrite](https://www.postgresql.org/docs/14/sql-altertable.html#SQL-ALTERTABLE-NOTES).
+:::
+
 
 ## solutions
 

docs/docs/adding-not-nullable-field.md

@@ -5,41 +5,16 @@ title: adding-not-nullable-field
 
 Use a check constraint instead of setting a column as `NOT NULL`.
 
-:::note Postgres Version
-
-In Postgres versions 11 of later, adding a non-null column with a default will complete without a table scan.
-:::
-
 ## problem
 
-Adding a column as `NOT NULL` requires a table scan and the `ALTER TABLE` requires
-an `ACCESS EXCLUSIVE` lock. Reads and writes will be disabled while this statement is running.
-
-## solutions
-
-### adding a non-nullable column
-
-Add a column as nullable and use a check constraint to verify integrity. The check constraint should be added as `NOT NULL` and then validated.
-
-Instead of:
-
-```sql
-ALTER TABLE "recipe" ADD COLUMN "view_count" integer DEFAULT 10 NOT NULL;
-```
+Adding a column as `NOT NULL` is no longer covered by this rule. See ["adding-required-field (set a non-volatile default)"](adding-required-field.md#set-a-non-volatile-default) for more information on how to add new columns with `NOT NULL`.
 
-Use:
 
-```sql
-ALTER TABLE "recipe" ADD COLUMN "view_count" integer DEFAULT 10;
-ALTER TABLE "recipe" ADD CONSTRAINT view_count_not_null
-    CHECK ("view_count" IS NOT NULL) NOT VALID;
--- backfill column so it's not null
-ALTER TABLE "recipe" VALIDATE CONSTRAINT view_count_not_null;
-```
+Modifying a column to be `NOT NULL` may fail if the column contains records with a `NULL` value, requiring a full table scan to check before executing. Old application code may also try to write `NULL` values to the table.
 
-Add the column as nullable, add a check constraint as `NOT VALID` to verify new rows and updates are, backfill the column so it no longer contains null values, validate the constraint to verify existing rows are valid.
+`ALTER TABLE` also requires an `ACCESS EXCLUSIVE` lock which will disable reads and writes while this statement is running.
 
-See ["How not valid constraints work"](constraint-missing-not-valid.md#how-not-valid-validate-works) for more information on adding constraints as `NOT VALID`.
+## solutions
 
 ### setting an existing column as non-nullable
 
@@ -54,11 +29,20 @@ Use:
 ```sql
 ALTER TABLE "recipe" ADD CONSTRAINT view_count_not_null
     CHECK ("view_count" IS NOT NULL) NOT VALID;
--- backfill column so it's not null
+
+-- Backfill column so it's not null
+-- Update ...
+
 ALTER TABLE "recipe" VALIDATE CONSTRAINT view_count_not_null;
-```
 
-Add a check constraint as `NOT VALID` to verify new rows and updates are, backfill the column so it no longer contains null values, validate the constraint to verify existing rows are valid.
+-- If pg version >= 12, set not null without doing a table scan
+ALTER TABLE table_name ALTER COLUMN column_name SET NOT NULL;
+```
+For each step, note that:
+1. Adding the constraint acquires an `ACCESS EXCLUSIVE` lock, but this is done extremely quickly.
+2. You MUST backfill the column before validating the constraint
+3. Validating the constraint does require a full table scan, but only acquires a [`SHARE UPDATE EXCLUSIVE`](https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-DESC-VALIDATE-CONSTRAINT) lock which allows for normal operations to continue.
+4. If using postgres version 12 or later, it forgoes the full table scan by checking the existing constraint.
 
 See ["How not valid constraints work"](constraint-missing-not-valid.md#how-not-valid-validate-works) for more information on adding constraints as `NOT VALID`.
 
@@ -68,56 +52,61 @@ See ["How not valid constraints work"](constraint-missing-not-valid.md#how-not-v
 Instead of:
 
 ```python
-# models.py
+# migrations/*.py
+from alembic import op
 import sqlalchemy as sa
 
-class Recipe(BaseModel):
-    view_count = sa.Column(sa.BigInteger, default=10, nullable=False)
-```
-
-Use:
-
-```python
-# models.py
-import sqlalchemy as sa
+def schema_upgrades():
+    op.alter_column(
+        "recipe",
+        "view_count",
+        existing_type=sa.BigInteger(),
+        nullable=False,
+    )
 
-class Recipe(BaseModel):
-    __table_args__ = (
-        sa.CheckConstraint(
-            "view_count IS NOT NULL",
-            name="view_count_not_null",
-        )
+def schema_downgrades():
+    op.alter_column(
+        "recipe",
+        "view_count",
+        existing_type=sa.BigInteger(),
+        nullable=True,
     )
-    view_count = sa.Column(sa.BigInteger, default=10, nullable=True)
 ```
 
-Create Alembic migration manually, because Alembic not creates migration for constraints automatically. See the related [GitHub Issue here](https://github.com/sqlalchemy/alembic/issues/508).
-
 ```python
 # migrations/*.py
 from alembic import op
 import sqlalchemy as sa
 
 def schema_upgrades():
-    op.add_column(
-        "recipe",
-        sa.Column("view_count", sa.BigInteger(), nullable=True),
-    )
     op.create_check_constraint(
         constraint_name="view_count_not_null",
         table_name="recipe",
         condition="view_count IS NOT NULL",
         postgresql_not_valid=True,
     )
+    # Backfill existing rows to get rid of any NULL values. You have have to split
+    # this into two migrations
     op.execute(
         sa.text("ALTER TABLE recipe VALIDATE CONSTRAINT view_count_not_null"),
     )
+    op.alter_column( # only include this step on pg version >= 12
+        "recipe",
+        "view_count",
+        existing_type=sa.BigInteger(),
+        nullable=False,
+    )
 
 
 def schema_downgrades():
+    op.alter_column( # only include this step on pg version >= 12
+        "recipe",
+        "view_count",
+        existing_type=sa.BigInteger(),
+        nullable=True,
+    )
     op.drop_constraint(
         constraint_name="view_count_not_null",
         table_name="recipe",
     )
-    op.drop_column("recipe", "view_count")
 ```

linter/src/rules/adding_not_null_field.rs

@@ -7,16 +7,10 @@ use squawk_parser::ast::{AlterTableCmds, AlterTableType, RawStmt, Stmt};
 #[must_use]
 pub fn adding_not_nullable_field(
     tree: &[RawStmt],
-    pg_version: Option<Version>,
+    _pg_version: Option<Version>,
     _assume_in_transaction: bool,
 ) -> Vec<RuleViolation> {
     let mut errs = vec![];
-    if let Some(pg_version) = pg_version {
-        let pg_11 = Version::new(11, Some(0), Some(0));
-        if pg_version >= pg_11 {
-            return errs;
-        }
-    }
 
     for raw_stmt in tree {
         match &raw_stmt.stmt {
@@ -42,32 +36,23 @@ pub fn adding_not_nullable_field(
 
 #[cfg(test)]
 mod test_rules {
-    use std::str::FromStr;
-
     use crate::{
         check_sql_with_rule,
-        versions::Version,
         violations::{RuleViolation, RuleViolationKind},
     };
 
-    fn lint_sql(sql: &str, pg_version: Option<Version>) -> Vec<RuleViolation> {
-        check_sql_with_rule(
-            sql,
-            &RuleViolationKind::AddingNotNullableField,
-            pg_version,
-            false,
-        )
-        .unwrap()
+    fn lint_sql(sql: &str) -> Vec<RuleViolation> {
+        check_sql_with_rule(sql, &RuleViolationKind::AddingNotNullableField, None, false).unwrap()
     }
 
     use insta::assert_debug_snapshot;
 
     #[test]
     fn set_not_null() {
-        let sql = r#"
+        let bad_sql = r#"
 ALTER TABLE "core_recipe" ALTER COLUMN "foo" SET NOT NULL;
         "#;
-        assert_debug_snapshot!(lint_sql(sql, None));
+        assert_debug_snapshot!(lint_sql(bad_sql));
     }
 
     #[test]
@@ -80,7 +65,7 @@ ALTER TABLE "core_recipe" ADD COLUMN "foo" integer DEFAULT 10 NOT NULL;
 ALTER TABLE "core_recipe" ALTER COLUMN "foo" DROP DEFAULT;
 COMMIT;
         "#;
-        assert_debug_snapshot!(lint_sql(ok_sql, None));
+        assert_debug_snapshot!(lint_sql(ok_sql));
     }
 
     #[test]
@@ -89,19 +74,6 @@ COMMIT;
 -- This won't work if the table is populated, but that error is caught by adding-required-field.
 ALTER TABLE "core_recipe" ADD COLUMN "foo" integer NOT NULL;
         "#;
-        assert_debug_snapshot!(lint_sql(ok_sql, None));
-    }
-
-    #[test]
-    fn adding_field_that_is_not_nullable_in_version_11() {
-        let ok_sql = r#"
-BEGIN;
---
--- Add field foo to recipe
---
-ALTER TABLE "core_recipe" ADD COLUMN "foo" integer NOT NULL DEFAULT 10;
-COMMIT;
-        "#;
-        assert_debug_snapshot!(lint_sql(ok_sql, Some(Version::from_str("11.0.0").unwrap()),));
+        assert_debug_snapshot!(lint_sql(ok_sql));
     }
 }