Commit 0e38a0ae authored by pbair's avatar pbair

Fix schema helpers for restricted migrations

Update concurrent schema helpers that combine DML and DDL operations to
require the correct mode of operation according to
`restrict_gitlab_migration`. Suppress any operations that don't fit
within the required mode of operation.
parent 95758f02
...@@ -692,6 +692,8 @@ module Gitlab ...@@ -692,6 +692,8 @@ module Gitlab
# batch_column_name - option for tables without a primary key, in this case # batch_column_name - option for tables without a primary key, in this case
# another unique integer column can be used. Example: :user_id # 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, limit: nil) def undo_cleanup_concurrent_column_type_change(table, column, old_type, type_cast_function: nil, batch_column_name: :id, limit: nil)
Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode!
temp_column = "#{column}_for_type_change" temp_column = "#{column}_for_type_change"
# Using a descriptive name that includes orinal column's name risks # Using a descriptive name that includes orinal column's name risks
...@@ -1639,7 +1641,9 @@ into similar problems in the future (e.g. when new tables are created). ...@@ -1639,7 +1641,9 @@ into similar problems in the future (e.g. when new tables are created).
old_value = Arel::Nodes::NamedFunction.new(type_cast_function, [old_value]) old_value = Arel::Nodes::NamedFunction.new(type_cast_function, [old_value])
end end
Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.with_suppressed do
update_column_in_batches(table, new, old_value, batch_column_name: batch_column_name) update_column_in_batches(table, new, old_value, batch_column_name: batch_column_name)
end
add_not_null_constraint(table, new) unless old_col.null add_not_null_constraint(table, new) unless old_col.null
......
...@@ -134,6 +134,8 @@ module Gitlab ...@@ -134,6 +134,8 @@ module Gitlab
# batch_column_name - option is for tables without primary key, in this # batch_column_name - option is for tables without primary key, in this
# case another unique integer column can be used. Example: :user_id # 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) def rename_column_concurrently(table, old_column, new_column, type: nil, batch_column_name: :id)
Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode!
setup_renamed_column(__callee__, table, old_column, new_column, type, batch_column_name) setup_renamed_column(__callee__, table, old_column, new_column, type, batch_column_name)
with_lock_retries do with_lock_retries do
...@@ -181,6 +183,8 @@ module Gitlab ...@@ -181,6 +183,8 @@ module Gitlab
# case another unique integer column can be used. Example: :user_id # 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) def undo_cleanup_concurrent_column_rename(table, old_column, new_column, type: nil, batch_column_name: :id)
Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas.require_ddl_mode!
setup_renamed_column(__callee__, table, new_column, old_column, type, batch_column_name) setup_renamed_column(__callee__, table, new_column, old_column, type, batch_column_name)
with_lock_retries do with_lock_retries do
......
...@@ -96,6 +96,12 @@ RSpec.describe Gitlab::Database::MigrationHelpers::V2 do ...@@ -96,6 +96,12 @@ RSpec.describe Gitlab::Database::MigrationHelpers::V2 do
expect(new_record_1.reload).to have_attributes(status: 1, original: 'updated', renamed: 'updated') 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) expect(new_record_2.reload).to have_attributes(status: 1, original: nil, renamed: nil)
end end
it 'requires the helper to run in ddl mode' do
expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_ddl_mode!)
migration.public_send(operation, :_test_table, :original, :renamed)
end
end end
describe '#rename_column_concurrently' do describe '#rename_column_concurrently' do
......
...@@ -1390,6 +1390,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1390,6 +1390,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
it 'reverses the operations of cleanup_concurrent_column_type_change' do it 'reverses the operations of cleanup_concurrent_column_type_change' do
expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_ddl_mode!)
expect(model).to receive(:check_trigger_permissions!).with(:users) expect(model).to receive(:check_trigger_permissions!).with(:users)
expect(model).to receive(:create_column_from).with( expect(model).to receive(:create_column_from).with(
...@@ -1415,6 +1417,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do ...@@ -1415,6 +1417,8 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
end end
it 'passes the type_cast_function, batch_column_name and limit' do it 'passes the type_cast_function, batch_column_name and limit' do
expect(Gitlab::Database::QueryAnalyzers::RestrictAllowedSchemas).to receive(:require_ddl_mode!)
expect(model).to receive(:column_exists?).with(:users, :other_batch_column).and_return(true) 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(:check_trigger_permissions!).with(:users)
......
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