Commit c17349a9 authored by Igor Drozdov's avatar Igor Drozdov

Fix bulk insert for models with composite primary key

We need to work it around by passing primary key explicitly
parent df37bd78
...@@ -141,6 +141,12 @@ module BulkInsertSafe ...@@ -141,6 +141,12 @@ module BulkInsertSafe
raise ArgumentError, "returns needs to be :ids or nil" raise ArgumentError, "returns needs to be :ids or nil"
end end
# Handle insertions for tables with a composite primary key
primary_keys = connection.schema_cache.primary_keys(table_name)
if unique_by.blank? && primary_key != primary_keys
unique_by = primary_keys
end
transaction do transaction do
items.each_slice(batch_size).flat_map do |item_batch| items.each_slice(batch_size).flat_map do |item_batch|
attributes = _bulk_insert_item_attributes( attributes = _bulk_insert_item_attributes(
......
...@@ -29,6 +29,15 @@ class ProjectAuthorization < ApplicationRecord ...@@ -29,6 +29,15 @@ class ProjectAuthorization < ApplicationRecord
EOF EOF
end end
end end
# This method overrides its ActiveRecord's version in order to work correctly
# with composite primary keys and fix the tests for Rails 6.1
#
# Consider using BulkInsertSafe module instead since we plan to refactor it in
# https://gitlab.com/gitlab-org/gitlab/-/issues/331264
def self.insert_all(attributes)
super(attributes, unique_by: connection.schema_cache.primary_keys(table_name))
end
end end
ProjectAuthorization.prepend_mod_with('ProjectAuthorization') ProjectAuthorization.prepend_mod_with('ProjectAuthorization')
...@@ -580,7 +580,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe ...@@ -580,7 +580,7 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe
it 'idempotently cleans up after failed background migrations' do it 'idempotently cleans up after failed background migrations' do
expect(partitioned_model.count).to eq(0) expect(partitioned_model.count).to eq(0)
partitioned_model.insert!(record2.attributes) partitioned_model.insert(record2.attributes, unique_by: [:id, :created_at])
expect_next_instance_of(Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable) do |backfill| expect_next_instance_of(Gitlab::Database::PartitioningMigrationHelpers::BackfillPartitionedTable) do |backfill|
allow(backfill).to receive(:transaction_open?).and_return(false) allow(backfill).to receive(:transaction_open?).and_return(false)
......
...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::ImportExport::ImportFailureService do ...@@ -43,7 +43,7 @@ RSpec.describe Gitlab::ImportExport::ImportFailureService do
let(:importable) { create(:merge_request) } let(:importable) { create(:merge_request) }
it 'raise exception' do it 'raise exception' do
expect { subject }.to raise_exception(ActiveRecord::AssociationNotFoundError, "Association named 'import_failures' was not found on MergeRequest; perhaps you misspelled it?") expect { subject }.to raise_exception(ActiveRecord::AssociationNotFoundError, /Association named 'import_failures' was not found on MergeRequest/)
end end
end end
end end
......
...@@ -20,6 +20,13 @@ RSpec.describe BulkInsertSafe do ...@@ -20,6 +20,13 @@ RSpec.describe BulkInsertSafe do
t.index :name, unique: true t.index :name, unique: true
end end
create_table :bulk_insert_items_with_composite_pk, id: false, force: true do |t|
t.integer :id, null: true
t.string :name, null: true
end
execute("ALTER TABLE bulk_insert_items_with_composite_pk ADD PRIMARY KEY (id,name);")
end end
end end
...@@ -27,6 +34,7 @@ RSpec.describe BulkInsertSafe do ...@@ -27,6 +34,7 @@ RSpec.describe BulkInsertSafe do
ActiveRecord::Schema.define do ActiveRecord::Schema.define do
drop_table :bulk_insert_items, force: true drop_table :bulk_insert_items, force: true
drop_table :bulk_insert_parent_items, force: true drop_table :bulk_insert_parent_items, force: true
drop_table :bulk_insert_items_with_composite_pk, force: true
end end
end end
...@@ -227,5 +235,28 @@ RSpec.describe BulkInsertSafe do ...@@ -227,5 +235,28 @@ RSpec.describe BulkInsertSafe do
end end
end end
end end
context 'when a model with composite primary key is inserted' do
let_it_be(:bulk_insert_items_with_composite_pk_class) do
Class.new(ActiveRecord::Base) do
self.table_name = 'bulk_insert_items_with_composite_pk'
include BulkInsertSafe
end
end
let(:new_object) { bulk_insert_items_with_composite_pk_class.new(id: 1, name: 'composite') }
it 'successfully inserts an item' do
expect(ActiveRecord::InsertAll).to receive(:new)
.with(
bulk_insert_items_with_composite_pk_class, [new_object.as_json], on_duplicate: :raise, returning: false, unique_by: %w[id name]
).and_call_original
expect { bulk_insert_items_with_composite_pk_class.bulk_insert!([new_object]) }.to(
change(bulk_insert_items_with_composite_pk_class, :count).from(0).to(1)
)
end
end
end end
end end
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