diff options
Diffstat (limited to 'activerecord')
-rw-r--r-- | activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb | 4 | ||||
-rw-r--r-- | activerecord/lib/active_record/insert_all.rb | 9 | ||||
-rw-r--r-- | activerecord/test/cases/insert_all_test.rb | 87 |
3 files changed, 95 insertions, 5 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index f5da19f0f6..ca8bbc14da 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -516,8 +516,8 @@ module ActiveRecord sql = +"INSERT #{insert.into} #{insert.values_list}" if insert.skip_duplicates? - any_column = quote_column_name(insert.model.columns.first.name) - sql << " ON DUPLICATE KEY UPDATE #{any_column}=#{any_column}" + no_op_column = quote_column_name(insert.keys.first) + sql << " ON DUPLICATE KEY UPDATE #{no_op_column}=#{no_op_column}" elsif insert.update_duplicates? sql << " ON DUPLICATE KEY UPDATE " sql << insert.updatable_columns.map { |column| "#{column}=VALUES(#{column})" }.join(",") diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index 4b02d40aa0..ed7a37b255 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -21,7 +21,10 @@ module ActiveRecord end def execute - connection.exec_query to_sql, "Bulk Insert" + message = "#{model} " + message += "Bulk " if inserts.many? + message += (on_duplicate == :update ? "Upsert" : "Insert") + connection.exec_query to_sql, message end def updatable_columns @@ -111,7 +114,7 @@ module ActiveRecord class Builder attr_reader :model - delegate :skip_duplicates?, :update_duplicates?, to: :insert_all + delegate :skip_duplicates?, :update_duplicates?, :keys, to: :insert_all def initialize(insert_all) @insert_all, @model, @connection = insert_all, insert_all.model, insert_all.connection @@ -122,7 +125,7 @@ module ActiveRecord end def values_list - types = extract_types_from_columns_on(model.table_name, keys: insert_all.keys) + types = extract_types_from_columns_on(model.table_name, keys: keys) values_list = insert_all.map_key_with_value do |key, value| bind = Relation::QueryAttribute.new(key, value, types[key]) diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index fc25701c80..f24c63031c 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -104,6 +104,44 @@ class InsertAllTest < ActiveRecord::TestCase end end + def test_insert_all_with_skip_duplicates_and_autonumber_id_not_given + skip unless supports_insert_on_duplicate_skip? + + assert_difference "Book.count", 1 do + # These two books are duplicates according to an index on %i[author_id name] + # but their IDs are not specified so they will be assigned different IDs + # by autonumber. We will get an exception from MySQL if we attempt to skip + # one of these records by assigning its ID. + Book.insert_all [ + { author_id: 8, name: "Refactoring" }, + { author_id: 8, name: "Refactoring" } + ] + end + end + + def test_insert_all_with_skip_duplicates_and_autonumber_id_given + skip unless supports_insert_on_duplicate_skip? + + assert_difference "Book.count", 1 do + Book.insert_all [ + { id: 200, author_id: 8, name: "Refactoring" }, + { id: 201, author_id: 8, name: "Refactoring" } + ] + end + end + + def test_skip_duplicates_strategy_does_not_secretly_upsert + skip unless supports_insert_on_duplicate_skip? + + book = Book.create!(author_id: 8, name: "Refactoring", format: "EXPECTED") + + assert_no_difference "Book.count" do + Book.insert(author_id: 8, name: "Refactoring", format: "UNEXPECTED") + end + + assert_equal "EXPECTED", book.reload.format + end + def test_insert_all_will_raise_if_duplicates_are_skipped_only_for_a_certain_conflict_target skip unless supports_insert_on_duplicate_skip? && supports_insert_conflict_target? @@ -143,6 +181,42 @@ class InsertAllTest < ActiveRecord::TestCase end end + def test_insert_logs_message_including_model_name + skip unless supports_insert_conflict_target? + + capture_log_output do |output| + Book.insert(name: "Rework", author_id: 1) + assert_match "Book Insert", output.string + end + end + + def test_insert_all_logs_message_including_model_name + skip unless supports_insert_conflict_target? + + capture_log_output do |output| + Book.insert_all [{ name: "Remote", author_id: 1 }, { name: "Renote", author_id: 1 }] + assert_match "Book Bulk Insert", output.string + end + end + + def test_upsert_logs_message_including_model_name + skip unless supports_insert_on_duplicate_update? + + capture_log_output do |output| + Book.upsert(name: "Remote", author_id: 1) + assert_match "Book Upsert", output.string + end + end + + def test_upsert_all_logs_message_including_model_name + skip unless supports_insert_on_duplicate_update? + + capture_log_output do |output| + Book.upsert_all [{ name: "Remote", author_id: 1 }, { name: "Renote", author_id: 1 }] + assert_match "Book Bulk Upsert", output.string + end + end + def test_upsert_all_updates_existing_records skip unless supports_insert_on_duplicate_update? @@ -186,4 +260,17 @@ class InsertAllTest < ActiveRecord::TestCase Book.insert_all! [{ unknown_attribute: "Test" }] end end + + private + + def capture_log_output + output = StringIO.new + old_logger, ActiveRecord::Base.logger = ActiveRecord::Base.logger, ActiveSupport::Logger.new(output) + + begin + yield output + ensure + ActiveRecord::Base.logger = old_logger + end + end end |