Commit 55b16db3 authored by Krasimir Angelov's avatar Krasimir Angelov

Extract convert_to_bigint_column migration helper

We do a lot of "#{column})_convert_to_bigint", this extracts it in a
helper method, and updates rest of the code to use it.

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59957.
parent 04e86156
...@@ -905,6 +905,10 @@ module Gitlab ...@@ -905,6 +905,10 @@ module Gitlab
end end
end end
def convert_to_bigint_column(column)
"#{column}_convert_to_bigint"
end
# Initializes the conversion of a set of integer columns to bigint # Initializes the conversion of a set of integer columns to bigint
# #
# It can be used for converting both a Primary Key and any Foreign Keys # It can be used for converting both a Primary Key and any Foreign Keys
...@@ -948,7 +952,7 @@ module Gitlab ...@@ -948,7 +952,7 @@ module Gitlab
check_trigger_permissions!(table) check_trigger_permissions!(table)
conversions = columns.to_h { |column| [column, "#{column}_convert_to_bigint"] } conversions = columns.to_h { |column| [column, convert_to_bigint_column(column)] }
with_lock_retries do with_lock_retries do
conversions.each do |(source_column, temporary_name)| conversions.each do |(source_column, temporary_name)|
...@@ -975,7 +979,7 @@ module Gitlab ...@@ -975,7 +979,7 @@ module Gitlab
# columns - The name, or array of names, of the column(s) that we're converting to bigint. # columns - The name, or array of names, of the column(s) that we're converting to bigint.
def revert_initialize_conversion_of_integer_to_bigint(table, columns) def revert_initialize_conversion_of_integer_to_bigint(table, columns)
columns = Array.wrap(columns) columns = Array.wrap(columns)
temporary_columns = columns.map { |column| "#{column}_convert_to_bigint" } temporary_columns = columns.map { |column| convert_to_bigint_column(column) }
trigger_name = rename_trigger_name(table, columns, temporary_columns) trigger_name = rename_trigger_name(table, columns, temporary_columns)
remove_rename_triggers_for_postgresql(table, trigger_name) remove_rename_triggers_for_postgresql(table, trigger_name)
...@@ -1039,7 +1043,7 @@ module Gitlab ...@@ -1039,7 +1043,7 @@ module Gitlab
conversions = Array.wrap(columns).to_h do |column| conversions = Array.wrap(columns).to_h do |column|
raise ArgumentError, "Column #{column} does not exist on #{table}" unless column_exists?(table, column) raise ArgumentError, "Column #{column} does not exist on #{table}" unless column_exists?(table, column)
temporary_name = "#{column}_convert_to_bigint" temporary_name = convert_to_bigint_column(column)
raise ArgumentError, "Column #{temporary_name} does not exist on #{table}" unless column_exists?(table, temporary_name) raise ArgumentError, "Column #{temporary_name} does not exist on #{table}" unless column_exists?(table, temporary_name)
[column, temporary_name] [column, temporary_name]
...@@ -1075,7 +1079,7 @@ module Gitlab ...@@ -1075,7 +1079,7 @@ module Gitlab
job_class_name: 'CopyColumnUsingBackgroundMigrationJob', job_class_name: 'CopyColumnUsingBackgroundMigrationJob',
table_name: table, table_name: table,
column_name: primary_key, column_name: primary_key,
job_arguments: [columns, columns.map { |c| "#{c}_convert_to_bigint" }].to_json job_arguments: [columns, columns.map { |column| convert_to_bigint_column(column) }].to_json
]) ])
execute("DELETE FROM batched_background_migrations WHERE #{conditions}") execute("DELETE FROM batched_background_migrations WHERE #{conditions}")
......
...@@ -8,6 +8,10 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo ...@@ -8,6 +8,10 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo
let(:sub_batch_size) { 1000 } let(:sub_batch_size) { 1000 }
let(:pause_ms) { 0 } let(:pause_ms) { 0 }
let(:helpers) do
ActiveRecord::Migration.new.extend(Gitlab::Database::MigrationHelpers)
end
before do before do
ActiveRecord::Base.connection.execute(<<~SQL) ActiveRecord::Base.connection.execute(<<~SQL)
CREATE TABLE #{table_name} CREATE TABLE #{table_name}
...@@ -15,8 +19,8 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo ...@@ -15,8 +19,8 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo
id integer NOT NULL, id integer NOT NULL,
name character varying, name character varying,
fk integer NOT NULL, fk integer NOT NULL,
id_convert_to_bigint bigint DEFAULT 0 NOT NULL, #{helpers.convert_to_bigint_column(:id)} bigint DEFAULT 0 NOT NULL,
fk_convert_to_bigint bigint DEFAULT 0 NOT NULL, #{helpers.convert_to_bigint_column(:fk)} bigint DEFAULT 0 NOT NULL,
name_convert_to_text text DEFAULT 'no name' name_convert_to_text text DEFAULT 'no name'
); );
SQL SQL
...@@ -41,18 +45,20 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo ...@@ -41,18 +45,20 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo
let(:migration_class) { described_class.name } let(:migration_class) { described_class.name }
it 'copies all primary keys in range' do it 'copies all primary keys in range' do
copy_columns.perform(12, 15, table_name, 'id', sub_batch_size, pause_ms, 'id', 'id_convert_to_bigint') temporary_column = helpers.convert_to_bigint_column(:id)
copy_columns.perform(12, 15, table_name, 'id', sub_batch_size, pause_ms, 'id', temporary_column)
expect(test_table.where('id = id_convert_to_bigint').pluck(:id)).to contain_exactly(12, 15) expect(test_table.where("id = #{temporary_column}").pluck(:id)).to contain_exactly(12, 15)
expect(test_table.where(id_convert_to_bigint: 0).pluck(:id)).to contain_exactly(11, 19) expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(11, 19)
expect(test_table.all.count).to eq(4) expect(test_table.all.count).to eq(4)
end end
it 'copies all foreign keys in range' do it 'copies all foreign keys in range' do
copy_columns.perform(10, 14, table_name, 'id', sub_batch_size, pause_ms, 'fk', 'fk_convert_to_bigint') temporary_column = helpers.convert_to_bigint_column(:fk)
copy_columns.perform(10, 14, table_name, 'id', sub_batch_size, pause_ms, 'fk', temporary_column)
expect(test_table.where('fk = fk_convert_to_bigint').pluck(:id)).to contain_exactly(11, 12) expect(test_table.where("fk = #{temporary_column}").pluck(:id)).to contain_exactly(11, 12)
expect(test_table.where(fk_convert_to_bigint: 0).pluck(:id)).to contain_exactly(15, 19) expect(test_table.where(temporary_column => 0).pluck(:id)).to contain_exactly(15, 19)
expect(test_table.all.count).to eq(4) expect(test_table.all.count).to eq(4)
end end
...@@ -68,18 +74,20 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo ...@@ -68,18 +74,20 @@ RSpec.describe Gitlab::BackgroundMigration::CopyColumnUsingBackgroundMigrationJo
it 'copies multiple columns when given' do it 'copies multiple columns when given' do
columns_to_copy_from = %w[id fk] columns_to_copy_from = %w[id fk]
columns_to_copy_to = %w[id_convert_to_bigint fk_convert_to_bigint] id_tmp_column = helpers.convert_to_bigint_column('id')
fk_tmp_column = helpers.convert_to_bigint_column('fk')
columns_to_copy_to = [id_tmp_column, fk_tmp_column]
subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to) subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to)
expect(test_table.where('id = id_convert_to_bigint AND fk = fk_convert_to_bigint').pluck(:id)).to contain_exactly(11, 12, 15) expect(test_table.where("id = #{id_tmp_column} AND fk = #{fk_tmp_column}").pluck(:id)).to contain_exactly(11, 12, 15)
expect(test_table.where(id_convert_to_bigint: 0).where(fk_convert_to_bigint: 0).pluck(:id)).to contain_exactly(19) expect(test_table.where(id_tmp_column => 0).where(fk_tmp_column => 0).pluck(:id)).to contain_exactly(19)
expect(test_table.all.count).to eq(4) expect(test_table.all.count).to eq(4)
end end
it 'raises error when number of source and target columns does not match' do it 'raises error when number of source and target columns does not match' do
columns_to_copy_from = %w[id fk] columns_to_copy_from = %w[id fk]
columns_to_copy_to = %w[id_convert_to_bigint] columns_to_copy_to = [helpers.convert_to_bigint_column(:id)]
expect do expect do
subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to) subject.perform(10, 15, table_name, 'id', sub_batch_size, pause_ms, columns_to_copy_from, columns_to_copy_to)
......
...@@ -1703,10 +1703,16 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1703,10 +1703,16 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
end end
describe '#convert_to_bigint_column' do
it 'returns the name of the temporary column used to convert to bigint' do
expect(model.convert_to_bigint_column(:id)).to eq('id_convert_to_bigint')
end
end
describe '#initialize_conversion_of_integer_to_bigint' do describe '#initialize_conversion_of_integer_to_bigint' do
let(:table) { :test_table } let(:table) { :test_table }
let(:column) { :id } let(:column) { :id }
let(:tmp_column) { "#{column}_convert_to_bigint" } let(:tmp_column) { model.convert_to_bigint_column(column) }
before do before do
model.create_table table, id: false do |t| model.create_table table, id: false do |t|
...@@ -1775,11 +1781,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1775,11 +1781,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
context 'when multiple columns are given' do context 'when multiple columns are given' do
it 'creates the correct columns and installs the trigger' do it 'creates the correct columns and installs the trigger' do
columns_to_convert = %i[id non_nullable_column nullable_column] columns_to_convert = %i[id non_nullable_column nullable_column]
temporary_columns = %w[ temporary_columns = columns_to_convert.map { |column| model.convert_to_bigint_column(column) }
id_convert_to_bigint
non_nullable_column_convert_to_bigint
nullable_column_convert_to_bigint
]
expect(model).to receive(:add_column).with(table, temporary_columns[0], :bigint, default: 0, null: false) expect(model).to receive(:add_column).with(table, temporary_columns[0], :bigint, default: 0, null: false)
expect(model).to receive(:add_column).with(table, temporary_columns[1], :bigint, default: 0, null: false) expect(model).to receive(:add_column).with(table, temporary_columns[1], :bigint, default: 0, null: false)
...@@ -1808,11 +1810,12 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1808,11 +1810,12 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
let(:columns) { :id } let(:columns) { :id }
it 'removes column, trigger, and function' do it 'removes column, trigger, and function' do
trigger_name = model.rename_trigger_name(table, :id, :id_convert_to_bigint) temporary_column = model.convert_to_bigint_column(:id)
trigger_name = model.rename_trigger_name(table, :id, temporary_column)
model.revert_initialize_conversion_of_integer_to_bigint(table, columns) model.revert_initialize_conversion_of_integer_to_bigint(table, columns)
expect(model.column_exists?(table, :id_convert_to_bigint)).to eq(false) expect(model.column_exists?(table, temporary_column)).to eq(false)
expect_trigger_not_to_exist(table, trigger_name) expect_trigger_not_to_exist(table, trigger_name)
expect_function_not_to_exist(trigger_name) expect_function_not_to_exist(trigger_name)
end end
...@@ -1822,12 +1825,14 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1822,12 +1825,14 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
let(:columns) { [:id, :other_id] } let(:columns) { [:id, :other_id] }
it 'removes column, trigger, and function' do it 'removes column, trigger, and function' do
trigger_name = model.rename_trigger_name(table, columns, [:id_convert_to_bigint, :other_id_convert_to_bigint]) temporary_columns = columns.map { |column| model.convert_to_bigint_column(column) }
trigger_name = model.rename_trigger_name(table, columns, temporary_columns)
model.revert_initialize_conversion_of_integer_to_bigint(table, columns) model.revert_initialize_conversion_of_integer_to_bigint(table, columns)
expect(model.column_exists?(table, :id_convert_to_bigint)).to eq(false) temporary_columns.each do |column|
expect(model.column_exists?(table, :other_id_convert_to_bigint)).to eq(false) expect(model.column_exists?(table, column)).to eq(false)
end
expect_trigger_not_to_exist(table, trigger_name) expect_trigger_not_to_exist(table, trigger_name)
expect_function_not_to_exist(trigger_name) expect_function_not_to_exist(trigger_name)
end end
...@@ -1837,7 +1842,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1837,7 +1842,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
describe '#backfill_conversion_of_integer_to_bigint' do describe '#backfill_conversion_of_integer_to_bigint' do
let(:table) { :_test_backfill_table } let(:table) { :_test_backfill_table }
let(:column) { :id } let(:column) { :id }
let(:tmp_column) { "#{column}_convert_to_bigint" } let(:tmp_column) { model.convert_to_bigint_column(column) }
before do before do
model.create_table table, id: false do |t| model.create_table table, id: false do |t|
...@@ -1915,14 +1920,14 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1915,14 +1920,14 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
interval: 120, interval: 120,
batch_size: 2, batch_size: 2,
sub_batch_size: 1, sub_batch_size: 1,
job_arguments: [[column.to_s], ["#{column}_convert_to_bigint"]] job_arguments: [[column.to_s], [model.convert_to_bigint_column(column)]]
) )
end end
end end
context 'when multiple columns are being converted' do context 'when multiple columns are being converted' do
let(:other_column) { :other_id } let(:other_column) { :other_id }
let(:other_tmp_column) { "#{other_column}_convert_to_bigint" } let(:other_tmp_column) { model.convert_to_bigint_column(other_column) }
let(:columns) { [column, other_column] } let(:columns) { [column, other_column] }
it 'creates the batched migration tracking record' do it 'creates the batched migration tracking record' do
......
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