Commit 9122625a authored by Yannis Roussos's avatar Yannis Roussos Committed by Andreas Brandl

Add undo helpers for change_column_type_concurrently

- Add undo_change_column_type_concurrently
  Reverses operations performed by change_column_type_concurrently
- Add undo_cleanup_concurrent_column_type_change
  Reverses operations performed by cleanup_concurrent_column_type_change
- Add specs for both migration helpers
- Update Renaming Columns section of the What Requires
  Downtime Guide
parent 81527d40
---
title: Add undo helpers for change_column_type_concurrently and cleanup_concurrent_column_type_change
merge_request: 44155
author:
type: other
...@@ -176,7 +176,7 @@ class ChangeUsersUsernameStringToText < ActiveRecord::Migration[4.2] ...@@ -176,7 +176,7 @@ class ChangeUsersUsernameStringToText < ActiveRecord::Migration[4.2]
end end
def down def down
cleanup_concurrent_column_type_change :users, :username undo_change_column_type_concurrently :users, :username
end end
end end
``` ```
...@@ -197,7 +197,7 @@ class ChangeUsersUsernameStringToTextCleanup < ActiveRecord::Migration[4.2] ...@@ -197,7 +197,7 @@ class ChangeUsersUsernameStringToTextCleanup < ActiveRecord::Migration[4.2]
end end
def down def down
change_column_type_concurrently :users, :username, :string undo_cleanup_concurrent_column_type_change :users, :username, :string
end end
end end
``` ```
......
...@@ -544,6 +544,16 @@ module Gitlab ...@@ -544,6 +544,16 @@ module Gitlab
rename_column_concurrently(table, column, temp_column, type: new_type, type_cast_function: type_cast_function, batch_column_name: batch_column_name) rename_column_concurrently(table, column, temp_column, type: new_type, type_cast_function: type_cast_function, batch_column_name: batch_column_name)
end end
# Reverses operations performed by change_column_type_concurrently.
#
# table - The table containing the column.
# column - The name of the column to change.
def undo_change_column_type_concurrently(table, column)
temp_column = "#{column}_for_type_change"
undo_rename_column_concurrently(table, column, temp_column)
end
# Performs cleanup of a concurrent type change. # Performs cleanup of a concurrent type change.
# #
# table - The table containing the column. # table - The table containing the column.
...@@ -560,6 +570,65 @@ module Gitlab ...@@ -560,6 +570,65 @@ module Gitlab
end end
end end
# Reverses operations performed by cleanup_concurrent_column_type_change.
#
# table - The table containing the column.
# column - The name of the column to change.
# old_type - The type of the original column used with change_column_type_concurrently.
# type_cast_function - Required if the conversion back to the original type is not automatic
# batch_column_name - option for tables without a primary key, in this case
# another unique integer column can be used. Example: :user_id
def undo_cleanup_concurrent_column_type_change(table, column, old_type, type_cast_function: nil, batch_column_name: :id)
temp_column = "#{column}_for_type_change"
# Using a descriptive name that includes orinal column's name risks
# taking us above the 63 character limit, so we use a hash
identifier = "#{table}_#{column}_for_type_change"
hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)
temp_undo_cleanup_column = "tmp_undo_cleanup_column_#{hashed_identifier}"
unless column_exists?(table, batch_column_name)
raise "Column #{batch_column_name} does not exist on #{table}"
end
if transaction_open?
raise 'undo_cleanup_concurrent_column_type_change can not be run inside a transaction'
end
check_trigger_permissions!(table)
begin
create_column_from(
table,
column,
temp_undo_cleanup_column,
type: old_type,
batch_column_name: batch_column_name,
type_cast_function: type_cast_function
)
transaction do
# This has to be performed in a transaction as otherwise we might
# have inconsistent data.
rename_column(table, column, temp_column)
rename_column(table, temp_undo_cleanup_column, column)
install_rename_triggers(table, column, temp_column)
end
rescue
# create_column_from can not run inside a transaction, which means
# that there is a risk that if any of the operations that follow it
# fail, we'll be left with an inconsistent schema
# For those reasons, we make sure that we drop temp_undo_cleanup_column
# if an error is caught
if column_exists?(table, temp_undo_cleanup_column)
remove_column(table, temp_undo_cleanup_column)
end
raise
end
end
# Cleans up a concurrent column name. # Cleans up a concurrent column name.
# #
# This method takes care of removing previously installed triggers as well # This method takes care of removing previously installed triggers as well
......
...@@ -933,6 +933,19 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -933,6 +933,19 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
end end
describe '#undo_change_column_type_concurrently' do
it 'reverses the operations of change_column_type_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, "old_for_type_change")
model.undo_change_column_type_concurrently(:users, :old)
end
end
describe '#cleanup_concurrent_column_type_change' do describe '#cleanup_concurrent_column_type_change' do
it 'cleans up the type changing procedure' do it 'cleans up the type changing procedure' do
expect(model).to receive(:cleanup_concurrent_column_rename) expect(model).to receive(:cleanup_concurrent_column_rename)
...@@ -945,6 +958,94 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -945,6 +958,94 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
end end
describe '#undo_cleanup_concurrent_column_type_change' 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_type_change(:users, :old, :new) }
.to raise_error(RuntimeError)
end
end
context 'outside a transaction' do
let(:temp_column) { "old_for_type_change" }
let(:temp_undo_cleanup_column) do
identifier = "users_old_for_type_change"
hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)
"tmp_undo_cleanup_column_#{hashed_identifier}"
end
let(:trigger_name) { model.rename_trigger_name(:users, :old, :old_for_type_change) }
before do
allow(model).to receive(:transaction_open?).and_return(false)
end
it 'reverses the operations of cleanup_concurrent_column_type_change' do
expect(model).to receive(:check_trigger_permissions!).with(:users)
expect(model).to receive(:create_column_from).with(
:users,
:old,
temp_undo_cleanup_column,
type: :string,
batch_column_name: :id,
type_cast_function: nil
).and_return(true)
expect(model).to receive(:rename_column)
.with(:users, :old, temp_column)
expect(model).to receive(:rename_column)
.with(:users, temp_undo_cleanup_column, :old)
expect(model).to receive(:install_rename_triggers_for_postgresql)
.with(trigger_name, '"users"', '"old"', '"old_for_type_change"')
model.undo_cleanup_concurrent_column_type_change(:users, :old, :string)
end
it 'passes the type_cast_function and batch_column_name' do
expect(model).to receive(:column_exists?).with(:users, :other_batch_column).and_return(true)
expect(model).to receive(:check_trigger_permissions!).with(:users)
expect(model).to receive(:create_column_from).with(
:users,
:old,
temp_undo_cleanup_column,
type: :string,
batch_column_name: :other_batch_column,
type_cast_function: :custom_type_cast_function
).and_return(true)
expect(model).to receive(:rename_column)
.with(:users, :old, temp_column)
expect(model).to receive(:rename_column)
.with(:users, temp_undo_cleanup_column, :old)
expect(model).to receive(:install_rename_triggers_for_postgresql)
.with(trigger_name, '"users"', '"old"', '"old_for_type_change"')
model.undo_cleanup_concurrent_column_type_change(
:users,
:old,
:string,
type_cast_function: :custom_type_cast_function,
batch_column_name: :other_batch_column
)
end
it 'raises an error with invalid batch_column_name' do
expect do
model.undo_cleanup_concurrent_column_type_change(:users, :old, :new, batch_column_name: :invalid)
end.to raise_error(RuntimeError, /Column invalid does not exist on users/)
end
end
end
describe '#install_rename_triggers_for_postgresql' do describe '#install_rename_triggers_for_postgresql' do
it 'installs the triggers for PostgreSQL' do it 'installs the triggers for PostgreSQL' do
expect(model).to receive(:execute) expect(model).to receive(:execute)
......
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