diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d4b4a9..62a5965 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/online_migrations/change_column_type_helpers.rb b/lib/online_migrations/change_column_type_helpers.rb index bd115aa..1d11598 100644 --- a/lib/online_migrations/change_column_type_helpers.rb +++ b/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 diff --git a/test/schema_statements/changing_column_type_test.rb b/test/schema_statements/changing_column_type_test.rb index f446358..0ec0e37 100644 --- a/test/schema_statements/changing_column_type_test.rb +++ b/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)