Merge branch 'fix-migration-helper' into 'master'
Add helpers to exactly undo cleanup_concurrent_column_rename See merge request gitlab-org/gitlab-ce!32183
This commit is contained in:
commit
fc08d48cf0
|
@ -12,6 +12,6 @@ class RenameAllowLocalRequestsFromHooksAndServicesApplicationSetting < ActiveRec
|
|||
end
|
||||
|
||||
def down
|
||||
cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
|
||||
undo_rename_column_concurrently :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
|
||||
end
|
||||
end
|
||||
|
|
|
@ -12,6 +12,6 @@ class CleanupAllowLocalRequestsFromHooksAndServicesApplicationSettingRename < Ac
|
|||
end
|
||||
|
||||
def down
|
||||
rename_column_concurrently :application_settings, :allow_local_requests_from_web_hooks_and_services, :allow_local_requests_from_hooks_and_services
|
||||
undo_cleanup_concurrent_column_rename :application_settings, :allow_local_requests_from_hooks_and_services, :allow_local_requests_from_web_hooks_and_services
|
||||
end
|
||||
end
|
||||
|
|
|
@ -470,7 +470,7 @@ module Gitlab
|
|||
# We set the default value _after_ adding the column so we don't end up
|
||||
# updating any existing data with the default value. This isn't
|
||||
# necessary since we copy over old values further down.
|
||||
change_column_default(table, new, old_col.default) if old_col.default
|
||||
change_column_default(table, new, old_col.default) unless old_col.default.nil?
|
||||
|
||||
install_rename_triggers(table, old, new)
|
||||
|
||||
|
@ -482,6 +482,16 @@ module Gitlab
|
|||
copy_foreign_keys(table, old, new)
|
||||
end
|
||||
|
||||
def undo_rename_column_concurrently(table, old, new)
|
||||
trigger_name = rename_trigger_name(table, old, new)
|
||||
|
||||
check_trigger_permissions!(table)
|
||||
|
||||
remove_rename_triggers_for_postgresql(table, trigger_name)
|
||||
|
||||
remove_column(table, new)
|
||||
end
|
||||
|
||||
# Installs triggers in a table that keep a new column in sync with an old
|
||||
# one.
|
||||
#
|
||||
|
@ -547,6 +557,35 @@ module Gitlab
|
|||
remove_column(table, old)
|
||||
end
|
||||
|
||||
def undo_cleanup_concurrent_column_rename(table, old, new, type: nil)
|
||||
if transaction_open?
|
||||
raise 'undo_cleanup_concurrent_column_rename can not be run inside a transaction'
|
||||
end
|
||||
|
||||
check_trigger_permissions!(table)
|
||||
|
||||
new_column = column_for(table, new)
|
||||
|
||||
add_column(table, old, type || new_column.type,
|
||||
limit: new_column.limit,
|
||||
precision: new_column.precision,
|
||||
scale: new_column.scale)
|
||||
|
||||
# We set the default value _after_ adding the column so we don't end up
|
||||
# updating any existing data with the default value. This isn't
|
||||
# necessary since we copy over old values further down.
|
||||
change_column_default(table, old, new_column.default) unless new_column.default.nil?
|
||||
|
||||
install_rename_triggers(table, old, new)
|
||||
|
||||
update_column_in_batches(table, old, Arel::Table.new(table)[new])
|
||||
|
||||
change_column_null(table, old, false) unless new_column.null
|
||||
|
||||
copy_indexes(table, new, old)
|
||||
copy_foreign_keys(table, new, old)
|
||||
end
|
||||
|
||||
# Changes the column type of a table using a background migration.
|
||||
#
|
||||
# Because this method uses a background migration it's more suitable for
|
||||
|
|
|
@ -576,6 +576,38 @@ describe Gitlab::Database::MigrationHelpers do
|
|||
|
||||
model.rename_column_concurrently(:users, :old, :new)
|
||||
end
|
||||
|
||||
context 'when default is false' do
|
||||
let(:old_column) do
|
||||
double(:column,
|
||||
type: :boolean,
|
||||
limit: nil,
|
||||
default: false,
|
||||
null: false,
|
||||
precision: nil,
|
||||
scale: nil)
|
||||
end
|
||||
|
||||
it 'copies the default to the new column' do
|
||||
expect(model).to receive(:change_column_default)
|
||||
.with(:users, :new, old_column.default)
|
||||
|
||||
model.rename_column_concurrently(:users, :old, :new)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#undo_rename_column_concurrently' do
|
||||
it 'reverses the operations of rename_column_concurrently' do
|
||||
expect(model).to receive(:check_trigger_permissions!).with(:users)
|
||||
|
||||
expect(model).to receive(:remove_rename_triggers_for_postgresql)
|
||||
.with(:users, /trigger_.{12}/)
|
||||
|
||||
expect(model).to receive(:remove_column).with(:users, :new)
|
||||
|
||||
model.undo_rename_column_concurrently(:users, :old, :new)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -592,6 +624,80 @@ describe Gitlab::Database::MigrationHelpers do
|
|||
end
|
||||
end
|
||||
|
||||
describe '#undo_cleanup_concurrent_column_rename' do
|
||||
context 'in a transaction' do
|
||||
it 'raises RuntimeError' do
|
||||
allow(model).to receive(:transaction_open?).and_return(true)
|
||||
|
||||
expect { model.undo_cleanup_concurrent_column_rename(:users, :old, :new) }
|
||||
.to raise_error(RuntimeError)
|
||||
end
|
||||
end
|
||||
|
||||
context 'outside a transaction' do
|
||||
let(:new_column) do
|
||||
double(:column,
|
||||
type: :integer,
|
||||
limit: 8,
|
||||
default: 0,
|
||||
null: false,
|
||||
precision: 5,
|
||||
scale: 1)
|
||||
end
|
||||
|
||||
let(:trigger_name) { model.rename_trigger_name(:users, :old, :new) }
|
||||
|
||||
before do
|
||||
allow(model).to receive(:transaction_open?).and_return(false)
|
||||
allow(model).to receive(:column_for).and_return(new_column)
|
||||
end
|
||||
|
||||
it 'reverses the operations of cleanup_concurrent_column_rename' do
|
||||
expect(model).to receive(:check_trigger_permissions!).with(:users)
|
||||
|
||||
expect(model).to receive(:install_rename_triggers_for_postgresql)
|
||||
.with(trigger_name, '"users"', '"old"', '"new"')
|
||||
|
||||
expect(model).to receive(:add_column)
|
||||
.with(:users, :old, :integer,
|
||||
limit: new_column.limit,
|
||||
precision: new_column.precision,
|
||||
scale: new_column.scale)
|
||||
|
||||
expect(model).to receive(:change_column_default)
|
||||
.with(:users, :old, new_column.default)
|
||||
|
||||
expect(model).to receive(:update_column_in_batches)
|
||||
|
||||
expect(model).to receive(:change_column_null).with(:users, :old, false)
|
||||
|
||||
expect(model).to receive(:copy_indexes).with(:users, :new, :old)
|
||||
expect(model).to receive(:copy_foreign_keys).with(:users, :new, :old)
|
||||
|
||||
model.undo_cleanup_concurrent_column_rename(:users, :old, :new)
|
||||
end
|
||||
|
||||
context 'when default is false' do
|
||||
let(:new_column) do
|
||||
double(:column,
|
||||
type: :boolean,
|
||||
limit: nil,
|
||||
default: false,
|
||||
null: false,
|
||||
precision: nil,
|
||||
scale: nil)
|
||||
end
|
||||
|
||||
it 'copies the default to the old column' do
|
||||
expect(model).to receive(:change_column_default)
|
||||
.with(:users, :old, new_column.default)
|
||||
|
||||
model.undo_cleanup_concurrent_column_rename(:users, :old, :new)
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
describe '#change_column_type_concurrently' do
|
||||
it 'changes the column type' do
|
||||
expect(model).to receive(:rename_column_concurrently)
|
||||
|
|
Loading…
Reference in New Issue