From 4c55376d6ca5d229d822df95372126c119b9a687 Mon Sep 17 00:00:00 2001 From: Katharina Przybill <30441792+kathap@users.noreply.github.com> Date: Wed, 4 Sep 2024 17:43:47 +0200 Subject: [PATCH 1/2] Drop constraint quota_definitions_name_key On the name column in quota_defintions there were two unique constraints/indexes. One unique index with name qd_name_index, which is similar in postgres and mysql, and then an unique constraint on column name with name in postgres=quota_definitions_name_key and name in mysql=name. Since the unique index already exists, we don't need the unique constraint. Dropping it with this migration. --- ...e_constraint_quota_definitions_name_key.rb | 31 +++++++ ...straint_quota_definitions_name_key_spec.rb | 81 +++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100644 db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb create mode 100644 spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb diff --git a/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb b/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb new file mode 100644 index 00000000000..4bd8074c64e --- /dev/null +++ b/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb @@ -0,0 +1,31 @@ +Sequel.migration do + up do + if self.class.name.match?(/mysql/i) + alter_table :quota_definitions do + drop_constraint :name, if_exists: true + end + elsif self.class.name.match?(/postgres/i) + alter_table :quota_definitions do + drop_constraint :quota_definitions_name_key, if_exists: true + end + end + end + + down do + if self.class.name.match?(/mysql/i) + # mysql 5 is not so smart as mysql 8, prevent Mysql2::Error: Duplicate key name 'name' + alter_table :quota_definitions do + # rubocop:disable Sequel/ConcurrentIndex + drop_index :name, name: :name if @db.indexes(:quota_definitions).include?(:name) + # rubocop:enable Sequel/ConcurrentIndex + end + alter_table :quota_definitions do + add_unique_constraint :name, name: :name, if_not_exists: true + end + elsif self.class.name.match?(/postgres/i) + alter_table :quota_definitions do + add_unique_constraint :name, name: :quota_definitions_name_key, if_not_exists: true + end + end + end +end diff --git a/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb b/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb new file mode 100644 index 00000000000..4b762ca1c09 --- /dev/null +++ b/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb @@ -0,0 +1,81 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'migration to add or remove unique constraint on name column in quota_definitions table', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb' } + end + describe 'up migration' do + context 'the unique constraint on name column exists' do + it 'removes the unique constraint' do + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + end + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).not_to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + end + end + + context 'the unique constraint on name column does not exist' do + it 'does not throw an error' do + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).not_to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + end + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).not_to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + end + end + end + end + end + + describe 'down migration' do + context 'unique constraint on name column does not exist' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + end + + it 'adds the unique constraint' do + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).not_to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + end + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + end + end + end + + context 'unique constraint on name column does exist' do + it 'does not fail' do + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + end + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error + if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).to include(:name) + elsif db.database_type == :postgres + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + end + end + end + end +end From 580a8f1d312dc394398c9aba0f000bbb60c33789 Mon Sep 17 00:00:00 2001 From: Sven Krieger <37476281+svkrieger@users.noreply.github.com> Date: Tue, 15 Oct 2024 14:14:38 +0200 Subject: [PATCH 2/2] Adjust migration to support mysql 5.7 --- ...e_constraint_quota_definitions_name_key.rb | 42 +++--- ...straint_quota_definitions_name_key_spec.rb | 122 ++++++++++++------ 2 files changed, 105 insertions(+), 59 deletions(-) diff --git a/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb b/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb index 4bd8074c64e..7f564271763 100644 --- a/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb +++ b/db/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key.rb @@ -1,30 +1,36 @@ Sequel.migration do up do - if self.class.name.match?(/mysql/i) - alter_table :quota_definitions do - drop_constraint :name, if_exists: true + if database_type == :mysql + if indexes(:quota_definitions).include?(:name) + alter_table :quota_definitions do + # rubocop:disable Sequel/ConcurrentIndex + drop_index :name, name: :name + # rubocop:enable Sequel/ConcurrentIndex + end end - elsif self.class.name.match?(/postgres/i) - alter_table :quota_definitions do - drop_constraint :quota_definitions_name_key, if_exists: true + elsif database_type == :postgres + if indexes(:quota_definitions).include?(:quota_definitions_name_key) + alter_table :quota_definitions do + drop_constraint :quota_definitions_name_key + end end end end down do - if self.class.name.match?(/mysql/i) - # mysql 5 is not so smart as mysql 8, prevent Mysql2::Error: Duplicate key name 'name' - alter_table :quota_definitions do - # rubocop:disable Sequel/ConcurrentIndex - drop_index :name, name: :name if @db.indexes(:quota_definitions).include?(:name) - # rubocop:enable Sequel/ConcurrentIndex + if database_type == :mysql + unless indexes(:quota_definitions).include?(:name) + alter_table :quota_definitions do + # rubocop:disable Sequel/ConcurrentIndex + add_index :name, name: :name, unique: true + # rubocop:enable Sequel/ConcurrentIndex + end end - alter_table :quota_definitions do - add_unique_constraint :name, name: :name, if_not_exists: true - end - elsif self.class.name.match?(/postgres/i) - alter_table :quota_definitions do - add_unique_constraint :name, name: :quota_definitions_name_key, if_not_exists: true + elsif database_type == :postgres + unless indexes(:quota_definitions).include?(:quota_definitions_name_key) + alter_table :quota_definitions do + add_unique_constraint :name, name: :quota_definitions_name_key + end end end end diff --git a/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb b/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb index 4b762ca1c09..7881c3955f3 100644 --- a/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb +++ b/spec/migrations/20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb @@ -6,73 +6,113 @@ let(:migration_filename) { '20240808118000_drop_unique_constraint_quota_definitions_name_key_spec.rb' } end describe 'up migration' do - context 'the unique constraint on name column exists' do + context 'mysql' do + before do + skip if db.database_type != :mysql + end + it 'removes the unique constraint' do - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) - end + expect(db.indexes(:quota_definitions)).to include(:name) expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).not_to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) - end + expect(db.indexes(:quota_definitions)).not_to include(:name) end - context 'the unique constraint on name column does not exist' do + context 'unique constraint on name column does not exist' do + before do + db.drop_index :quota_definitions, :name, name: :name + end + it 'does not throw an error' do + expect(db.indexes(:quota_definitions)).not_to include(:name) expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).not_to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + expect(db.indexes(:quota_definitions)).not_to include(:name) + end + end + end + + context 'postgres' do + before do + skip if db.database_type != :postgres + end + + it 'removes the unique constraint' do + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) + end + + context 'unique constraint on name column does not exist' do + before do + db.alter_table :quota_definitions do + drop_constraint :quota_definitions_name_key end + end + + it 'does not throw an error' do + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).not_to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) - end + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) end end end end describe 'down migration' do - context 'unique constraint on name column does not exist' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + end + + context 'mysql' do before do - Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + skip if db.database_type != :mysql end it 'adds the unique constraint' do - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).not_to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) - end + expect(db.indexes(:quota_definitions)).not_to include(:name) expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql + expect(db.indexes(:quota_definitions)).to include(:name) + end + + context 'unique constraint on name column already exists' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + + db.alter_table :quota_definitions do + add_index :name, name: :name + end + end + + it 'does not fail' do + expect(db.indexes(:quota_definitions)).to include(:name) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error expect(db.indexes(:quota_definitions)).to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) end end end - context 'unique constraint on name column does exist' do - it 'does not fail' do + context 'postgres' do + before do + skip if db.database_type != :postgres + end + + it 'adds the unique constraint' do + expect(db.indexes(:quota_definitions)).not_to include(:quota_definitions_name_key) expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).to include(:name) - elsif db.database_type == :postgres - expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + end + + context 'unique constraint on name column already exists' do + before do + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + + db.alter_table :quota_definitions do + add_unique_constraint :name, name: :quota_definitions_name_key + end end - expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error - if db.database_type == :mysql - expect(db.indexes(:quota_definitions)).to include(:name) - elsif db.database_type == :postgres + + it 'does not fail' do + expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) + expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }.not_to raise_error expect(db.indexes(:quota_definitions)).to include(:quota_definitions_name_key) end end