Skip to content

G
gitlab-ce
Commit ee816a4c authored by Dylan Griffith's avatar Dylan Griffith
Browse files
Options

Merge branch '334289-remove-pg11-partitioned-foreign-keys' into 'master' 

Remove code for PG11 custom foreign keys

See merge request gitlab-org/gitlab!64658
parents c5a28073 5af8afcd
Hide whitespace changes
Inline Side-by-side
Showing with 120 additions and 368 deletions
lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers.rb
View file @ ee816a4c
......@@ -79,135 +79,6 @@ module Gitlab
"#{prefix}#{hashed_identifier}"
end
# Creates a "foreign key" that references a partitioned table. Because foreign keys referencing partitioned
# tables are not supported in PG11, this does not create a true database foreign key, but instead implements the
# same functionality at the database level by using triggers.
#
# Example:
#
# add_partitioned_foreign_key :issues, :projects
#
# Available options:
#
# :column - name of the referencing column (otherwise inferred from the referenced table name)
# :primary_key - name of the primary key in the referenced table (defaults to id)
# :on_delete - supports either :cascade for ON DELETE CASCADE or :nullify for ON DELETE SET NULL
#
def add_partitioned_foreign_key(from_table, to_table, column: nil, primary_key: :id, on_delete: :cascade)
cascade_delete = extract_cascade_option(on_delete)
update_foreign_keys(from_table, to_table, column, primary_key, cascade_delete) do |current_keys, existing_key, specified_key|
if existing_key.nil?
unless specified_key.save
raise "failed to create foreign key: #{specified_key.errors.full_messages.to_sentence}"
end
current_keys << specified_key
else
Gitlab::AppLogger.warn "foreign key not added because it already exists: #{specified_key}"
current_keys
end
end
end
# Drops a "foreign key" that references a partitioned table. This method ONLY applies to foreign keys previously
# created through the `add_partitioned_foreign_key` method. Standard database foreign keys should be managed
# through the familiar Rails helpers.
#
# Example:
#
# remove_partitioned_foreign_key :issues, :projects
#
# Available options:
#
# :column - name of the referencing column (otherwise inferred from the referenced table name)
# :primary_key - name of the primary key in the referenced table (defaults to id)
#
def remove_partitioned_foreign_key(from_table, to_table, column: nil, primary_key: :id)
update_foreign_keys(from_table, to_table, column, primary_key) do |current_keys, existing_key, specified_key|
if existing_key
existing_key.delete
current_keys.delete(existing_key)
else
Gitlab::AppLogger.warn "foreign key not removed because it doesn't exist: #{specified_key}"
end
current_keys
end
end
private
def fk_function_name(table)
object_name(table, 'fk_cascade_function')
end
def fk_trigger_name(table)
object_name(table, 'fk_cascade_trigger')
end
def fk_from_spec(from_table, to_table, from_column, to_column, cascade_delete)
PartitionedForeignKey.new(from_table: from_table.to_s, to_table: to_table.to_s, from_column: from_column.to_s,
to_column: to_column.to_s, cascade_delete: cascade_delete)
end
def update_foreign_keys(from_table, to_table, from_column, to_column, cascade_delete = nil)
assert_not_in_transaction_block(scope: 'partitioned foreign key')
from_column ||= "#{to_table.to_s.singularize}_id"
specified_key = fk_from_spec(from_table, to_table, from_column, to_column, cascade_delete)
current_keys = PartitionedForeignKey.by_referenced_table(to_table).to_a
existing_key = find_existing_key(current_keys, specified_key)
final_keys = yield current_keys, existing_key, specified_key
fn_name = fk_function_name(to_table)
trigger_name = fk_trigger_name(to_table)
with_lock_retries do
drop_trigger(to_table, trigger_name, if_exists: true)
if final_keys.empty?
drop_function(fn_name, if_exists: true)
else
create_or_replace_fk_function(fn_name, final_keys)
create_trigger(to_table, trigger_name, fn_name, fires: 'AFTER DELETE')
end
end
end
def extract_cascade_option(on_delete)
case on_delete
when :cascade then true
when :nullify then false
else raise ArgumentError, "invalid option #{on_delete} for :on_delete"
end
end
def find_existing_key(keys, key)
keys.find { |k| k.from_table == key.from_table && k.from_column == key.from_column }
end
def create_or_replace_fk_function(fn_name, fk_specs)
create_trigger_function(fn_name, replace: true) do
cascade_statements = build_cascade_statements(fk_specs)
cascade_statements << 'RETURN OLD;'
cascade_statements.join("\n")
end
end
def build_cascade_statements(foreign_keys)
foreign_keys.map do |fks|
if fks.cascade_delete?
"DELETE FROM #{fks.from_table} WHERE #{fks.from_column} = OLD.#{fks.to_column};"
else
"UPDATE #{fks.from_table} SET #{fks.from_column} = NULL WHERE #{fks.from_column} = OLD.#{fks.to_column};"
end
end
end
end
end
end
......
lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key.rb deleted 100644 → 0
View file @ c5a28073
# frozen_string_literal: true
module Gitlab
module Database
module PartitioningMigrationHelpers
class PartitionedForeignKey < ApplicationRecord
validates_with PartitionedForeignKeyValidator
scope :by_referenced_table, ->(table) { where(to_table: table) }
end
end
end
end
lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_validator.rb deleted 100644 → 0
View file @ c5a28073
# frozen_string_literal: true
module Gitlab
module Database
module PartitioningMigrationHelpers
class PartitionedForeignKeyValidator < ActiveModel::Validator
def validate(record)
validate_key_part(record, :from_table, :from_column)
validate_key_part(record, :to_table, :to_column)
end
private
def validate_key_part(record, table_field, column_field)
if !connection.table_exists?(record[table_field])
record.errors.add(table_field, 'must be a valid table')
elsif !connection.column_exists?(record[table_field], record[column_field])
record.errors.add(column_field, 'must be a valid column')
end
end
def connection
ActiveRecord::Base.connection
end
end
end
end
end
spec/lib/gitlab/database/partitioning_migration_helpers/foreign_key_helpers_spec.rb
View file @ ee816a4c
......@@ -3,192 +3,142 @@
require 'spec_helper'
RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::ForeignKeyHelpers do
include Database::TriggerHelpers
include Database::TableSchemaHelpers
let(:model) do
ActiveRecord::Migration.new.extend(described_class)
let(:migration) do
ActiveRecord::Migration.new.extend(Gitlab::Database::PartitioningMigrationHelpers)
end
let_it_be(:connection) { ActiveRecord::Base.connection }
let(:referenced_table) { :issues }
let(:function_name) { '_test_partitioned_foreign_keys_function' }
let(:trigger_name) { '_test_partitioned_foreign_keys_trigger' }
let(:source_table_name) { '_test_partitioned_table' }
let(:target_table_name) { '_test_referenced_table' }
let(:column_name) { "#{target_table_name}_id" }
let(:foreign_key_name) { '_test_partitioned_fk' }
let(:partition_schema) { 'gitlab_partitions_dynamic' }
let(:partition1_name) { "#{partition_schema}.#{source_table_name}_202001" }
let(:partition2_name) { "#{partition_schema}.#{source_table_name}_202002" }
let(:options) do
{
column: column_name,
name: foreign_key_name,
on_delete: :cascade,
validate: true
}
end
before do
allow(model).to receive(:puts)
allow(model).to receive(:fk_function_name).and_return(function_name)
allow(model).to receive(:fk_trigger_name).and_return(trigger_name)
allow(migration).to receive(:puts)
connection.execute(<<~SQL)
CREATE TABLE #{target_table_name} (
id serial NOT NULL,
PRIMARY KEY (id)
);
CREATE TABLE #{source_table_name} (
id serial NOT NULL,
#{column_name} int NOT NULL,
created_at timestamptz NOT NULL,
PRIMARY KEY (id, created_at)
) PARTITION BY RANGE (created_at);
CREATE TABLE #{partition1_name} PARTITION OF #{source_table_name}
FOR VALUES FROM ('2020-01-01') TO ('2020-02-01');
CREATE TABLE #{partition2_name} PARTITION OF #{source_table_name}
FOR VALUES FROM ('2020-02-01') TO ('2020-03-01');
SQL
end
describe 'adding a foreign key' do
describe '#add_concurrent_partitioned_foreign_key' do
before do
allow(model).to receive(:transaction_open?).and_return(false)
end
context 'when the table has no foreign keys' do
it 'creates a trigger function to handle the single cascade' do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
context 'when the table already has foreign keys' do
context 'when the foreign key is from a different table' do
before do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
end
it 'creates a trigger function to handle the multiple cascades' do
model.add_partitioned_foreign_key :epic_issues, referenced_table
expect_function_to_contain(function_name,
'delete from issue_assignees where issue_id = old.id',
'delete from epic_issues where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
context 'when the foreign key is from the same table' do
before do
model.add_partitioned_foreign_key :issues, referenced_table, column: :moved_to_id
end
context 'when the foreign key is from a different column' do
it 'creates a trigger function to handle the multiple cascades' do
model.add_partitioned_foreign_key :issues, referenced_table, column: :duplicated_to_id
expect_function_to_contain(function_name,
'delete from issues where moved_to_id = old.id',
'delete from issues where duplicated_to_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
context 'when the foreign key is from the same column' do
it 'ignores the duplicate and properly recreates the trigger function' do
model.add_partitioned_foreign_key :issues, referenced_table, column: :moved_to_id
expect_function_to_contain(function_name, 'delete from issues where moved_to_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
end
end
allow(migration).to receive(:foreign_key_exists?)
.with(source_table_name, target_table_name, anything)
.and_return(false)
context 'when the foreign key is set to nullify' do
it 'creates a trigger function that nullifies the foreign key' do
model.add_partitioned_foreign_key :issue_assignees, referenced_table, on_delete: :nullify
expect_function_to_contain(function_name, 'update issue_assignees set issue_id = null where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
allow(migration).to receive(:with_lock_retries).and_yield
end
context 'when the referencing column is a custom value' do
it 'creates a trigger function with the correct column name' do
model.add_partitioned_foreign_key :issues, referenced_table, column: :duplicated_to_id
context 'when the foreign key does not exist on the parent table' do
it 'creates the foreign key on each partition, and the parent table' do
expect(migration).to receive(:foreign_key_exists?)
.with(source_table_name, target_table_name, **options)
.and_return(false)
expect_function_to_contain(function_name, 'delete from issues where duplicated_to_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
expect(migration).to receive(:concurrent_partitioned_foreign_key_name).and_return(foreign_key_name)
context 'when the referenced column is a custom value' do
let(:referenced_table) { :user_details }
expect_add_concurrent_fk_and_call_original(partition1_name, target_table_name, **options)
expect_add_concurrent_fk_and_call_original(partition2_name, target_table_name, **options)
it 'creates a trigger function with the correct column name' do
model.add_partitioned_foreign_key :user_preferences, referenced_table, column: :user_id, primary_key: :user_id
expect(migration).to receive(:with_lock_retries).ordered.and_yield
expect(migration).to receive(:add_foreign_key)
.with(source_table_name, target_table_name, **options)
.ordered
.and_call_original
expect_function_to_contain(function_name, 'delete from user_preferences where user_id = old.user_id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
end
end
migration.add_concurrent_partitioned_foreign_key(source_table_name, target_table_name, column: column_name)
context 'when the given key definition is invalid' do
it 'raises an error with the appropriate message' do
expect do
model.add_partitioned_foreign_key :issue_assignees, referenced_table, column: :not_a_real_issue_id
end.to raise_error(/From column must be a valid column/)
expect_foreign_key_to_exist(source_table_name, foreign_key_name)
end
end
context 'when run inside a transaction' do
it 'raises an error' do
expect(model).to receive(:transaction_open?).and_return(true)
expect do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
end.to raise_error(/can not be run inside a transaction/)
def expect_add_concurrent_fk_and_call_original(source_table_name, target_table_name, options)
expect(migration).to receive(:add_concurrent_foreign_key)
.ordered
.with(source_table_name, target_table_name, options)
.and_wrap_original do |_, source_table_name, target_table_name, options|
connection.add_foreign_key(source_table_name, target_table_name, **options)
end
end
end
end
context 'removing a foreign key' do
before do
allow(model).to receive(:transaction_open?).and_return(false)
end
context 'when the foreign key exists on the parent table' do
it 'does not attempt to create any foreign keys' do
expect(migration).to receive(:concurrent_partitioned_foreign_key_name).and_return(foreign_key_name)
context 'when the table has multiple foreign keys' do
before do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
model.add_partitioned_foreign_key :epic_issues, referenced_table
end
expect(migration).to receive(:foreign_key_exists?)
.with(source_table_name, target_table_name, **options)
.and_return(true)
it 'creates a trigger function without the removed cascade' do
expect_function_to_contain(function_name,
'delete from issue_assignees where issue_id = old.id',
'delete from epic_issues where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
expect(migration).not_to receive(:add_concurrent_foreign_key)
expect(migration).not_to receive(:with_lock_retries)
expect(migration).not_to receive(:add_foreign_key)
model.remove_partitioned_foreign_key :issue_assignees, referenced_table
migration.add_concurrent_partitioned_foreign_key(source_table_name, target_table_name, column: column_name)
expect_function_to_contain(function_name, 'delete from epic_issues where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
expect_foreign_key_not_to_exist(source_table_name, foreign_key_name)
end
end
context 'when the table has only one remaining foreign key' do
before do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
context 'when additional foreign key options are given' do
let(:options) do
{
column: column_name,
name: '_my_fk_name',
on_delete: :restrict,
validate: true
}
end
it 'removes the trigger function altogether' do
expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
model.remove_partitioned_foreign_key :issue_assignees, referenced_table
expect_function_not_to_exist(function_name)
expect_trigger_not_to_exist(referenced_table, trigger_name)
end
end
it 'forwards them to the foreign key helper methods' do
expect(migration).to receive(:foreign_key_exists?)
.with(source_table_name, target_table_name, **options)
.and_return(false)
context 'when the foreign key does not exist' do
before do
model.add_partitioned_foreign_key :issue_assignees, referenced_table
end
expect(migration).not_to receive(:concurrent_partitioned_foreign_key_name)
it 'ignores the invalid key and properly recreates the trigger function' do
expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
expect_add_concurrent_fk(partition1_name, target_table_name, **options)
expect_add_concurrent_fk(partition2_name, target_table_name, **options)
model.remove_partitioned_foreign_key :issues, referenced_table, column: :moved_to_id
expect(migration).to receive(:with_lock_retries).ordered.and_yield
expect(migration).to receive(:add_foreign_key).with(source_table_name, target_table_name, **options).ordered
expect_function_to_contain(function_name, 'delete from issue_assignees where issue_id = old.id')
expect_valid_function_trigger(referenced_table, trigger_name, function_name, after: 'delete')
migration.add_concurrent_partitioned_foreign_key(source_table_name, target_table_name,
column: column_name, name: '_my_fk_name', on_delete: :restrict)
end
end
context 'when run outside a transaction' do
it 'raises an error' do
expect(model).to receive(:transaction_open?).and_return(true)
expect do
model.remove_partitioned_foreign_key :issue_assignees, referenced_table
end.to raise_error(/can not be run inside a transaction/)
def expect_add_concurrent_fk(source_table_name, target_table_name, options)
expect(migration).to receive(:add_concurrent_foreign_key)
.ordered
.with(source_table_name, target_table_name, options)
end
end
end
......
spec/lib/gitlab/database/partitioning_migration_helpers/partitioned_foreign_key_spec.rb deleted 100644 → 0
View file @ c5a28073
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::PartitionedForeignKey do
let(:foreign_key) do
described_class.new(
to_table: 'issues',
from_table: 'issue_assignees',
from_column: 'issue_id',
to_column: 'id',
cascade_delete: true)
end
describe 'validations' do
it 'allows keys that reference valid tables and columns' do
expect(foreign_key).to be_valid
end
it 'does not allow keys without a valid to_table' do
foreign_key.to_table = 'this_is_not_a_real_table'
expect(foreign_key).not_to be_valid
expect(foreign_key.errors[:to_table].first).to eq('must be a valid table')
end
it 'does not allow keys without a valid from_table' do
foreign_key.from_table = 'this_is_not_a_real_table'
expect(foreign_key).not_to be_valid
expect(foreign_key.errors[:from_table].first).to eq('must be a valid table')
end
it 'does not allow keys without a valid to_column' do
foreign_key.to_column = 'this_is_not_a_real_fk'
expect(foreign_key).not_to be_valid
expect(foreign_key.errors[:to_column].first).to eq('must be a valid column')
end
it 'does not allow keys without a valid from_column' do
foreign_key.from_column = 'this_is_not_a_real_pk'
expect(foreign_key).not_to be_valid
expect(foreign_key.errors[:from_column].first).to eq('must be a valid column')
end
end
end
spec/support/helpers/database/table_schema_helpers.rb
View file @ ee816a4c
......@@ -43,6 +43,14 @@ module Database
expect(index_exists_by_name(name, schema: schema)).to be_nil
end
def expect_foreign_key_to_exist(table_name, name, schema: nil)
expect(foreign_key_exists_by_name(table_name, name, schema: schema)).to eq(true)
end
def expect_foreign_key_not_to_exist(table_name, name, schema: nil)
expect(foreign_key_exists_by_name(table_name, name, schema: schema)).to be_nil
end
def expect_check_constraint(table_name, name, definition, schema: nil)
expect(check_constraint_definition(table_name, name, schema: schema)).to eq("CHECK ((#{definition}))")
end
......@@ -133,6 +141,18 @@ module Database
SQL
end
def foreign_key_exists_by_name(table_name, foreign_key_name, schema: nil)
table_name = schema ? "#{schema}.#{table_name}" : table_name
connection.select_value(<<~SQL)
SELECT true
FROM pg_catalog.pg_constraint
WHERE pg_constraint.conrelid = '#{table_name}'::regclass
AND pg_constraint.contype = 'f'
AND pg_constraint.conname = '#{foreign_key_name}'
SQL
end
def check_constraint_definition(table_name, constraint_name, schema: nil)
table_name = schema ? "#{schema}.#{table_name}" : table_name
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7