diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb
index 3a342abe65d4e3b260ca48e2fb6c4b5947d7fee4..c1f4b8d5e2500366882cfe25c41b625a1d5eb5c9 100644
--- a/lib/gitlab/database/migration_helpers.rb
+++ b/lib/gitlab/database/migration_helpers.rb
@@ -79,63 +79,6 @@ def remove_timestamps(table_name, options = {})
end
end
- # @deprecated Use `create_table` in V2 instead
- #
- # Creates a new table, optionally allowing the caller to add check constraints to the table.
- # Aside from that addition, this method should behave identically to Rails' `create_table` method.
- #
- # Example:
- #
- # create_table_with_constraints :some_table do |t|
- # t.integer :thing, null: false
- # t.text :other_thing
- #
- # t.check_constraint :thing_is_not_null, 'thing IS NOT NULL'
- # t.text_limit :other_thing, 255
- # end
- #
- # See Rails' `create_table` for more info on the available arguments.
- def create_table_with_constraints(table_name, **options, &block)
- helper_context = self
-
- with_lock_retries do
- check_constraints = []
-
- create_table(table_name, **options) do |t|
- t.define_singleton_method(:check_constraint) do |name, definition|
- helper_context.send(:validate_check_constraint_name!, name) # rubocop:disable GitlabSecurity/PublicSend
-
- check_constraints << { name: name, definition: definition }
- end
-
- t.define_singleton_method(:text_limit) do |column_name, limit, name: nil|
- # rubocop:disable GitlabSecurity/PublicSend
- name = helper_context.send(:text_limit_name, table_name, column_name, name: name)
- helper_context.send(:validate_check_constraint_name!, name)
- # rubocop:enable GitlabSecurity/PublicSend
-
- column_name = helper_context.quote_column_name(column_name)
- definition = "char_length(#{column_name}) <= #{limit}"
-
- check_constraints << { name: name, definition: definition }
- end
-
- t.instance_eval(&block) unless block.nil?
- end
-
- next if check_constraints.empty?
-
- constraint_clauses = check_constraints.map do |constraint|
- "ADD CONSTRAINT #{quote_table_name(constraint[:name])} CHECK (#{constraint[:definition]})"
- end
-
- execute(<<~SQL)
- ALTER TABLE #{quote_table_name(table_name)}
- #{constraint_clauses.join(",\n")}
- SQL
- end
- end
-
# Creates a new index, concurrently
#
# Example:
diff --git a/lib/gitlab/database/migration_helpers/v2.rb b/lib/gitlab/database/migration_helpers/v2.rb
index b5b8b58681c10c0e1106d9f3c335bfcc33155fae..ef48d601eb9a8a3fc7ae0d2e09a933ad0fafd412 100644
--- a/lib/gitlab/database/migration_helpers/v2.rb
+++ b/lib/gitlab/database/migration_helpers/v2.rb
@@ -5,24 +5,6 @@ module Database
module MigrationHelpers
module V2
include Gitlab::Database::MigrationHelpers
-
- # Superseded by `create_table` override below
- def create_table_with_constraints(*_)
- raise <<~EOM
- #create_table_with_constraints is not supported anymore - use #create_table instead, for example:
-
- create_table :db_guides do |t|
- t.bigint :stars, default: 0, null: false
- t.text :title, limit: 128
- t.text :notes, limit: 1024
-
- t.check_constraint 'stars > 1000', name: 'so_many_stars'
- end
-
- See https://docs.gitlab.com/ee/development/database/strings_and_the_text_data_type.html
- EOM
- end
-
# Creates a new table, optionally allowing the caller to add text limit constraints to the table.
# This method only extends Rails' `create_table` method
#
diff --git a/rubocop/migration_helpers.rb b/rubocop/migration_helpers.rb
index d14b1bdd6bbbb6b4194bc9640839a8d8183f6c2f..db8fc079774b81384809613824f7a45d73e9b120 100644
--- a/rubocop/migration_helpers.rb
+++ b/rubocop/migration_helpers.rb
@@ -21,7 +21,7 @@ module MigrationHelpers
# or through a create/alter table (TABLE_METHODS)
ADD_COLUMN_METHODS = %i(add_column change_column_type_concurrently).freeze
- TABLE_METHODS = %i(create_table create_table_if_not_exists change_table create_table_with_constraints).freeze
+ TABLE_METHODS = %i(create_table create_table_if_not_exists change_table).freeze
def high_traffic_tables
@high_traffic_tables ||= rubocop_migrations_config.dig('Migration/UpdateLargeTable', 'HighTrafficTables')
diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb
index 3f6528558b1b9d304decf95717f762e3e9269cd6..982a06152a9b6f95d13391b3fbcea066d31575ba 100644
--- a/spec/lib/gitlab/database/migration_helpers_spec.rb
+++ b/spec/lib/gitlab/database/migration_helpers_spec.rb
@@ -120,157 +120,6 @@
end
end
- describe '#create_table_with_constraints' do
- let(:table_name) { :test_table }
- let(:column_attributes) do
- [
- { name: 'id', sql_type: 'bigint', null: false, default: nil },
- { name: 'created_at', sql_type: 'timestamp with time zone', null: false, default: nil },
- { name: 'updated_at', sql_type: 'timestamp with time zone', null: false, default: nil },
- { name: 'some_id', sql_type: 'integer', null: false, default: nil },
- { name: 'active', sql_type: 'boolean', null: false, default: 'true' },
- { name: 'name', sql_type: 'text', null: true, default: nil }
- ]
- end
-
- before do
- allow(model).to receive(:transaction_open?).and_return(true)
- end
-
- context 'when no check constraints are defined' do
- it 'creates the table as expected' do
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
- end
-
- expect_table_columns_to_match(column_attributes, table_name)
- end
- end
-
- context 'when check constraints are defined' do
- context 'when the text_limit is explicity named' do
- it 'creates the table as expected' do
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
-
- t.text_limit :name, 255, name: 'check_name_length'
- t.check_constraint :some_id_is_positive, 'some_id > 0'
- end
-
- expect_table_columns_to_match(column_attributes, table_name)
-
- expect_check_constraint(table_name, 'check_name_length', 'char_length(name) <= 255')
- expect_check_constraint(table_name, 'some_id_is_positive', 'some_id > 0')
- end
- end
-
- context 'when the text_limit is not named' do
- it 'creates the table as expected, naming the text limit' do
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
-
- t.text_limit :name, 255
- t.check_constraint :some_id_is_positive, 'some_id > 0'
- end
-
- expect_table_columns_to_match(column_attributes, table_name)
-
- expect_check_constraint(table_name, 'check_cda6f69506', 'char_length(name) <= 255')
- expect_check_constraint(table_name, 'some_id_is_positive', 'some_id > 0')
- end
- end
-
- it 'runs the change within a with_lock_retries' do
- expect(model).to receive(:with_lock_retries).ordered.and_yield
- expect(model).to receive(:create_table).ordered.and_call_original
- expect(model).to receive(:execute).with(<<~SQL).ordered
- ALTER TABLE "#{table_name}"\nADD CONSTRAINT "check_cda6f69506" CHECK (char_length("name") <= 255)
- SQL
-
- model.create_table_with_constraints table_name do |t|
- t.text :name
- t.text_limit :name, 255
- end
- end
-
- context 'when with_lock_retries re-runs the block' do
- it 'only creates constraint for unique definitions' do
- expected_sql = <<~SQL
- ALTER TABLE "#{table_name}"\nADD CONSTRAINT "check_cda6f69506" CHECK (char_length("name") <= 255)
- SQL
-
- expect(model).to receive(:create_table).twice.and_call_original
-
- expect(model).to receive(:execute).with(expected_sql).and_raise(ActiveRecord::LockWaitTimeout)
- expect(model).to receive(:execute).with(expected_sql).and_call_original
-
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
-
- t.text_limit :name, 255
- end
-
- expect_table_columns_to_match(column_attributes, table_name)
-
- expect_check_constraint(table_name, 'check_cda6f69506', 'char_length(name) <= 255')
- end
- end
-
- context 'when constraints are given invalid names' do
- let(:expected_max_length) { described_class::MAX_IDENTIFIER_NAME_LENGTH }
- let(:expected_error_message) { "The maximum allowed constraint name is #{expected_max_length} characters" }
-
- context 'when the explicit text limit name is not valid' do
- it 'raises an error' do
- too_long_length = expected_max_length + 1
-
- expect do
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
-
- t.text_limit :name, 255, name: ('a' * too_long_length)
- t.check_constraint :some_id_is_positive, 'some_id > 0'
- end
- end.to raise_error(expected_error_message)
- end
- end
-
- context 'when a check constraint name is not valid' do
- it 'raises an error' do
- too_long_length = expected_max_length + 1
-
- expect do
- model.create_table_with_constraints table_name do |t|
- t.timestamps_with_timezone
- t.integer :some_id, null: false
- t.boolean :active, null: false, default: true
- t.text :name
-
- t.text_limit :name, 255
- t.check_constraint ('a' * too_long_length), 'some_id > 0'
- end
- end.to raise_error(expected_error_message)
- end
- end
- end
- end
- end
-
describe '#add_concurrent_index' do
context 'outside a transaction' do
before do
diff --git a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
index a6a072e2caf7d2e2e4ab7201e72a505d3e158964..032cc12ab941a79804cc80f604b8f4bd11a67634 100644
--- a/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
+++ b/spec/rubocop/cop/migration/add_limit_to_text_columns_spec.rb
@@ -78,30 +78,6 @@ def up
end
RUBY
end
-
- context 'for migrations before 2021_09_10_00_00_00' do
- it 'when limit: attribute is used (which is not supported yet for this version): registers an offense' do
- allow(cop).to receive(:version).and_return(described_class::TEXT_LIMIT_ATTRIBUTE_ALLOWED_SINCE - 5)
-
- expect_offense(<<~RUBY)
- class TestTextLimits < ActiveRecord::Migration[6.0]
- def up
- create_table :test_text_limit_attribute do |t|
- t.integer :test_id, null: false
- t.text :name, limit: 100
- ^^^^ Text columns should always have a limit set (255 is suggested). Using limit: is not supported in this version. You can add a limit to a `text` column by using `add_text_limit` or `.text_limit` inside `create_table`
- end
-
- create_table_with_constraints :test_text_limit_attribute do |t|
- t.integer :test_id, null: false
- t.text :name, limit: 100
- ^^^^ Text columns should always have a limit set (255 is suggested). Using limit: is not supported in this version. You can add a limit to a `text` column by using `add_text_limit` or `.text_limit` inside `create_table`
- end
- end
- end
- RUBY
- end
- end
end
context 'when text array columns are defined without a limit' do