From 6e155d013e30024fe0417c987a86d9481c359a3f Mon Sep 17 00:00:00 2001
From: Max Orefice <morefice@gitlab.com>
Date: Tue, 13 Feb 2024 11:42:23 +0000
Subject: [PATCH] Check FK existence in validate_partitioned_foreign_key
---
.../foreign_key_helpers.rb | 9 ++-
.../foreign_key_helpers_spec.rb | 72 ++++++++++++++++---
2 files changed, 70 insertions(+), 11 deletions(-)
diff --git a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb
index 7d9c12d776e5..0132919a864d 100644
--- a/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb
+++ b/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb
@@ -94,8 +94,15 @@ def add_concurrent_partitioned_foreign_key(source, target, column:, **options)
def validate_partitioned_foreign_key(source, column, name: nil)
assert_not_in_transaction_block(scope: ERROR_SCOPE)
+ fk_name = name || concurrent_partitioned_foreign_key_name(source, column)
+
Gitlab::Database::PostgresPartitionedTable.each_partition(source) do |partition|
- validate_foreign_key(partition.identifier, column, name: name)
+ unless foreign_key_exists?(partition.identifier, name: fk_name)
+ Gitlab::AppLogger.warn("Missing #{fk_name} on #{partition.identifier}")
+ next
+ end
+
+ validate_foreign_key(partition.identifier, column, name: fk_name)
end
end
diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb
index 5f1e8842f187..69283015058f 100644
--- a/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb
+++ b/spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb
@@ -227,16 +227,68 @@ def expect_add_concurrent_fk(source_table_name, target_table_name, options)
)
end
- it 'validates FK for each partition' do
- allow(migration).to receive(:statement_timeout_disabled?).and_return(false)
- expect(migration).to receive(:execute).with(/SET statement_timeout TO 0/).twice
- expect(migration).to receive(:execute).with(/RESET statement_timeout/).twice
- expect(migration).to receive(:execute)
- .with(/ALTER TABLE #{partition1_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
- expect(migration).to receive(:execute)
- .with(/ALTER TABLE #{partition2_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
-
- migration.validate_partitioned_foreign_key(source_table_name, column_name, name: foreign_key_name)
+ context 'when name is provided' do
+ it 'validates FK for each partition', :aggregate_failures do
+ expect(migration).to receive(:foreign_key_exists?)
+ .with(partition1_name, name: foreign_key_name)
+ .and_return(true).twice
+ expect(migration).to receive(:foreign_key_exists?)
+ .with(partition2_name, name: foreign_key_name)
+ .and_return(true).twice
+
+ allow(migration).to receive(:statement_timeout_disabled?).and_return(false)
+ expect(migration).to receive(:execute).with(/SET statement_timeout TO 0/).twice
+ expect(migration).to receive(:execute).with(/RESET statement_timeout/).twice
+ expect(migration).to receive(:execute)
+ .with(/ALTER TABLE #{partition1_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
+ expect(migration).to receive(:execute)
+ .with(/ALTER TABLE #{partition2_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
+
+ migration.validate_partitioned_foreign_key(source_table_name, column_name, name: foreign_key_name)
+ end
+ end
+
+ context 'when name is not provided' do
+ it 'validates FK for each partition', :aggregate_failures do
+ expect(migration).to receive(:foreign_key_exists?)
+ .with(partition1_name, name: anything)
+ .and_return(true).twice
+ expect(migration).to receive(:foreign_key_exists?)
+ .with(partition2_name, name: anything)
+ .and_return(true).twice
+
+ allow(migration).to receive(:statement_timeout_disabled?).and_return(false)
+ expect(migration).to receive(:execute).with(/SET statement_timeout TO 0/).twice
+ expect(migration).to receive(:execute).with(/RESET statement_timeout/).twice
+ expect(migration).to receive(:execute).with(/ALTER TABLE #{partition1_name} VALIDATE CONSTRAINT/).ordered
+ expect(migration).to receive(:execute).with(/ALTER TABLE #{partition2_name} VALIDATE CONSTRAINT/).ordered
+
+ migration.validate_partitioned_foreign_key(source_table_name, column_name)
+ end
+ end
+
+ context 'when FK does not exist for a given partition' do
+ before do
+ allow(migration).to receive(:foreign_key_exists?)
+ .with(partition1_name, name: foreign_key_name)
+ .and_return(true)
+ allow(migration).to receive(:foreign_key_exists?)
+ .with(partition2_name, name: foreign_key_name)
+ .and_return(false)
+ end
+
+ it 'does not validate missing FK', :aggregate_failures do
+ allow(migration).to receive(:statement_timeout_disabled?).and_return(false)
+ expect(migration).to receive(:execute).with(/SET statement_timeout TO 0/)
+ expect(migration).to receive(:execute).with(/RESET statement_timeout/)
+ expect(migration).to receive(:execute)
+ .with(/ALTER TABLE #{partition1_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
+ expect(migration).not_to receive(:execute)
+ .with(/ALTER TABLE #{partition2_name} VALIDATE CONSTRAINT #{foreign_key_name}/).ordered
+ expect(Gitlab::AppLogger).to receive(:warn).with("Missing #{foreign_key_name} on #{partition2_name}")
+
+ migration.validate_partitioned_foreign_key(source_table_name, column_name, name: foreign_key_name)
+ end
end
end
end
--
GitLab