Promote check constraint to NOT NULL on PostgreSQL >= 12 when chang…

…ing column type (#35)

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## master (unreleased)
 
+- Promote check constraint to `NOT NULL` on PostgreSQL >= 12 when changing column type
 - Fix `safety_assured` with `revert`
 
 ## 0.8.1 (2023-08-04)

lib/online_migrations/change_column_type_helpers.rb

@@ -113,10 +113,11 @@ def initialize_columns_type_change(table_name, columns_and_types, **options)
 
           if raw_connection.server_version >= 11_00_00
             if primary_key(table_name) == column_name.to_s && old_col.type == :integer
-              # If the column to be converted is a Primary Key, set it to
-              # `NOT NULL DEFAULT 0` and we'll copy the correct values when backfilling.
-              # That way, we skip the expensive validation step required to add
-              #  a `NOT NULL` constraint at the end of the process.
+              # For PG < 11 and Primary Key conversions, setting a column as the PK
+              # converts even check constraints to NOT NULL column constraints
+              # and forces an inline re-verification of the whole table.
+              # To avoid this, we instead set it to `NOT NULL DEFAULT 0` and we'll
+              # copy the correct values when backfilling.
               add_column(table_name, tmp_column_name, new_type,
                 **old_col_options.merge(column_options).merge(default: old_col.default || 0, null: false))
             else
@@ -247,22 +248,7 @@ def finalize_columns_type_change(table_name, *column_names)
 
           # At this point we are sure there are no NULLs in this column
           transaction do
-            # For PG < 11 and Primary Key conversions, setting a column as the PK
-            # converts even check constraints to NOT NULL column constraints
-            # and forces an inline re-verification of the whole table.
-            #
-            # For PG >= 12 we can "promote" CHECK constraint to NOT NULL constraint,
-            # but for older versions we can set attribute as NOT NULL directly
-            # through PG internal tables.
-            # In-depth analysis of implications of this was made, so this approach
-            # is considered safe - https://habr.com/ru/company/haulmont/blog/493954/  (in russian).
-            execute(<<-SQL.strip_heredoc)
-              UPDATE pg_catalog.pg_attribute
-              SET attnotnull = true
-              WHERE attrelid = #{quote(table_name)}::regclass
-                AND attname = #{quote(tmp_column_name)}
-            SQL
-
+            __set_not_null(table_name, tmp_column_name)
             remove_not_null_constraint(table_name, tmp_column_name)
           end
         end
@@ -482,6 +468,29 @@ def __copy_check_constraints(table_name, from_column, to_column)
         end
       end
 
+      def __set_not_null(table_name, column_name)
+        # For PG >= 12 we can "promote" CHECK constraint to NOT NULL constraint:
+        # https://github.com/postgres/postgres/commit/bbb96c3704c041d139181c6601e5bc770e045d26
+        if raw_connection.server_version >= 12_00_00
+          execute(<<-SQL.strip_heredoc)
+            ALTER TABLE #{quote_table_name(table_name)}
+            ALTER #{quote_column_name(column_name)}
+            SET NOT NULL
+          SQL
+        else
+          # For older versions we can set attribute as NOT NULL directly
+          # through PG internal tables.
+          # In-depth analysis of implications of this was made, so this approach
+          # is considered safe - https://habr.com/ru/company/haulmont/blog/493954/  (in russian).
+          execute(<<-SQL.strip_heredoc)
+            UPDATE pg_catalog.pg_attribute
+            SET attnotnull = true
+            WHERE attrelid = #{quote(table_name)}::regclass
+              AND attname = #{quote(column_name)}
+          SQL
+        end
+      end
+
       def __check_constraints_for(table_name, column_name)
         __check_constraints(table_name).select { |c| c["column_name"] == column_name }
       end

test/schema_statements/changing_column_type_test.rb

@@ -260,6 +260,30 @@ def test_finalize_column_type_change_copies_check_constraints
       end
     end
 
+    def test_finalize_column_type_change_preserves_not_null_without_default_before_12
+      with_postgres(11) do
+        @connection.initialize_column_type_change(:projects, :description, :text)
+
+        assert_sql("UPDATE pg_catalog.pg_attribute") do
+          @connection.finalize_column_type_change(:projects, :description)
+        end
+
+        assert_equal false, column_for(:projects, :description).null
+      end
+    end
+
+    def test_finalize_column_type_change_preserves_not_null_without_default_after_12
+      with_postgres(12) do
+        @connection.initialize_column_type_change(:projects, :description, :text)
+
+        refute_sql("UPDATE pg_catalog.pg_attribute") do
+          @connection.finalize_column_type_change(:projects, :description)
+        end
+
+        assert_equal false, column_for(:projects, :description).null
+      end
+    end
+
     def test_finalize_column_type_change_keeps_columns_in_sync
       @connection.initialize_column_type_change(:projects, :id, :bigint)
       @connection.finalize_column_type_change(:projects, :id)