From afce2af15316075ba2d3b06b23f8abf461aad899 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Tue, 17 Jan 2023 19:10:35 +0200 Subject: [PATCH 01/11] Add generator for partitioning FKs --- lib/generators/gitlab/ci/partitioning/USAGE | 29 ++++ .../ci/partitioning/foreign_keys_generator.rb | 129 ++++++++++++++++++ .../foreign_key_definition.rb.template | 32 +++++ .../templates/foreign_key_index.rb.template | 17 +++ .../templates/foreign_key_removal.rb.template | 30 ++++ .../foreign_key_validation.rb.template | 17 +++ .../foreign_keys_generator_spec.rb | 49 +++++++ 7 files changed, 303 insertions(+) create mode 100644 lib/generators/gitlab/ci/partitioning/USAGE create mode 100644 lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb create mode 100644 lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template create mode 100644 lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template create mode 100644 lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template create mode 100644 lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template create mode 100644 spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb diff --git a/lib/generators/gitlab/ci/partitioning/USAGE b/lib/generators/gitlab/ci/partitioning/USAGE new file mode 100644 index 00000000000000..8690a246b9b0a6 --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/USAGE @@ -0,0 +1,29 @@ +This generator creates the migrations needed for updating the foreign keys when partitioning the tables + +Usage: + rails generate gitlab:ci:partitioning:foreign_keys [options] + +Options: + [--skip-namespace], [--no-skip-namespace] # Skip namespace (affects only isolated engines) + [--skip-collision-check], [--no-skip-collision-check] # Skip collision check + --target=TARGET # Target table name + --source=SOURCE # Source table name + [--partitioning-column=PARTITIONING_COLUMN] # The column that is used for partitioning + # Default: partition_id + [--database=DATABASE] # Database name connection + # Default: ci + +Runtime options: + -f, [--force] # Overwrite files that already exist + -p, [--pretend], [--no-pretend] # Run but do not make any changes + -q, [--quiet], [--no-quiet] # Suppress status output + -s, [--skip], [--no-skip] # Skip files that already exist + +Example: + rails generate gitlab:ci:partitioning:foreign_keys --target ci_builds --source ci_build_trace_chunks + + This will generate: + create db/post_migrate/20230118112350_add_fk_index_to_ci_build_trace_chunks_on_partition_id_and_build_id.rb + create db/post_migrate/20230118112351_add_fk_to_ci_build_trace_chunks_on_partition_id_and_build_id.rb + create db/post_migrate/20230118112352_validate_fk_on_ci_build_trace_chunks_partition_id_and_build_id.rb + create db/post_migrate/20230118112353_remove_fk_to_ci_builds_ci_build_trace_chunks_on_build_id.rb diff --git a/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb new file mode 100644 index 00000000000000..b05d9dd552de70 --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record/migration' + +module Gitlab + module Ci + module Partitioning + class ForeignKeysGenerator < Rails::Generators::Base + include ActiveRecord::Generators::Migration + + desc 'This generator creates the migrations needed for updating the foreign keys when partitioning the tables' + + source_root File.expand_path('templates', __dir__) + + class_option :target, type: :string, required: true, desc: 'Target table name' + class_option :source, type: :string, required: true, desc: 'Source table name' + class_option :partitioning_column, type: :string, default: :partition_id, + desc: 'The column that is used for partitioning' + class_option :database, type: :string, default: :ci, + desc: 'Database name connection' + + def create_fk_index_migration + migration_template( + '../templates/foreign_key_index.rb.template', + fk_index_file_name) + end + + def create_fk_definition_migration + migration_template( + '../templates/foreign_key_definition.rb.template', + fk_definition_file_name) + end + + def create_fk_validation_migration + migration_template( + '../templates/foreign_key_validation.rb.template', + fk_validation_file_name) + end + + def remove_old_fk_migration + migration_template( + '../templates/foreign_key_removal.rb.template', + fk_removal_file_name) + end + + private + + def fk_index_file_name + post_migration_file_path( + "add_fk_index_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_definition_file_name + post_migration_file_path( + "add_fk_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_validation_file_name + post_migration_file_path( + "validate_fk_on_#{source_table_name}_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_removal_file_name + post_migration_file_path( + "remove_fk_to_#{target_table_name}_#{source_table_name}_on_#{foreign_key_column}.rb") + end + + def post_migration_file_path(name) + File.join(db_migrate_path, name) + end + + def db_migrate_path + super.sub('migrate', 'post_migrate') + end + + def source_table_name + options[:source] + end + + def target_table_name + options[:target] + end + + def partitioning_column + options[:partitioning_column] + end + + def foreign_keys_candidates + connection + .foreign_keys(source_table_name) + .select { |fk| fk.to_table == target_table_name } + .reject { |fk| fk.name.end_with?('_p') } + end + + def fk_candidate + @fk_candidate ||= foreign_keys_candidates.first + end + + def foreign_key_name + fk_candidate.name + end + + def partitioned_foreign_key_name + "#{foreign_key_name}_p" + end + + def foreign_key_column + fk_candidate.column + end + + def fk_on_delete_option + fk_candidate.on_delete + end + + def fk_target_column + fk_candidate.primary_key + end + + def connection + Gitlab::Database + .database_base_models + .fetch(options[:database]) + .connection + end + end + end + end +end diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template new file mode 100644 index 00000000000000..cde089c235e95d --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= partitioned_foreign_key_name %> + PARTITION_COLUMN = :<%= partitioning_column %> + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [PARTITION_COLUMN, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :<%= fk_on_delete_option %>, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists(SOURCE_TABLE_NAME, name: FK_NAME) + end + end +end diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template new file mode 100644 index 00000000000000..4896d9313333fa --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + INDEX_NAME = :index_<%= source_table_name -%>_on_<%= partitioning_column -%>_<%= foreign_key_column %> + TABLE_NAME = :<%= source_table_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + add_concurrent_index(TABLE_NAME, COLUMNS, name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end +end diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template new file mode 100644 index 00000000000000..cb781299b9fea4 --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :<%= source_table_name %> + TARGET_TABLE_NAME = :<%= target_table_name %> + COLUMN = :<%= foreign_key_column %> + TARGET_COLUMN = :<%= fk_target_column %> + FK_NAME = :<%= foreign_key_name %> + + def up + with_lock_retries do + remove_foreign_key_if_exists(SOURCE_TABLE_NAME, name: FK_NAME) + end + end + + def down + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: COLUMN, + target_column: TARGET_COLUMN, + validate: true, + reverse_lock_order: true, + on_delete: :<%= fk_on_delete_option %>, + name: FK_NAME + ) + end +end diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template new file mode 100644 index 00000000000000..bad7d17a51b958 --- /dev/null +++ b/lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Database::Migration.current_version %>] + disable_ddl_transaction! + + TABLE_NAME = :<%= source_table_name %> + FK_NAME = :<%= partitioned_foreign_key_name %> + COLUMNS = [:<%= partitioning_column -%>, :<%= foreign_key_column -%>] + + def up + validate_foreign_key(TABLE_NAME, COLUMNS, name: FK_NAME) + end + + def down + # no-op + end +end diff --git a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb new file mode 100644 index 00000000000000..8af306dbf5217b --- /dev/null +++ b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Ci::Partitioning::ForeignKeysGenerator, :silence_stdout, category: :continuous_integration do + include MigrationsHelpers + + before do + ActiveRecord::Schema.define do + create_table :tmp_builds, force: true do |t| + t.integer :partition_id + t.index [:id, :partition_id], unique: true + end + + create_table :tmp_metadata, force: true do |t| + t.integer :partition_id + t.references :builds, foreign_key: { to_table: :tmp_builds, on_delete: :cascade } + end + end + end + + after do + FileUtils.rm_rf(destination_root) + end + + def run_generator(args = generator_args, config = generator_config) + described_class.start(args, config) + end + + def migrations_paths + [migrations_dir] + end + + let(:generator_args) { ['--source', 'tmp_metadata', '--target', 'tmp_builds', '--database', 'main'] } + let(:generator_config) { { destination_root: destination_root } } + let(:destination_root) { File.expand_path("../tmp", __dir__) } + let(:migrations_dir) { File.join(destination_root, 'db', 'post_migrate') } + + it 'generates foreign key migrations' do + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTmpMetadataOnPartitionIdAndBuildsId + AddFkToTmpMetadataOnPartitionIdAndBuildsId + ValidateFkOnTmpMetadataPartitionIdAndBuildsId + RemoveFkToTmpBuildsTmpMetadataOnBuildsId + ]) + end +end -- GitLab From 06dd744247675f3ffd704681c99d56fc45dce496 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Thu, 19 Jan 2023 11:47:59 +0200 Subject: [PATCH 02/11] Include generator in the partitioning docs --- doc/development/database/table_partitioning.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index fe92ebc768a494..e2c604741385f5 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -381,6 +381,12 @@ result in a `Key is still referenced from table ...` error and updating the partition column on the source table would raise a `Key is not present in table ...` error. +This migration can be automatically generated using: + +```shell +rails generate gitlab:ci:partitioning:foreign_keys --target target_table_name --source source_table_name +``` + ### Step 5 - Swap primary key Swap the primary key including the partitioning key column. This can be done only after -- GitLab From 7acfa6ae740609d2322be22feb1b9007f2f73d62 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Fri, 20 Jan 2023 17:47:17 +0200 Subject: [PATCH 03/11] Execute generated migrations --- .../foreign_keys_generator_spec.rb | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb index 8af306dbf5217b..c880320d120315 100644 --- a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb @@ -2,17 +2,18 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Partitioning::ForeignKeysGenerator, :silence_stdout, category: :continuous_integration do +RSpec.describe Gitlab::Ci::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, +category: :continuous_integration do include MigrationsHelpers before do ActiveRecord::Schema.define do - create_table :tmp_builds, force: true do |t| + create_table :tmp_builds, force: :cascade do |t| t.integer :partition_id t.index [:id, :partition_id], unique: true end - create_table :tmp_metadata, force: true do |t| + create_table :tmp_metadata, force: :cascade do |t| t.integer :partition_id t.references :builds, foreign_key: { to_table: :tmp_builds, on_delete: :cascade } end @@ -21,20 +22,19 @@ after do FileUtils.rm_rf(destination_root) - end - def run_generator(args = generator_args, config = generator_config) - described_class.start(args, config) - end + table(:schema_migrations).where(version: migrations.map(&:version)).delete_all - def migrations_paths - [migrations_dir] + active_record_base.connection.execute(<<~SQL) + DROP TABLE tmp_metadata; + DROP TABLE tmp_builds; + SQL end + let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } + let(:generator_args) { ['--source', 'tmp_metadata', '--target', 'tmp_builds', '--database', 'main'] } let(:generator_config) { { destination_root: destination_root } } - let(:destination_root) { File.expand_path("../tmp", __dir__) } - let(:migrations_dir) { File.join(destination_root, 'db', 'post_migrate') } it 'generates foreign key migrations' do run_generator @@ -45,5 +45,44 @@ def migrations_paths ValidateFkOnTmpMetadataPartitionIdAndBuildsId RemoveFkToTmpBuildsTmpMetadataOnBuildsId ]) + + schema_migrate_up! + + fks = Gitlab::Database::PostgresForeignKey + .by_referenced_table_identifier('public.tmp_builds') + .by_constrained_table_identifier('public.tmp_metadata') + + expect(fks.size).to eq(1) + + foreign_key = fks.first + + expect(foreign_key.name).to end_with('_p') + expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) + expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) + expect(foreign_key.on_delete_action).to eq('cascade') + # needs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108742 + # expect(foreign_key.on_update_action).to eq('cascade') + + index = active_record_base.connection.indexes('tmp_metadata').find do |index| + index.columns == %w[partition_id builds_id] + end + + expect(index).to be_present + end + + def run_generator(args = generator_args, config = generator_config) + described_class.start(args, config) + end + + # We want to execute only the newly generated migrations + def migrations_paths + [File.join(destination_root, 'db', 'post_migrate')] + end + + # There is no need to migrate down before executing the tests because these + # migrations were not already executed and we don't need to run it after + # the tests because we're removing the tables. + def schema_migrate_down! + # no-op end end -- GitLab From cad644549ba0be05b94dde830362938e34eb5663 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 23 Jan 2023 11:05:55 +0200 Subject: [PATCH 04/11] Allow user to select FK definition --- .../ci/partitioning/foreign_keys_generator.rb | 26 ++++++++++++++++- .../foreign_keys_generator_spec.rb | 28 +++++++++---------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb index b05d9dd552de70..9c44b6918bb27c 100644 --- a/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb +++ b/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb @@ -94,7 +94,7 @@ def foreign_keys_candidates end def fk_candidate - @fk_candidate ||= foreign_keys_candidates.first + @fk_candidate ||= select_foreign_key end def foreign_key_name @@ -123,6 +123,30 @@ def connection .fetch(options[:database]) .connection end + + def select_foreign_key + case foreign_keys_candidates.size + when 0 + raise Thor::InvocationError, "No FK found between #{source_table_name} and #{target_table_name}" + when 1 + foreign_keys_candidates.first + else + select_fk_from_user_input + end + end + + def select_fk_from_user_input + options = (0...foreign_keys_candidates.size).to_a.map(&:to_s) + + say "There are multiple FKs between #{source_table_name} and #{target_table_name}:" + foreign_keys_candidates.each.with_index do |fk, index| + say "\t#{index} : #{fk}" + end + + input = ask("Please select one:", limited_to: options, default: '0') + + foreign_keys_candidates.fetch(input.to_i) + end end end end diff --git a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb index c880320d120315..386cac4e4d81d9 100644 --- a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb @@ -3,19 +3,19 @@ require 'spec_helper' RSpec.describe Gitlab::Ci::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, -category: :continuous_integration do +feature_category: :continuous_integration do include MigrationsHelpers before do ActiveRecord::Schema.define do - create_table :tmp_builds, force: :cascade do |t| + create_table :_test_tmp_builds, force: :cascade do |t| t.integer :partition_id t.index [:id, :partition_id], unique: true end - create_table :tmp_metadata, force: :cascade do |t| + create_table :_test_tmp_metadata, force: :cascade do |t| t.integer :partition_id - t.references :builds, foreign_key: { to_table: :tmp_builds, on_delete: :cascade } + t.references :builds, foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } end end end @@ -26,31 +26,31 @@ table(:schema_migrations).where(version: migrations.map(&:version)).delete_all active_record_base.connection.execute(<<~SQL) - DROP TABLE tmp_metadata; - DROP TABLE tmp_builds; + DROP TABLE _test_tmp_metadata; + DROP TABLE _test_tmp_builds; SQL end let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } - let(:generator_args) { ['--source', 'tmp_metadata', '--target', 'tmp_builds', '--database', 'main'] } + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', '_test_tmp_builds', '--database', 'main'] } let(:generator_config) { { destination_root: destination_root } } it 'generates foreign key migrations' do run_generator expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ - AddFkIndexToTmpMetadataOnPartitionIdAndBuildsId - AddFkToTmpMetadataOnPartitionIdAndBuildsId - ValidateFkOnTmpMetadataPartitionIdAndBuildsId - RemoveFkToTmpBuildsTmpMetadataOnBuildsId + AddFkIndexToTestTmpMetadataOnPartitionIdAndBuildsId + AddFkToTestTmpMetadataOnPartitionIdAndBuildsId + ValidateFkOnTestTmpMetadataPartitionIdAndBuildsId + RemoveFkToTestTmpBuildsTestTmpMetadataOnBuildsId ]) schema_migrate_up! fks = Gitlab::Database::PostgresForeignKey - .by_referenced_table_identifier('public.tmp_builds') - .by_constrained_table_identifier('public.tmp_metadata') + .by_referenced_table_identifier('public._test_tmp_builds') + .by_constrained_table_identifier('public._test_tmp_metadata') expect(fks.size).to eq(1) @@ -63,7 +63,7 @@ # needs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108742 # expect(foreign_key.on_update_action).to eq('cascade') - index = active_record_base.connection.indexes('tmp_metadata').find do |index| + index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| index.columns == %w[partition_id builds_id] end -- GitLab From 6ae1fa33cbdc68575384299133500eb23316aeaa Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 23 Jan 2023 11:58:53 +0200 Subject: [PATCH 05/11] Remove Ci namespace --- .../database/table_partitioning.md | 2 +- .../ci/partitioning/foreign_keys_generator.rb | 153 ------------------ .../gitlab/{ci => }/partitioning/USAGE | 4 +- .../partitioning/foreign_keys_generator.rb | 151 +++++++++++++++++ .../foreign_key_definition.rb.template | 0 .../templates/foreign_key_index.rb.template | 0 .../templates/foreign_key_removal.rb.template | 0 .../foreign_key_validation.rb.template | 0 .../foreign_keys_generator_spec.rb | 2 +- 9 files changed, 155 insertions(+), 157 deletions(-) delete mode 100644 lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb rename lib/generators/gitlab/{ci => }/partitioning/USAGE (90%) create mode 100644 lib/generators/gitlab/partitioning/foreign_keys_generator.rb rename lib/generators/gitlab/{ci => }/partitioning/templates/foreign_key_definition.rb.template (100%) rename lib/generators/gitlab/{ci => }/partitioning/templates/foreign_key_index.rb.template (100%) rename lib/generators/gitlab/{ci => }/partitioning/templates/foreign_key_removal.rb.template (100%) rename lib/generators/gitlab/{ci => }/partitioning/templates/foreign_key_validation.rb.template (100%) rename spec/lib/generators/gitlab/{ci => }/partitioning/foreign_keys_generator_spec.rb (96%) diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index e2c604741385f5..d1b77c2a8505ae 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -384,7 +384,7 @@ partition column on the source table would raise a This migration can be automatically generated using: ```shell -rails generate gitlab:ci:partitioning:foreign_keys --target target_table_name --source source_table_name +rails generate gitlab:partitioning:foreign_keys --target target_table_name --source source_table_name ``` ### Step 5 - Swap primary key diff --git a/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb deleted file mode 100644 index 9c44b6918bb27c..00000000000000 --- a/lib/generators/gitlab/ci/partitioning/foreign_keys_generator.rb +++ /dev/null @@ -1,153 +0,0 @@ -# frozen_string_literal: true - -require 'rails/generators' -require 'rails/generators/active_record/migration' - -module Gitlab - module Ci - module Partitioning - class ForeignKeysGenerator < Rails::Generators::Base - include ActiveRecord::Generators::Migration - - desc 'This generator creates the migrations needed for updating the foreign keys when partitioning the tables' - - source_root File.expand_path('templates', __dir__) - - class_option :target, type: :string, required: true, desc: 'Target table name' - class_option :source, type: :string, required: true, desc: 'Source table name' - class_option :partitioning_column, type: :string, default: :partition_id, - desc: 'The column that is used for partitioning' - class_option :database, type: :string, default: :ci, - desc: 'Database name connection' - - def create_fk_index_migration - migration_template( - '../templates/foreign_key_index.rb.template', - fk_index_file_name) - end - - def create_fk_definition_migration - migration_template( - '../templates/foreign_key_definition.rb.template', - fk_definition_file_name) - end - - def create_fk_validation_migration - migration_template( - '../templates/foreign_key_validation.rb.template', - fk_validation_file_name) - end - - def remove_old_fk_migration - migration_template( - '../templates/foreign_key_removal.rb.template', - fk_removal_file_name) - end - - private - - def fk_index_file_name - post_migration_file_path( - "add_fk_index_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") - end - - def fk_definition_file_name - post_migration_file_path( - "add_fk_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") - end - - def fk_validation_file_name - post_migration_file_path( - "validate_fk_on_#{source_table_name}_#{partitioning_column}_and_#{foreign_key_column}.rb") - end - - def fk_removal_file_name - post_migration_file_path( - "remove_fk_to_#{target_table_name}_#{source_table_name}_on_#{foreign_key_column}.rb") - end - - def post_migration_file_path(name) - File.join(db_migrate_path, name) - end - - def db_migrate_path - super.sub('migrate', 'post_migrate') - end - - def source_table_name - options[:source] - end - - def target_table_name - options[:target] - end - - def partitioning_column - options[:partitioning_column] - end - - def foreign_keys_candidates - connection - .foreign_keys(source_table_name) - .select { |fk| fk.to_table == target_table_name } - .reject { |fk| fk.name.end_with?('_p') } - end - - def fk_candidate - @fk_candidate ||= select_foreign_key - end - - def foreign_key_name - fk_candidate.name - end - - def partitioned_foreign_key_name - "#{foreign_key_name}_p" - end - - def foreign_key_column - fk_candidate.column - end - - def fk_on_delete_option - fk_candidate.on_delete - end - - def fk_target_column - fk_candidate.primary_key - end - - def connection - Gitlab::Database - .database_base_models - .fetch(options[:database]) - .connection - end - - def select_foreign_key - case foreign_keys_candidates.size - when 0 - raise Thor::InvocationError, "No FK found between #{source_table_name} and #{target_table_name}" - when 1 - foreign_keys_candidates.first - else - select_fk_from_user_input - end - end - - def select_fk_from_user_input - options = (0...foreign_keys_candidates.size).to_a.map(&:to_s) - - say "There are multiple FKs between #{source_table_name} and #{target_table_name}:" - foreign_keys_candidates.each.with_index do |fk, index| - say "\t#{index} : #{fk}" - end - - input = ask("Please select one:", limited_to: options, default: '0') - - foreign_keys_candidates.fetch(input.to_i) - end - end - end - end -end diff --git a/lib/generators/gitlab/ci/partitioning/USAGE b/lib/generators/gitlab/partitioning/USAGE similarity index 90% rename from lib/generators/gitlab/ci/partitioning/USAGE rename to lib/generators/gitlab/partitioning/USAGE index 8690a246b9b0a6..40b7cf0c53e85f 100644 --- a/lib/generators/gitlab/ci/partitioning/USAGE +++ b/lib/generators/gitlab/partitioning/USAGE @@ -1,7 +1,7 @@ This generator creates the migrations needed for updating the foreign keys when partitioning the tables Usage: - rails generate gitlab:ci:partitioning:foreign_keys [options] + rails generate gitlab:partitioning:foreign_keys [options] Options: [--skip-namespace], [--no-skip-namespace] # Skip namespace (affects only isolated engines) @@ -20,7 +20,7 @@ Runtime options: -s, [--skip], [--no-skip] # Skip files that already exist Example: - rails generate gitlab:ci:partitioning:foreign_keys --target ci_builds --source ci_build_trace_chunks + rails generate gitlab:partitioning:foreign_keys --target ci_builds --source ci_build_trace_chunks This will generate: create db/post_migrate/20230118112350_add_fk_index_to_ci_build_trace_chunks_on_partition_id_and_build_id.rb diff --git a/lib/generators/gitlab/partitioning/foreign_keys_generator.rb b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb new file mode 100644 index 00000000000000..e013188c75330e --- /dev/null +++ b/lib/generators/gitlab/partitioning/foreign_keys_generator.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'rails/generators' +require 'rails/generators/active_record/migration' + +module Gitlab + module Partitioning + class ForeignKeysGenerator < Rails::Generators::Base + include ActiveRecord::Generators::Migration + + desc 'This generator creates the migrations needed for updating the foreign keys when partitioning the tables' + + source_root File.expand_path('templates', __dir__) + + class_option :target, type: :string, required: true, desc: 'Target table name' + class_option :source, type: :string, required: true, desc: 'Source table name' + class_option :partitioning_column, type: :string, default: :partition_id, + desc: 'The column that is used for partitioning' + class_option :database, type: :string, default: :ci, + desc: 'Database name connection' + + def create_fk_index_migration + migration_template( + '../templates/foreign_key_index.rb.template', + fk_index_file_name) + end + + def create_fk_definition_migration + migration_template( + '../templates/foreign_key_definition.rb.template', + fk_definition_file_name) + end + + def create_fk_validation_migration + migration_template( + '../templates/foreign_key_validation.rb.template', + fk_validation_file_name) + end + + def remove_old_fk_migration + migration_template( + '../templates/foreign_key_removal.rb.template', + fk_removal_file_name) + end + + private + + def fk_index_file_name + post_migration_file_path( + "add_fk_index_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_definition_file_name + post_migration_file_path( + "add_fk_to_#{source_table_name}_on_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_validation_file_name + post_migration_file_path( + "validate_fk_on_#{source_table_name}_#{partitioning_column}_and_#{foreign_key_column}.rb") + end + + def fk_removal_file_name + post_migration_file_path( + "remove_fk_to_#{target_table_name}_#{source_table_name}_on_#{foreign_key_column}.rb") + end + + def post_migration_file_path(name) + File.join(db_migrate_path, name) + end + + def db_migrate_path + super.sub('migrate', 'post_migrate') + end + + def source_table_name + options[:source] + end + + def target_table_name + options[:target] + end + + def partitioning_column + options[:partitioning_column] + end + + def foreign_keys_candidates + connection + .foreign_keys(source_table_name) + .select { |fk| fk.to_table == target_table_name } + .reject { |fk| fk.name.end_with?('_p') } + end + + def fk_candidate + @fk_candidate ||= select_foreign_key + end + + def foreign_key_name + fk_candidate.name + end + + def partitioned_foreign_key_name + "#{foreign_key_name}_p" + end + + def foreign_key_column + fk_candidate.column + end + + def fk_on_delete_option + fk_candidate.on_delete + end + + def fk_target_column + fk_candidate.primary_key + end + + def connection + Gitlab::Database + .database_base_models + .fetch(options[:database]) + .connection + end + + def select_foreign_key + case foreign_keys_candidates.size + when 0 + raise Thor::InvocationError, "No FK found between #{source_table_name} and #{target_table_name}" + when 1 + foreign_keys_candidates.first + else + select_fk_from_user_input + end + end + + def select_fk_from_user_input + options = (0...foreign_keys_candidates.size).to_a.map(&:to_s) + + say "There are multiple FKs between #{source_table_name} and #{target_table_name}:" + foreign_keys_candidates.each.with_index do |fk, index| + say "\t#{index} : #{fk}" + end + + input = ask("Please select one:", limited_to: options, default: '0') + + foreign_keys_candidates.fetch(input.to_i) + end + end + end +end diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template similarity index 100% rename from lib/generators/gitlab/ci/partitioning/templates/foreign_key_definition.rb.template rename to lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template similarity index 100% rename from lib/generators/gitlab/ci/partitioning/templates/foreign_key_index.rb.template rename to lib/generators/gitlab/partitioning/templates/foreign_key_index.rb.template diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template similarity index 100% rename from lib/generators/gitlab/ci/partitioning/templates/foreign_key_removal.rb.template rename to lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template diff --git a/lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template similarity index 100% rename from lib/generators/gitlab/ci/partitioning/templates/foreign_key_validation.rb.template rename to lib/generators/gitlab/partitioning/templates/foreign_key_validation.rb.template diff --git a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb similarity index 96% rename from spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb rename to spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb index 386cac4e4d81d9..b620b015614718 100644 --- a/spec/lib/generators/gitlab/ci/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Gitlab::Ci::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, +RSpec.describe Gitlab::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, feature_category: :continuous_integration do include MigrationsHelpers -- GitLab From 63525fcf0dcd64006c3f58d718d883582bc6f9eb Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 23 Jan 2023 11:59:38 +0200 Subject: [PATCH 06/11] Add script helper --- doc/development/database/table_partitioning.md | 2 +- scripts/partitioning/generate-fk | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) create mode 100755 scripts/partitioning/generate-fk diff --git a/doc/development/database/table_partitioning.md b/doc/development/database/table_partitioning.md index d1b77c2a8505ae..0d5e3c233f626c 100644 --- a/doc/development/database/table_partitioning.md +++ b/doc/development/database/table_partitioning.md @@ -384,7 +384,7 @@ partition column on the source table would raise a This migration can be automatically generated using: ```shell -rails generate gitlab:partitioning:foreign_keys --target target_table_name --source source_table_name +./scripts/partitioning/generate-fk --source source_table_name --target target_table_name ``` ### Step 5 - Swap primary key diff --git a/scripts/partitioning/generate-fk b/scripts/partitioning/generate-fk new file mode 100755 index 00000000000000..4f2dac1d61e2cc --- /dev/null +++ b/scripts/partitioning/generate-fk @@ -0,0 +1,3 @@ +#!/bin/bash + +exec bundle exec rails generate gitlab:partitioning:foreign_keys "$@" -- GitLab From b70044602f9a26a1cb3ca8e7c6391a589aac60d2 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 23 Jan 2023 15:17:49 +0200 Subject: [PATCH 07/11] Use reverse lock order when removing FKs --- .../templates/foreign_key_definition.rb.template | 7 ++++++- .../partitioning/templates/foreign_key_removal.rb.template | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template index cde089c235e95d..f2f9acea923ce7 100644 --- a/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_definition.rb.template @@ -26,7 +26,12 @@ class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Data def down with_lock_retries do - remove_foreign_key_if_exists(SOURCE_TABLE_NAME, name: FK_NAME) + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) end end end diff --git a/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template index cb781299b9fea4..b4e881074ad936 100644 --- a/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template +++ b/lib/generators/gitlab/partitioning/templates/foreign_key_removal.rb.template @@ -11,7 +11,12 @@ class <%= migration_class_name %> < Gitlab::Database::Migration[<%= Gitlab::Data def up with_lock_retries do - remove_foreign_key_if_exists(SOURCE_TABLE_NAME, name: FK_NAME) + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) end end -- GitLab From 28e0b7cdab0cdd15bb477eb6308c7eadd5317979 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Mon, 23 Jan 2023 16:04:13 +0200 Subject: [PATCH 08/11] Add tests for multiple FKs --- .../foreign_keys_generator_spec.rb | 90 ++++++++++++++----- 1 file changed, 66 insertions(+), 24 deletions(-) diff --git a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb index b620b015614718..2c158a6c82f8e8 100644 --- a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true require 'spec_helper' +require 'active_support/testing/stream' RSpec.describe Gitlab::Partitioning::ForeignKeysGenerator, :migration, :silence_stdout, feature_category: :continuous_integration do + include ActiveSupport::Testing::Stream include MigrationsHelpers before do @@ -33,41 +35,81 @@ let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } - let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', '_test_tmp_builds', '--database', 'main'] } let(:generator_config) { { destination_root: destination_root } } + let(:generator_args) { ['--source', source_table_name, '--target', target_table_name, '--database', 'main'] } + let(:source_table_name) { '_test_tmp_metadata' } + let(:target_table_name) { '_test_tmp_builds' } - it 'generates foreign key migrations' do - run_generator + context 'without foreign keys' do + let(:target_table_name) { 'projects' } - expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ - AddFkIndexToTestTmpMetadataOnPartitionIdAndBuildsId - AddFkToTestTmpMetadataOnPartitionIdAndBuildsId - ValidateFkOnTestTmpMetadataPartitionIdAndBuildsId - RemoveFkToTestTmpBuildsTestTmpMetadataOnBuildsId - ]) + it 'does not generate migrations' do + output = capture(:stderr) { run_generator } - schema_migrate_up! + expect(migrations).to be_empty + expect(output).to match(/No FK found between _test_tmp_metadata and projects/) + end + end + + context 'with one FK' do + it 'generates foreign key migrations' do + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndBuildsId + AddFkToTestTmpMetadataOnPartitionIdAndBuildsId + ValidateFkOnTestTmpMetadataPartitionIdAndBuildsId + RemoveFkToTestTmpBuildsTestTmpMetadataOnBuildsId + ]) + + schema_migrate_up! + + fks = Gitlab::Database::PostgresForeignKey + .by_referenced_table_identifier('public._test_tmp_builds') + .by_constrained_table_identifier('public._test_tmp_metadata') + + expect(fks.size).to eq(1) - fks = Gitlab::Database::PostgresForeignKey - .by_referenced_table_identifier('public._test_tmp_builds') - .by_constrained_table_identifier('public._test_tmp_metadata') + foreign_key = fks.first - expect(fks.size).to eq(1) + expect(foreign_key.name).to end_with('_p') + expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) + expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) + expect(foreign_key.on_delete_action).to eq('cascade') + # needs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108742 + # expect(foreign_key.on_update_action).to eq('cascade') - foreign_key = fks.first + index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| + index.columns == %w[partition_id builds_id] + end - expect(foreign_key.name).to end_with('_p') - expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) - expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) - expect(foreign_key.on_delete_action).to eq('cascade') - # needs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108742 - # expect(foreign_key.on_update_action).to eq('cascade') + expect(index).to be_present + end + end - index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| - index.columns == %w[partition_id builds_id] + context 'with many FKs' do + before do + ActiveRecord::Schema.define do + add_reference :_test_tmp_metadata, :job, + foreign_key: { to_table: :_test_tmp_builds, on_delete: :cascade } + end end - expect(index).to be_present + it 'generates migrations for the selected FK' do + expect(Thor::LineEditor) + .to receive(:readline) + .with('Please select one: [0, 1] (0) ', { default: '0', limited_to: %w[0 1] }) + .and_return('1') + + run_generator + + expect(migrations.sort_by(&:version).map(&:name)).to eq(%w[ + AddFkIndexToTestTmpMetadataOnPartitionIdAndJobId + AddFkToTestTmpMetadataOnPartitionIdAndJobId + ValidateFkOnTestTmpMetadataPartitionIdAndJobId + RemoveFkToTestTmpBuildsTestTmpMetadataOnJobId + ]) + end end def run_generator(args = generator_args, config = generator_config) -- GitLab From dfbebe100278ef4a2c30ef535edd5dd15180020e Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Wed, 25 Jan 2023 11:12:27 +0200 Subject: [PATCH 09/11] Use hardcoded table names everywhere --- .../gitlab/partitioning/foreign_keys_generator_spec.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb index 2c158a6c82f8e8..a28b5f242c9ac4 100644 --- a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -36,12 +36,10 @@ let_it_be(:destination_root) { File.expand_path("../tmp", __dir__) } let(:generator_config) { { destination_root: destination_root } } - let(:generator_args) { ['--source', source_table_name, '--target', target_table_name, '--database', 'main'] } - let(:source_table_name) { '_test_tmp_metadata' } - let(:target_table_name) { '_test_tmp_builds' } + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', '_test_tmp_builds', '--database', 'main'] } context 'without foreign keys' do - let(:target_table_name) { 'projects' } + let(:generator_args) { ['--source', '_test_tmp_metadata', '--target', 'projects', '--database', 'main'] } it 'does not generate migrations' do output = capture(:stderr) { run_generator } -- GitLab From 620e730138678e483074e3385b3a5f02ccb76fd8 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Wed, 25 Jan 2023 11:12:45 +0200 Subject: [PATCH 10/11] Remove usage file --- lib/generators/gitlab/partitioning/USAGE | 29 ------------------------ 1 file changed, 29 deletions(-) delete mode 100644 lib/generators/gitlab/partitioning/USAGE diff --git a/lib/generators/gitlab/partitioning/USAGE b/lib/generators/gitlab/partitioning/USAGE deleted file mode 100644 index 40b7cf0c53e85f..00000000000000 --- a/lib/generators/gitlab/partitioning/USAGE +++ /dev/null @@ -1,29 +0,0 @@ -This generator creates the migrations needed for updating the foreign keys when partitioning the tables - -Usage: - rails generate gitlab:partitioning:foreign_keys [options] - -Options: - [--skip-namespace], [--no-skip-namespace] # Skip namespace (affects only isolated engines) - [--skip-collision-check], [--no-skip-collision-check] # Skip collision check - --target=TARGET # Target table name - --source=SOURCE # Source table name - [--partitioning-column=PARTITIONING_COLUMN] # The column that is used for partitioning - # Default: partition_id - [--database=DATABASE] # Database name connection - # Default: ci - -Runtime options: - -f, [--force] # Overwrite files that already exist - -p, [--pretend], [--no-pretend] # Run but do not make any changes - -q, [--quiet], [--no-quiet] # Suppress status output - -s, [--skip], [--no-skip] # Skip files that already exist - -Example: - rails generate gitlab:partitioning:foreign_keys --target ci_builds --source ci_build_trace_chunks - - This will generate: - create db/post_migrate/20230118112350_add_fk_index_to_ci_build_trace_chunks_on_partition_id_and_build_id.rb - create db/post_migrate/20230118112351_add_fk_to_ci_build_trace_chunks_on_partition_id_and_build_id.rb - create db/post_migrate/20230118112352_validate_fk_on_ci_build_trace_chunks_partition_id_and_build_id.rb - create db/post_migrate/20230118112353_remove_fk_to_ci_builds_ci_build_trace_chunks_on_build_id.rb -- GitLab From 4b4a5def2a65a15966fd5eed0c5a6f30f819b328 Mon Sep 17 00:00:00 2001 From: Marius Bobin Date: Wed, 25 Jan 2023 11:18:45 +0200 Subject: [PATCH 11/11] Test FK on update option --- .../gitlab/partitioning/foreign_keys_generator_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb index a28b5f242c9ac4..7c7ca8207ffd3e 100644 --- a/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb +++ b/spec/lib/generators/gitlab/partitioning/foreign_keys_generator_spec.rb @@ -74,8 +74,7 @@ expect(foreign_key.constrained_columns).to eq(%w[partition_id builds_id]) expect(foreign_key.referenced_columns).to eq(%w[partition_id id]) expect(foreign_key.on_delete_action).to eq('cascade') - # needs https://gitlab.com/gitlab-org/gitlab/-/merge_requests/108742 - # expect(foreign_key.on_update_action).to eq('cascade') + expect(foreign_key.on_update_action).to eq('cascade') index = active_record_base.connection.indexes('_test_tmp_metadata').find do |index| index.columns == %w[partition_id builds_id] -- GitLab