diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-11 21:09:22 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2024-01-11 21:09:22 +0300 |
commit | 1bd9d2d9499d0d28e62254a28fcd3d913a8704af (patch) | |
tree | ea9969a5a4c3ac77858be20d69869674bed5ca43 /spec/lib | |
parent | d8877c12347443fa02e0ba53ad8d5cd318f6fa28 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'spec/lib')
-rw-r--r-- | spec/lib/gitlab/database/partitioning/list/convert_table_spec.rb | 468 |
1 files changed, 266 insertions, 202 deletions
diff --git a/spec/lib/gitlab/database/partitioning/list/convert_table_spec.rb b/spec/lib/gitlab/database/partitioning/list/convert_table_spec.rb index b30501cce21..e0b090f7ff9 100644 --- a/spec/lib/gitlab/database/partitioning/list/convert_table_spec.rb +++ b/spec/lib/gitlab/database/partitioning/list/convert_table_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Gitlab::Database::Partitioning::List::ConvertTable, feature_categ table_name: table_name, partitioning_column: partitioning_column, parent_table_name: parent_table_name, - zero_partition_value: partitioning_default + zero_partition_value: zero_partition_value ) end @@ -24,107 +24,121 @@ RSpec.describe Gitlab::Database::Partitioning::List::ConvertTable, feature_categ let(:async) { false } - it 'adds a check constraint' do - expect { prepare }.to change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier) - .count - }.from(0).to(1) - end - - context 'when it fails to add constraint' do - before do - allow(migration_context).to receive(:add_check_constraint) - end - - it 'raises UnableToPartition error' do - expect { prepare } - .to raise_error(described_class::UnableToPartition) - .and change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier) - .count - }.by(0) - end - end - - context 'when async' do - let(:async) { true } - - it 'adds a NOT VALID check constraint' do + shared_examples 'runs #prepare_for_partitioning' do + it 'adds a check constraint' do expect { prepare }.to change { Gitlab::Database::PostgresConstraint .check_constraints .by_table_identifier(table_identifier) .count }.from(0).to(1) + end - constraint = - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier) - .last + context 'when it fails to add constraint' do + before do + allow(migration_context).to receive(:add_check_constraint) + end - expect(constraint.definition).to end_with('NOT VALID') + it 'raises UnableToPartition error' do + expect { prepare } + .to raise_error(described_class::UnableToPartition) + .and change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier) + .count + }.by(0) + end end - it 'adds a PostgresAsyncConstraintValidation record' do - expect { prepare }.to change { - Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation.count - }.by(1) + context 'when async' do + let(:async) { true } - record = Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation - .where(table_name: table_name).last + it 'adds a NOT VALID check constraint' do + expect { prepare }.to change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier) + .count + }.from(0).to(1) - expect(record.name).to eq described_class::PARTITIONING_CONSTRAINT_NAME - expect(record).to be_check_constraint - end + constraint = + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier) + .last - context 'when constraint exists but is not valid' do - before do - converter.prepare_for_partitioning(async: true) + expect(constraint.definition).to end_with('NOT VALID') end - it 'validates the check constraint' do + it 'adds a PostgresAsyncConstraintValidation record' do expect { prepare }.to change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier).first.constraint_valid? - }.from(false).to(true) + Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation.count + }.by(1) + + record = Gitlab::Database::AsyncConstraints::PostgresAsyncConstraintValidation + .where(table_name: table_name).last + + expect(record.name).to eq described_class::PARTITIONING_CONSTRAINT_NAME + expect(record).to be_check_constraint end - context 'when it fails to validate constraint' do + context 'when constraint exists but is not valid' do before do - allow(migration_context).to receive(:validate_check_constraint) + converter.prepare_for_partitioning(async: true) end - it 'raises UnableToPartition error' do - expect { prepare } - .to raise_error(described_class::UnableToPartition, - starting_with('Error validating partitioning constraint')) - .and change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier) - .count - }.by(0) + it 'validates the check constraint' do + expect { prepare }.to change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier).first.constraint_valid? + }.from(false).to(true) end - end - end - context 'when constraint exists and is valid' do - before do - converter.prepare_for_partitioning(async: false) + context 'when it fails to validate constraint' do + before do + allow(migration_context).to receive(:validate_check_constraint) + end + + it 'raises UnableToPartition error' do + expect { prepare } + .to raise_error(described_class::UnableToPartition, + starting_with('Error validating partitioning constraint')) + .and change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier) + .count + }.by(0) + end + end end - it 'raises UnableToPartition error' do - expect(Gitlab::AppLogger).to receive(:info).with(starting_with('Nothing to do')) - prepare + context 'when constraint exists and is valid' do + before do + converter.prepare_for_partitioning(async: false) + end + + it 'raises UnableToPartition error' do + expect(Gitlab::AppLogger).to receive(:info).with(starting_with('Nothing to do')) + prepare + end end end end + + context 'when a single partitioning value is given' do + let(:zero_partition_value) { single_partitioning_value } + + include_examples 'runs #prepare_for_partitioning' + end + + context 'when multiple partitioning values are given' do + let(:zero_partition_value) { multiple_partitioning_values } + + include_examples 'runs #prepare_for_partitioning' + end end describe '#revert_preparation_for_partitioning' do @@ -132,15 +146,29 @@ RSpec.describe Gitlab::Database::Partitioning::List::ConvertTable, feature_categ converter.prepare_for_partitioning end + shared_examples 'runs #revert_preparation_for_partitioning' do + it 'removes a check constraint' do + expect { revert_prepare }.to change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier("#{connection.current_schema}.#{table_name}") + .count + }.from(1).to(0) + end + end + subject(:revert_prepare) { converter.revert_preparation_for_partitioning } - it 'removes a check constraint' do - expect { revert_prepare }.to change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier("#{connection.current_schema}.#{table_name}") - .count - }.from(1).to(0) + context 'when a single partitioning value is given' do + let(:zero_partition_value) { single_partitioning_value } + + include_examples 'runs #revert_preparation_for_partitioning' + end + + context 'when multiple partitioning values are given' do + let(:zero_partition_value) { multiple_partitioning_values } + + include_examples 'runs #revert_preparation_for_partitioning' end end @@ -153,128 +181,146 @@ RSpec.describe Gitlab::Database::Partitioning::List::ConvertTable, feature_categ converter.prepare_for_partitioning(async: async) end - context 'when the primary key is incorrect' do - before do - connection.execute(<<~SQL) - alter table #{referencing_table_name} drop constraint fk_referencing; -- this depends on the primary key - alter table #{other_referencing_table_name} drop constraint fk_referencing_other; -- this does too - alter table #{table_name} drop constraint #{table_name}_pkey; - alter table #{table_name} add constraint #{table_name}_pkey PRIMARY KEY (id); - SQL - end + shared_examples 'runs partition method' do + context 'when the primary key is incorrect' do + before do + connection.execute(<<~SQL) + alter table #{referencing_table_name} drop constraint fk_referencing; -- this depends on the primary key + alter table #{other_referencing_table_name} drop constraint fk_referencing_other; -- this does too + alter table #{table_name} drop constraint #{table_name}_pkey; + alter table #{table_name} add constraint #{table_name}_pkey PRIMARY KEY (id); + SQL + end - it 'throws a reasonable error message' do - expect { partition }.to raise_error(described_class::UnableToPartition, /#{partitioning_column}/) + it 'throws a reasonable error message' do + expect { partition }.to raise_error(described_class::UnableToPartition, /#{partitioning_column}/) + end end - end - context 'when there is not a supporting check constraint' do - before do - connection.execute(<<~SQL) - alter table #{table_name} drop constraint partitioning_constraint; - SQL - end + context 'when there is not a supporting check constraint' do + before do + connection.execute(<<~SQL) + alter table #{table_name} drop constraint partitioning_constraint; + SQL + end - it 'throws a reasonable error message' do - expect { partition }.to raise_error(described_class::UnableToPartition, /is not ready for partitioning./) + it 'throws a reasonable error message' do + expect { partition }.to raise_error(described_class::UnableToPartition, /is not ready for partitioning./) + end end - end - context 'when supporting check constraint is not valid' do - let(:async) { true } + context 'when supporting check constraint is not valid' do + let(:async) { true } - it 'throws a reasonable error message' do - expect { partition }.to raise_error(described_class::UnableToPartition, /is not ready for partitioning./) + it 'throws a reasonable error message' do + expect { partition }.to raise_error(described_class::UnableToPartition, /is not ready for partitioning./) + end end - end - - it 'migrates the table to a partitioned table' do - fks_before = migration_context.foreign_keys(table_name) - - partition - expect(Gitlab::Database::PostgresPartition.for_parent_table(parent_table_name).count).to eq(1) - expect(migration_context.foreign_keys(parent_table_name).map(&:options)).to match_array(fks_before.map(&:options)) + it 'migrates the table to a partitioned table' do + fks_before = migration_context.foreign_keys(table_name) - connection.execute(<<~SQL) - insert into #{table_name} (referenced_id, other_referenced_id) select #{referenced_table_name}.id, #{other_referenced_table_name}.id from #{referenced_table_name}, #{other_referenced_table_name}; - SQL + partition - # Create a second partition - connection.execute(<<~SQL) - create table #{table_name}2 partition of #{parent_table_name} FOR VALUES IN (2) - SQL + expect(Gitlab::Database::PostgresPartition.for_parent_table(parent_table_name).count).to eq(1) + expect(migration_context.foreign_keys(parent_table_name) + .map(&:options)).to match_array(fks_before.map(&:options)) - parent_model.create!(partitioning_column => 2, :referenced_id => 1, :other_referenced_id => 1) - expect(parent_model.pluck(:id)).to match_array([1, 2, 3]) - - expect { referencing_model.create!(partitioning_column => 1, :ref_id => 1) }.not_to raise_error - end + connection.execute(<<~SQL) + insert into #{table_name} (referenced_id, other_referenced_id) select #{referenced_table_name}.id, #{other_referenced_table_name}.id from #{referenced_table_name}, #{other_referenced_table_name}; + SQL - context 'when the existing table is owned by a different user' do - before do + # Create a second partition connection.execute(<<~SQL) - CREATE USER other_user SUPERUSER; - ALTER TABLE #{table_name} OWNER TO other_user; + create table #{table_name}2 partition of #{parent_table_name} FOR VALUES IN (2) SQL - end - let(:current_user) { model.connection.select_value('select current_user') } + parent_model.create!(partitioning_column => 2, :referenced_id => 1, :other_referenced_id => 1) + expect(parent_model.pluck(:id)).to match_array([1, 2, 3]) - it 'partitions without error' do - expect { partition }.not_to raise_error + expect { referencing_model.create!(partitioning_column => 1, :ref_id => 1) }.not_to raise_error end - end - context 'when an error occurs during the conversion' do - before do - # Set up the fault that we'd like to inject - fault.call - end + context 'when the existing table is owned by a different user' do + before do + connection.execute(<<~SQL) + CREATE USER other_user SUPERUSER; + ALTER TABLE #{table_name} OWNER TO other_user; + SQL + end - let(:old_fks) do - Gitlab::Database::PostgresForeignKey.by_referenced_table_identifier(table_identifier).not_inherited - end + let(:current_user) { model.connection.select_value('select current_user') } - let(:new_fks) do - Gitlab::Database::PostgresForeignKey.by_referenced_table_identifier(parent_table_identifier).not_inherited + it 'partitions without error' do + expect { partition }.not_to raise_error + end end - context 'when partitioning fails the first time' do - where(:case_name, :fault) do - [ - ["creating parent table", lazy { fail_sql_matching(/CREATE/i) }], - ["adding the first foreign key", lazy { fail_adding_fk(parent_table_name, referenced_table_name) }], - ["adding the second foreign key", lazy { fail_adding_fk(parent_table_name, other_referenced_table_name) }], - ["attaching table", lazy { fail_sql_matching(/ATTACH/i) }] - ] + context 'when an error occurs during the conversion' do + before do + # Set up the fault that we'd like to inject + fault.call end - with_them do - it 'recovers from a fault', :aggregate_failures do - expect { converter.partition }.to raise_error(/fault/) + let(:old_fks) do + Gitlab::Database::PostgresForeignKey.by_referenced_table_identifier(table_identifier).not_inherited + end - expect { converter.partition }.not_to raise_error - expect(Gitlab::Database::PostgresPartition.for_parent_table(parent_table_name).count).to eq(1) + let(:new_fks) do + Gitlab::Database::PostgresForeignKey.by_referenced_table_identifier(parent_table_identifier).not_inherited + end + + context 'when partitioning fails the first time' do + where(:case_name, :fault) do + [ + ["creating parent table", lazy { fail_sql_matching(/CREATE/i) }], + ["adding the first foreign key", lazy { fail_adding_fk(parent_table_name, referenced_table_name) }], + ["adding the second foreign key", lazy do + fail_adding_fk(parent_table_name, other_referenced_table_name) + end], + ["attaching table", lazy { fail_sql_matching(/ATTACH/i) }] + ] + end + + with_them do + it 'recovers from a fault', :aggregate_failures do + expect { converter.partition }.to raise_error(/fault/) + + expect { converter.partition }.not_to raise_error + expect(Gitlab::Database::PostgresPartition.for_parent_table(parent_table_name).count).to eq(1) + end end end end - end - context 'when table has LFK triggers' do - before do - migration_context.track_record_deletions(table_name) - end + context 'when table has LFK triggers' do + before do + migration_context.track_record_deletions(table_name) + end - it 'moves the trigger on the parent table', :aggregate_failures do - expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy + it 'moves the trigger on the parent table', :aggregate_failures do + expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy - expect { partition }.not_to raise_error + expect { partition }.not_to raise_error - expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy - expect(migration_context.has_loose_foreign_key?(parent_table_name)).to be_truthy + expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy + expect(migration_context.has_loose_foreign_key?(parent_table_name)).to be_truthy + end end end + + context 'when a single partitioning value is given' do + let(:zero_partition_value) { single_partitioning_value } + + include_examples 'runs partition method' + end + + context 'when multiple partitioning values are given' do + # Because of the common spec on line 220 + let(:zero_partition_value) { [1, 3, 4] } + + include_examples 'runs partition method' + end end describe '#revert_partitioning' do @@ -285,49 +331,67 @@ RSpec.describe Gitlab::Database::Partitioning::List::ConvertTable, feature_categ subject(:revert_conversion) { converter.revert_partitioning } - it 'detaches the partition' do - expect { revert_conversion }.to change { - Gitlab::Database::PostgresPartition - .for_parent_table(parent_table_name).count - }.from(1).to(0) - end - - it 'does not drop the child partition' do - expect { revert_conversion }.not_to change { table_oid(table_name) } - end + shared_examples 'runs #revert_partitioning' do + it 'detaches the partition' do + expect { revert_conversion }.to change { + Gitlab::Database::PostgresPartition + .for_parent_table(parent_table_name).count + }.from(1).to(0) + end - it 'removes the parent table' do - expect { revert_conversion }.to change { table_oid(parent_table_name).present? }.from(true).to(false) - end + it 'does not drop the child partition' do + expect { revert_conversion }.not_to change { table_oid(table_name) } + end - it 're-adds the check constraint' do - expect { revert_conversion }.to change { - Gitlab::Database::PostgresConstraint - .check_constraints - .by_table_identifier(table_identifier) - .count - }.by(1) - end + it 'removes the parent table' do + expect { revert_conversion }.to change { table_oid(parent_table_name).present? }.from(true).to(false) + end - it 'moves sequences back to the original table' do - expect { revert_conversion }.to change { converter.send(:sequences_owned_by, table_name).count }.from(0) - .and change { converter.send(:sequences_owned_by, parent_table_name).count }.to(0) - end + it 're-adds the check constraint' do + expect { revert_conversion }.to change { + Gitlab::Database::PostgresConstraint + .check_constraints + .by_table_identifier(table_identifier) + .count + }.by(1) + end - context 'when table has LFK triggers' do - before do - migration_context.track_record_deletions(parent_table_name) - migration_context.track_record_deletions(table_name) + it 'moves sequences back to the original table' do + expect { revert_conversion }.to change { converter.send(:sequences_owned_by, table_name).count } + .from(0) + .and change { + converter.send( + :sequences_owned_by, parent_table_name).count + }.to(0) end - it 'restores the trigger on the partition', :aggregate_failures do - expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy - expect(migration_context.has_loose_foreign_key?(parent_table_name)).to be_truthy + context 'when table has LFK triggers' do + before do + migration_context.track_record_deletions(parent_table_name) + migration_context.track_record_deletions(table_name) + end + + it 'restores the trigger on the partition', :aggregate_failures do + expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy + expect(migration_context.has_loose_foreign_key?(parent_table_name)).to be_truthy - expect { revert_conversion }.not_to raise_error + expect { revert_conversion }.not_to raise_error - expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy + expect(migration_context.has_loose_foreign_key?(table_name)).to be_truthy + end end end + + context 'when a single partitioning value is given' do + let(:zero_partition_value) { single_partitioning_value } + + include_examples 'runs #revert_partitioning' + end + + context 'when multiple partitioning values are given' do + let(:zero_partition_value) { multiple_partitioning_values } + + include_examples 'runs #revert_partitioning' + end end end |