Commit 2b2a7a48 authored by Yannis Roussos's avatar Yannis Roussos

Merge branch 'pb-fix-rename-column-concurrently' into 'master'

Fix rename column concurrently

See merge request gitlab-org/gitlab!52032
parents 55c67b9a 39c35e64
# frozen_string_literal: true
module Gitlab
module Database
module MigrationHelpers
module V2
include Gitlab::Database::MigrationHelpers
# Renames a column without requiring downtime.
#
# Concurrent renames work by using database triggers to ensure both the
# old and new column are in sync. However, this method will _not_ remove
# the triggers or the old column automatically; this needs to be done
# manually in a post-deployment migration. This can be done using the
# method `cleanup_concurrent_column_rename`.
#
# table - The name of the database table containing the column.
# old_column - The old column name.
# new_column - The new column name.
# type - The type of the new column. If no type is given the old column's
# type is used.
# batch_column_name - option is for tables without primary key, in this
# case another unique integer column can be used. Example: :user_id
def rename_column_concurrently(table, old_column, new_column, type: nil, batch_column_name: :id)
setup_renamed_column(__callee__, table, old_column, new_column, type, batch_column_name)
with_lock_retries do
install_bidirectional_triggers(table, old_column, new_column)
end
end
# Reverses operations performed by rename_column_concurrently.
#
# This method takes care of removing previously installed triggers as well
# as removing the new column.
#
# table - The name of the database table.
# old_column - The name of the old column.
# new_column - The name of the new column.
def undo_rename_column_concurrently(table, old_column, new_column)
teardown_rename_mechanism(table, old_column, new_column, column_to_remove: new_column)
end
# Cleans up a concurrent column name.
#
# This method takes care of removing previously installed triggers as well
# as removing the old column.
#
# table - The name of the database table.
# old_column - The name of the old column.
# new_column - The name of the new column.
def cleanup_concurrent_column_rename(table, old_column, new_column)
teardown_rename_mechanism(table, old_column, new_column, column_to_remove: old_column)
end
# Reverses the operations performed by cleanup_concurrent_column_rename.
#
# This method adds back the old_column removed
# by cleanup_concurrent_column_rename.
# It also adds back the triggers that are removed
# by cleanup_concurrent_column_rename.
#
# table - The name of the database table containing the column.
# old_column - The old column name.
# new_column - The new column name.
# type - The type of the old column. If no type is given the new column's
# type is used.
# batch_column_name - option is for tables without primary key, in this
# case another unique integer column can be used. Example: :user_id
#
def undo_cleanup_concurrent_column_rename(table, old_column, new_column, type: nil, batch_column_name: :id)
setup_renamed_column(__callee__, table, new_column, old_column, type, batch_column_name)
with_lock_retries do
install_bidirectional_triggers(table, old_column, new_column)
end
end
private
def setup_renamed_column(calling_operation, table, old_column, new_column, type, batch_column_name)
if transaction_open?
raise "#{calling_operation} can not be run inside a transaction"
end
column = columns(table).find { |column| column.name == old_column.to_s }
unless column
raise "Column #{old_column} does not exist on #{table}"
end
if column.default
raise "#{calling_operation} does not currently support columns with default values"
end
unless column_exists?(table, batch_column_name)
raise "Column #{batch_column_name} does not exist on #{table}"
end
check_trigger_permissions!(table)
unless column_exists?(table, new_column)
create_column_from(table, old_column, new_column, type: type, batch_column_name: batch_column_name)
end
end
def teardown_rename_mechanism(table, old_column, new_column, column_to_remove:)
return unless column_exists?(table, column_to_remove)
with_lock_retries do
check_trigger_permissions!(table)
remove_bidirectional_triggers(table, old_column, new_column)
remove_column(table, column_to_remove)
end
end
def install_bidirectional_triggers(table, old_column, new_column)
insert_trigger_name, update_old_trigger_name, update_new_trigger_name =
bidirectional_trigger_names(table, old_column, new_column)
quoted_table = quote_table_name(table)
quoted_old = quote_column_name(old_column)
quoted_new = quote_column_name(new_column)
create_insert_trigger(insert_trigger_name, quoted_table, quoted_old, quoted_new)
create_update_trigger(update_old_trigger_name, quoted_table, quoted_new, quoted_old)
create_update_trigger(update_new_trigger_name, quoted_table, quoted_old, quoted_new)
end
def remove_bidirectional_triggers(table, old_column, new_column)
insert_trigger_name, update_old_trigger_name, update_new_trigger_name =
bidirectional_trigger_names(table, old_column, new_column)
quoted_table = quote_table_name(table)
drop_trigger(insert_trigger_name, quoted_table)
drop_trigger(update_old_trigger_name, quoted_table)
drop_trigger(update_new_trigger_name, quoted_table)
end
def bidirectional_trigger_names(table, old_column, new_column)
%w[insert update_old update_new].map do |operation|
'trigger_' + Digest::SHA256.hexdigest("#{table}_#{old_column}_#{new_column}_#{operation}").first(12)
end
end
def function_name_for_trigger(trigger_name)
"function_for_#{trigger_name}"
end
def create_insert_trigger(trigger_name, quoted_table, quoted_old_column, quoted_new_column)
function_name = function_name_for_trigger(trigger_name)
execute(<<~SQL)
CREATE OR REPLACE FUNCTION #{function_name}()
RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
IF NEW.#{quoted_old_column} IS NULL AND NEW.#{quoted_new_column} IS NOT NULL THEN
NEW.#{quoted_old_column} = NEW.#{quoted_new_column};
END IF;
IF NEW.#{quoted_new_column} IS NULL AND NEW.#{quoted_old_column} IS NOT NULL THEN
NEW.#{quoted_new_column} = NEW.#{quoted_old_column};
END IF;
RETURN NEW;
END
$$;
DROP TRIGGER IF EXISTS #{trigger_name}
ON #{quoted_table};
CREATE TRIGGER #{trigger_name}
BEFORE INSERT ON #{quoted_table}
FOR EACH ROW EXECUTE FUNCTION #{function_name}();
SQL
end
def create_update_trigger(trigger_name, quoted_table, quoted_source_column, quoted_target_column)
function_name = function_name_for_trigger(trigger_name)
execute(<<~SQL)
CREATE OR REPLACE FUNCTION #{function_name}()
RETURNS trigger
LANGUAGE plpgsql
AS $$
BEGIN
NEW.#{quoted_target_column} := NEW.#{quoted_source_column};
RETURN NEW;
END
$$;
DROP TRIGGER IF EXISTS #{trigger_name}
ON #{quoted_table};
CREATE TRIGGER #{trigger_name}
BEFORE UPDATE OF #{quoted_source_column} ON #{quoted_table}
FOR EACH ROW EXECUTE FUNCTION #{function_name}();
SQL
end
def drop_trigger(trigger_name, quoted_table)
function_name = function_name_for_trigger(trigger_name)
execute(<<~SQL)
DROP TRIGGER IF EXISTS #{trigger_name}
ON #{quoted_table};
DROP FUNCTION IF EXISTS #{function_name};
SQL
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::MigrationHelpers::V2 do
include Database::TriggerHelpers
let(:migration) do
ActiveRecord::Migration.new.extend(described_class)
end
before do
allow(migration).to receive(:puts)
end
shared_examples_for 'Setting up to rename a column' do
let(:model) { Class.new(ActiveRecord::Base) }
before do
model.table_name = :test_table
end
context 'when called inside a transaction block' do
before do
allow(migration).to receive(:transaction_open?).and_return(true)
end
it 'raises an error' do
expect do
migration.public_send(operation, :test_table, :original, :renamed)
end.to raise_error("#{operation} can not be run inside a transaction")
end
end
context 'when the existing column has a default value' do
before do
migration.change_column_default :test_table, existing_column, 'default value'
end
it 'raises an error' do
expect do
migration.public_send(operation, :test_table, :original, :renamed)
end.to raise_error("#{operation} does not currently support columns with default values")
end
end
context 'when passing a batch column' do
context 'when the batch column does not exist' do
it 'raises an error' do
expect do
migration.public_send(operation, :test_table, :original, :renamed, batch_column_name: :missing)
end.to raise_error('Column missing does not exist on test_table')
end
end
context 'when the batch column does exist' do
it 'passes it when creating the column' do
expect(migration).to receive(:create_column_from)
.with(:test_table, existing_column, added_column, type: nil, batch_column_name: :status)
.and_call_original
migration.public_send(operation, :test_table, :original, :renamed, batch_column_name: :status)
end
end
end
it 'creates the renamed column, syncing existing data' do
existing_record_1 = model.create!(status: 0, existing_column => 'existing')
existing_record_2 = model.create!(status: 0, existing_column => nil)
migration.send(operation, :test_table, :original, :renamed)
model.reset_column_information
expect(migration.column_exists?(:test_table, added_column)).to eq(true)
expect(existing_record_1.reload).to have_attributes(status: 0, original: 'existing', renamed: 'existing')
expect(existing_record_2.reload).to have_attributes(status: 0, original: nil, renamed: nil)
end
it 'installs triggers to sync new data' do
migration.public_send(operation, :test_table, :original, :renamed)
model.reset_column_information
new_record_1 = model.create!(status: 1, original: 'first')
new_record_2 = model.create!(status: 1, renamed: 'second')
expect(new_record_1.reload).to have_attributes(status: 1, original: 'first', renamed: 'first')
expect(new_record_2.reload).to have_attributes(status: 1, original: 'second', renamed: 'second')
new_record_1.update!(original: 'updated')
new_record_2.update!(renamed: nil)
expect(new_record_1.reload).to have_attributes(status: 1, original: 'updated', renamed: 'updated')
expect(new_record_2.reload).to have_attributes(status: 1, original: nil, renamed: nil)
end
end
describe '#rename_column_concurrently' do
before do
allow(migration).to receive(:transaction_open?).and_return(false)
migration.create_table :test_table do |t|
t.integer :status, null: false
t.text :original
t.text :other_column
end
end
it_behaves_like 'Setting up to rename a column' do
let(:operation) { :rename_column_concurrently }
let(:existing_column) { :original }
let(:added_column) { :renamed }
end
context 'when the column to rename does not exist' do
it 'raises an error' do
expect do
migration.rename_column_concurrently :test_table, :missing_column, :renamed
end.to raise_error('Column missing_column does not exist on test_table')
end
end
end
describe '#undo_cleanup_concurrent_column_rename' do
before do
allow(migration).to receive(:transaction_open?).and_return(false)
migration.create_table :test_table do |t|
t.integer :status, null: false
t.text :other_column
t.text :renamed
end
end
it_behaves_like 'Setting up to rename a column' do
let(:operation) { :undo_cleanup_concurrent_column_rename }
let(:existing_column) { :renamed }
let(:added_column) { :original }
end
context 'when the renamed column does not exist' do
it 'raises an error' do
expect do
migration.undo_cleanup_concurrent_column_rename :test_table, :original, :missing_column
end.to raise_error('Column missing_column does not exist on test_table')
end
end
end
shared_examples_for 'Cleaning up from renaming a column' do
let(:connection) { migration.connection }
before do
allow(migration).to receive(:transaction_open?).and_return(false)
migration.create_table :test_table do |t|
t.integer :status, null: false
t.text :original
t.text :other_column
end
migration.rename_column_concurrently :test_table, :original, :renamed
end
context 'when the helper is called repeatedly' do
before do
migration.public_send(operation, :test_table, :original, :renamed)
end
it 'does not make repeated attempts to cleanup' do
expect(migration).not_to receive(:remove_column)
expect do
migration.public_send(operation, :test_table, :original, :renamed)
end.not_to raise_error
end
end
context 'when the renamed column exists' do
let(:triggers) do
[
['trigger_7cc71f92fd63', 'function_for_trigger_7cc71f92fd63', before: 'insert'],
['trigger_f1a1f619636a', 'function_for_trigger_f1a1f619636a', before: 'update'],
['trigger_769a49938884', 'function_for_trigger_769a49938884', before: 'update']
]
end
it 'removes the sync triggers and renamed columns' do
triggers.each do |(trigger_name, function_name, event)|
expect_function_to_exist(function_name)
expect_valid_function_trigger(:test_table, trigger_name, function_name, event)
end
expect(migration.column_exists?(:test_table, added_column)).to eq(true)
migration.public_send(operation, :test_table, :original, :renamed)
expect(migration.column_exists?(:test_table, added_column)).to eq(false)
triggers.each do |(trigger_name, function_name, _)|
expect_trigger_not_to_exist(:test_table, trigger_name)
expect_function_not_to_exist(function_name)
end
end
end
end
describe '#undo_rename_column_concurrently' do
it_behaves_like 'Cleaning up from renaming a column' do
let(:operation) { :undo_rename_column_concurrently }
let(:added_column) { :renamed }
end
end
describe '#cleanup_concurrent_column_rename' do
it_behaves_like 'Cleaning up from renaming a column' do
let(:operation) { :cleanup_concurrent_column_rename }
let(:added_column) { :original }
end
end
end
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