Don't over protect the MySQL users on insert_all

MySQL don't support conflict target on insert_all so we can't pass the
:unique_by option. But, if you don't check if the index exists MySQL will
successfully inset the data.

If there is a conflict in an index like the primary key the database will
raise an error. We should let the database decide if the data is acceptable
or not.

The test added on this commit exercise a table that has composite primary
keys, but, in order to keep all the Rails behavior working (given Rails
doesn't officially support composite primary keys) we set the primary
key to an auto increment :id column.
This commit is contained in:
Rafael Mendonça França 2020-11-04 01:17:49 +00:00
parent 8fd81f4980
commit 096e25bce7
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
4 changed files with 57 additions and 2 deletions

View File

@ -69,6 +69,8 @@ module ActiveRecord
attr_reader :scope_attributes
def find_unique_index_for(unique_by)
return unique_by if !connection.supports_insert_conflict_target?
name_or_columns = unique_by || model.primary_key
match = Array(name_or_columns).map(&:to_s)

View File

@ -3,6 +3,7 @@
require "cases/helper"
require "models/author"
require "models/book"
require "models/cart"
require "models/speedometer"
require "models/subscription"
require "models/subscriber"
@ -208,6 +209,33 @@ class InsertAllTest < ActiveRecord::TestCase
end
end
def test_insert_all_and_upsert_all_works_with_composite_primary_keys_when_unique_by_is_provided
skip unless supports_insert_conflict_target?
assert_difference "Cart.count", 2 do
Cart.insert_all [{ id: 1, shop_id: 1, title: "My cart" }], unique_by: [:shop_id, :id]
Cart.upsert_all [{ id: 3, shop_id: 2, title: "My other cart" }], unique_by: [:shop_id, :id]
end
error = assert_raises ArgumentError do
Cart.insert_all! [{ id: 2, shop_id: 1, title: "My cart" }]
end
assert_match "No unique index found for id", error.message
end
def test_insert_all_and_upsert_all_works_with_composite_primary_keys_when_unique_by_is_not_provided
skip unless supports_insert_on_duplicate_skip? && !supports_insert_conflict_target?
assert_difference "Cart.count", 3 do
Cart.insert_all [{ id: 1, shop_id: 1, title: "My cart" }]
Cart.insert_all! [{ id: 2, shop_id: 1, title: "My cart 2" }]
Cart.upsert_all [{ id: 3, shop_id: 2, title: "My other cart" }]
end
end
def test_insert_logs_message_including_model_name
skip unless supports_insert_conflict_target?
@ -260,8 +288,18 @@ class InsertAllTest < ActiveRecord::TestCase
assert_equal "New edition", Book.find(1).name
end
def test_upsert_all_updates_existing_record_by_configured_primary_key
skip unless supports_insert_on_duplicate_update?
def test_upsert_all_does_notupdates_existing_record_by_when_there_is_no_key
skip unless supports_insert_on_duplicate_update? && !supports_insert_conflict_target?
Speedometer.create!(speedometer_id: "s3", name: "Very fast")
Speedometer.upsert_all [{ speedometer_id: "s3", name: "New Speedometer" }]
assert_equal "Very fast", Speedometer.find("s3").name
end
def test_upsert_all_updates_existing_record_by_configured_primary_key_fails_when_database_supports_insert_conflict_target
skip unless supports_insert_on_duplicate_update? && supports_insert_conflict_target?
error = assert_raises ArgumentError do
Speedometer.upsert_all [{ speedometer_id: "s1", name: "New Speedometer" }]

View File

@ -0,0 +1,5 @@
# frozen_string_literal: true
class Cart < ActiveRecord::Base
self.primary_key = :id
end

View File

@ -152,6 +152,16 @@ ActiveRecord::Schema.define do
create_table :carriers, force: true
create_table :carts, force: true, primary_key: [:shop_id, :id] do |t|
if current_adapter?(:Mysql2Adapter)
t.bigint :id, index: true, auto_increment: true, null: false
else
t.bigint :id, index: true, null: false
end
t.bigint :shop_id
t.string :title
end
create_table :categories, force: true do |t|
t.string :name, null: false
t.string :type