Commit f1d64879 authored by Andreas Brandl's avatar Andreas Brandl

Revert constraints for add_column_with_default

There is a consistency issue with this: The outcome of the migration
(and whether or not we see a `NOT NULL` or a `CHECK` constraint) depends
on when the migration is being run and in which state the helper is at
that time. This includes a risk of seeing installations that have
different constraints, in particular differing from what we expect to
see in `db/structure.sql`.

As such, this commit reverts the line change.

A follow-up is
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/31736 which
deprecates the `add_column_with_default` helper anyways. For that, it's
helpful to keep using `NOT NULL` constraints (because the outcome is the
same if we just delegate to `add_column`).
parent 10e35363
......@@ -502,7 +502,7 @@ module Gitlab
update_column_in_batches(table, column, default_after_type_cast, &block)
end
add_not_null_constraint(table, column) unless allow_null
change_column_null(table, column, false) unless allow_null
# We want to rescue _all_ exceptions here, even those that don't inherit
# from StandardError.
rescue Exception => error # rubocop: disable all
......
......@@ -647,7 +647,7 @@ describe Gitlab::Database::MigrationHelpers do
expect(model).to receive(:update_column_in_batches)
.with(:projects, :foo, 10)
expect(model).not_to receive(:add_not_null_constraint)
expect(model).not_to receive(:change_column_null)
model.add_column_with_default(:projects, :foo, :integer,
default: 10,
......@@ -658,8 +658,8 @@ describe Gitlab::Database::MigrationHelpers do
expect(model).to receive(:update_column_in_batches)
.with(:projects, :foo, 10)
expect(model).to receive(:add_not_null_constraint)
.with(:projects, :foo)
expect(model).to receive(:change_column_null)
.with(:projects, :foo, false)
model.add_column_with_default(:projects, :foo, :integer, default: 10)
end
......@@ -678,8 +678,8 @@ describe Gitlab::Database::MigrationHelpers do
end
it 'removes the added column whenever changing a column NULL constraint fails' do
expect(model).to receive(:add_not_null_constraint)
.with(:projects, :foo)
expect(model).to receive(:change_column_null)
.with(:projects, :foo, false)
.and_raise(ActiveRecord::ActiveRecordError)
expect(model).to receive(:remove_column)
......@@ -699,7 +699,7 @@ describe Gitlab::Database::MigrationHelpers do
allow(model).to receive(:transaction).and_yield
allow(model).to receive(:column_for).with(:user_details, :foo).and_return(column)
allow(model).to receive(:update_column_in_batches).with(:user_details, :foo, 10, batch_column_name: :user_id)
allow(model).to receive(:add_not_null_constraint).with(:user_details, :foo)
allow(model).to receive(:change_column_null).with(:user_details, :foo, false)
allow(model).to receive(:change_column_default).with(:user_details, :foo, 10)
expect(model).to receive(:add_column)
......@@ -721,7 +721,7 @@ describe Gitlab::Database::MigrationHelpers do
allow(model).to receive(:transaction).and_yield
allow(model).to receive(:column_for).with(:projects, :foo).and_return(column)
allow(model).to receive(:update_column_in_batches).with(:projects, :foo, 10)
allow(model).to receive(:add_not_null_constraint).with(:projects, :foo)
allow(model).to receive(:change_column_null).with(:projects, :foo, false)
allow(model).to receive(:change_column_default).with(:projects, :foo, 10)
expect(model).to receive(:add_column)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment