From cd0eac748d12f3ad903d6c573bbf0386492e90ae Mon Sep 17 00:00:00 2001 From: Niklas van Schrick Date: Wed, 7 Oct 2026 20:58:18 +0200 Subject: [PATCH] Fix foreign key violations during partition detachments --- README.md | 3 + .../partitioning/partition_manager.rb | 69 +++++++++++ .../partitioning/partitioned_table.rb | 10 ++ .../partitioning/partition_manager_spec.rb | 108 +++++++++++++++++- .../partitioning/partitioned_table_spec.rb | 25 ++++ 5 files changed, 214 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index cff76ad..d8f3368 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,9 @@ When tables have foreign key relationships, registration order and retention con - A child table's `retain_for` must be less than or equal to the parent table's `retain_for`. If a child retains partitions longer than its parent, dropping the parent partition will fail because the child's foreign key still references it. +- Foreign keys between the partitioned tables must be declared with `drop_foreign_keys_on_detach` + on the model. Otherwise, the foreign key of a detached partition will block detaching partitions + of the referenced table. #### Schema Cleaner Integration diff --git a/lib/code0/zero_track/database/partitioning/partition_manager.rb b/lib/code0/zero_track/database/partitioning/partition_manager.rb index 0789dc9..ac2c9f1 100644 --- a/lib/code0/zero_track/database/partitioning/partition_manager.rb +++ b/lib/code0/zero_track/database/partitioning/partition_manager.rb @@ -64,6 +64,8 @@ def detach_partitions! model.partitioning_strategy.partitions_to_detach.each do |partition| detach_partition(partition, connection) end + + drop_declared_foreign_keys_from_detached_partitions(connection) end end @@ -109,6 +111,73 @@ def detach_partition(partition, connection) ) end + def drop_declared_foreign_keys_from_detached_partitions(connection) + constraint_names = model.foreign_keys_to_drop_on_detach || [] + return if constraint_names.empty? + + warn_about_unknown_foreign_keys(constraint_names, connection) + + detached_foreign_keys(constraint_names, connection).each do |qualified_table, constraint_name| + connection.execute( + "ALTER TABLE #{qualified_table} " \ + "DROP CONSTRAINT #{connection.quote_column_name(constraint_name)}" + ) + + logger.info( + message: 'Dropped foreign key from detached partition', + table_name: model.table_name, + partition_name: qualified_table, + constraint_name: constraint_name + ) + end + end + + # We check the parent table because we drop the FK from the partition + # when detaching. A missing FK on the detached partition is an expected state + def warn_about_unknown_foreign_keys(constraint_names, connection) + existing = parent_foreign_key_names(connection) + unknown = constraint_names - existing + return if unknown.empty? + + logger.warn( + message: 'Configured foreign keys to drop on detach do not exist on the partitioned table', + table_name: model.table_name, + constraint_names: unknown + ) + end + + def parent_foreign_key_names(connection) + connection.select_values(<<~SQL.squish) + SELECT con.conname + FROM pg_catalog.pg_constraint con + WHERE con.contype = 'f' + AND con.conrelid = #{connection.quote(model.table_name)}::regclass + SQL + end + + def detached_foreign_keys(constraint_names, connection) + schema = Rails.application.config.zero_track.db_partitioning.dynamic_partition_schema + quoted_names = constraint_names.map { |name| connection.quote(name) }.join(', ') + + rows = connection.select_rows(<<~SQL.squish) + SELECT (quote_ident(n.nspname) || '.' || quote_ident(c.relname)) AS qualified_table, + con.conname AS constraint_name + FROM pg_catalog.pg_constraint con + JOIN pg_catalog.pg_class c ON c.oid = con.conrelid + JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace + WHERE con.contype = 'f' + AND con.conname IN (#{quoted_names}) + AND n.nspname = #{connection.quote(schema)} + AND c.relkind = 'r' + AND NOT c.relispartition + AND NOT EXISTS ( + SELECT 1 FROM pg_catalog.pg_inherits i WHERE i.inhrelid = c.oid + ) + SQL + + rows.map { |qualified_table, constraint_name| [qualified_table, constraint_name] } + end + def drop_partition(detached_partition, connection) schema_name = connection.quote_table_name(detached_partition.schema) partition_name = connection.quote_table_name(detached_partition.name) diff --git a/lib/code0/zero_track/database/partitioning/partitioned_table.rb b/lib/code0/zero_track/database/partitioning/partitioned_table.rb index 0996330..3d86bb4 100644 --- a/lib/code0/zero_track/database/partitioning/partitioned_table.rb +++ b/lib/code0/zero_track/database/partitioning/partitioned_table.rb @@ -25,6 +25,16 @@ def partition_by(column, strategy:, **kwargs) @partitioning_strategy = strategy_class.new(self, column, **kwargs) end + + def drop_foreign_keys_on_detach(*constraint_names) + @foreign_keys_to_drop_on_detach ||= [] + @foreign_keys_to_drop_on_detach.concat(constraint_names.flatten.map(&:to_s)) + @foreign_keys_to_drop_on_detach + end + + def foreign_keys_to_drop_on_detach + @foreign_keys_to_drop_on_detach || [] + end end end end diff --git a/spec/code0/zero_track/database/partitioning/partition_manager_spec.rb b/spec/code0/zero_track/database/partitioning/partition_manager_spec.rb index ab0a811..4b7994b 100644 --- a/spec/code0/zero_track/database/partitioning/partition_manager_spec.rb +++ b/spec/code0/zero_track/database/partitioning/partition_manager_spec.rb @@ -9,19 +9,26 @@ let(:model) do model = double('Model', table_name: 'events') # rubocop:disable RSpec/VerifiedDoubles allow(model).to receive(:try).with(:partitioning_strategy).and_return(partitioning_strategy) - allow(model).to receive(:partitioning_strategy).and_return(partitioning_strategy) + allow(model).to receive_messages( + partitioning_strategy: partitioning_strategy, + foreign_keys_to_drop_on_detach: foreign_keys_to_drop_on_detach + ) allow(model).to receive(:with_connection).and_yield(connection) model end let(:partitioning_strategy) { double('Strategy') } + let(:foreign_keys_to_drop_on_detach) { [] } + let(:connection) do connection = double('Connection') allow(connection).to receive(:execute) allow(connection).to receive(:transaction).and_yield allow(connection).to receive(:quote_table_name) { |name| "\"#{name}\"" } + allow(connection).to receive(:quote_column_name) { |name| "\"#{name}\"" } allow(connection).to receive(:quote) { |value| "'#{value}'" } + allow(connection).to receive_messages(select_rows: [], select_values: []) connection end @@ -172,6 +179,105 @@ expect(connection).not_to have_received(:execute).with(a_string_matching(/DETACH/)) end + + context 'when the model declares foreign keys to drop on detach' do + let(:foreign_keys_to_drop_on_detach) { %w[fk_events_parent fk_events_other] } + + let(:partition) do + Code0::ZeroTrack::Database::Partitioning::TimePartition.new( + model, '2023-01-01', '2023-02-01', partition_name: 'events_202301' + ) + end + + before do + allow(partitioning_strategy).to receive(:partitions_to_detach).and_return([partition]) + end + + it 'drops the declared foreign keys found on detached partitions' do + allow(connection).to receive(:select_rows).and_return( + [['"partitions_dynamic"."events_202301"', 'fk_events_parent']] + ) + + described_class.new(model).detach_partitions! + + expect(connection).to have_received(:execute).with( + a_string_matching( + /ALTER TABLE "partitions_dynamic"."events_202301" DROP CONSTRAINT "fk_events_parent"/ + ) + ) + end + + it 'reconciles every detached partition, not only the one detached in this run' do + # Two already-detached partitions still carry the inherited foreign key. + allow(connection).to receive(:select_rows).and_return( + [ + ['"partitions_dynamic"."events_202301"', 'fk_events_parent'], + ['"partitions_dynamic"."events_202212"', 'fk_events_parent'] + ] + ) + + described_class.new(model).detach_partitions! + + expect(connection).to have_received(:execute).with( + a_string_matching(/"partitions_dynamic"."events_202301" DROP CONSTRAINT "fk_events_parent"/) + ) + expect(connection).to have_received(:execute).with( + a_string_matching(/"partitions_dynamic"."events_202212" DROP CONSTRAINT "fk_events_parent"/) + ) + end + + it 'scopes the catalog lookup to the declared constraint names and dynamic schema' do + described_class.new(model).detach_partitions! + + expect(connection).to have_received(:select_rows).with( + a_string_matching(/pg_catalog\.pg_constraint.*contype = 'f'/m) + ) + expect(connection).to have_received(:select_rows).with( + a_string_matching(/'fk_events_parent', 'fk_events_other'/) + ) + expect(connection).to have_received(:select_rows).with( + a_string_matching(/'partitions_dynamic'/) + ) + end + + it 'does not attempt to drop anything when the catalog reports no matching foreign keys' do + allow(connection).to receive(:select_rows).and_return([]) + + described_class.new(model).detach_partitions! + + expect(connection).not_to have_received(:execute).with(a_string_matching(/DROP CONSTRAINT/)) + end + + it 'skips the catalog lookup entirely when no foreign keys are declared' do + allow(model).to receive(:foreign_keys_to_drop_on_detach).and_return([]) + + described_class.new(model).detach_partitions! + + expect(connection).not_to have_received(:select_rows) + end + + it 'warns about declared foreign keys missing from the partitioned parent table' do + allow(connection).to receive(:select_values).and_return(%w[fk_events_parent]) + + described_class.new(model).detach_partitions! + + expect(rails_logger).to have_received(:warn).with( + hash_including( + message: /do not exist on the partitioned table/, + table_name: 'events', + constraint_names: %w[fk_events_other] + ) + ) + end + + it 'does not warn when all declared foreign keys exist on the partitioned parent table' do + allow(connection).to receive(:select_values).and_return(%w[fk_events_parent fk_events_other]) + + described_class.new(model).detach_partitions! + + expect(rails_logger).not_to have_received(:warn) + end + end end describe '#drop_partitions' do diff --git a/spec/code0/zero_track/database/partitioning/partitioned_table_spec.rb b/spec/code0/zero_track/database/partitioning/partitioned_table_spec.rb index e02b9f6..9263cef 100644 --- a/spec/code0/zero_track/database/partitioning/partitioned_table_spec.rb +++ b/spec/code0/zero_track/database/partitioning/partitioned_table_spec.rb @@ -61,5 +61,30 @@ end.to raise_error(ArgumentError, /already partitioned/) end end + + describe '.drop_foreign_keys_on_detach' do + it 'defaults to an empty list' do + expect(test_class.foreign_keys_to_drop_on_detach).to eq([]) + end + + it 'records declared constraint names as strings' do + test_class.drop_foreign_keys_on_detach(:fk_a, 'fk_b') + + expect(test_class.foreign_keys_to_drop_on_detach).to eq(%w[fk_a fk_b]) + end + + it 'accumulates across multiple declarations' do + test_class.drop_foreign_keys_on_detach(:fk_a) + test_class.drop_foreign_keys_on_detach(:fk_b, :fk_c) + + expect(test_class.foreign_keys_to_drop_on_detach).to eq(%w[fk_a fk_b fk_c]) + end + + it 'flattens an array argument' do + test_class.drop_foreign_keys_on_detach(%i[fk_a fk_b]) + + expect(test_class.foreign_keys_to_drop_on_detach).to eq(%w[fk_a fk_b]) + end + end end # rubocop:enable RSpec/VerifiedDoubles