Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow insert_all on relation #38899

Merged
merged 1 commit into from Apr 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
* Support bulk insert/upsert on relation to preserve scope values.

*Josef Šimánek*, *Ryuta Kamizono*

* Preserve column comment value on changing column name on MySQL.

*Islam Taha*
Expand Down
12 changes: 12 additions & 0 deletions activerecord/lib/active_record/association_relation.rb
Expand Up @@ -31,6 +31,18 @@ def create!(attributes = nil, &block)
scoping { @association.create!(attributes, &block) }
end

%w(insert insert_all insert! insert_all! upsert upsert_all).each do |method|
class_eval <<~RUBY
def #{method}(attributes, **kwargs)
if @association.reflection.through_reflection?
raise ArgumentError, "Bulk insert or upsert is currently not supported for has_many through association"
end

scoping { klass.#{method}(attributes, **kwargs) }
end
RUBY
end

private
def exec_queries
super do |record|
Expand Down
Expand Up @@ -1096,7 +1096,9 @@ def reset_scope # :nodoc:
SpawnMethods,
].flat_map { |klass|
klass.public_instance_methods(false)
} - self.public_instance_methods(false) - [:select] + [:scoping, :values]
} - self.public_instance_methods(false) - [:select] + [
:scoping, :values, :insert, :insert_all, :insert!, :insert_all!, :upsert, :upsert_all
]

delegate(*delegate_methods, to: :scope)

Expand Down
12 changes: 11 additions & 1 deletion activerecord/lib/active_record/insert_all.rb
Expand Up @@ -10,9 +10,15 @@ class InsertAll # :nodoc:
def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil)
raise ArgumentError, "Empty list of attributes passed" if inserts.blank?

@model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s).to_set
@model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s)
@on_duplicate, @returning, @unique_by = on_duplicate, returning, unique_by

if model.scope_attributes?
@scope_attributes = model.scope_attributes
@keys |= @scope_attributes.keys
end
@keys = @keys.to_set

@returning = (connection.supports_insert_returning? ? primary_keys : false) if @returning.nil?
@returning = false if @returning == []

Expand Down Expand Up @@ -49,6 +55,8 @@ def update_duplicates?
def map_key_with_value
inserts.map do |attributes|
attributes = attributes.stringify_keys
attributes.merge!(scope_attributes) if scope_attributes

verify_attributes(attributes)

keys.map do |key|
Expand All @@ -58,6 +66,8 @@ def map_key_with_value
end

private
attr_reader :scope_attributes

def find_unique_index_for(unique_by)
name_or_columns = unique_by || model.primary_key
match = Array(name_or_columns).map(&:to_s)
Expand Down
59 changes: 59 additions & 0 deletions activerecord/test/cases/insert_all_test.rb
@@ -1,8 +1,11 @@
# frozen_string_literal: true

require "cases/helper"
require "models/author"
require "models/book"
require "models/speedometer"
require "models/subscription"
require "models/subscriber"

class ReadonlyNameBook < Book
attr_readonly :name
Expand Down Expand Up @@ -341,6 +344,62 @@ def test_insert_all_with_enum_values
assert_equal ["published", "proposed"], Book.where(isbn: ["1234566", "1234567"]).order(:id).pluck(:status)
end

def test_insert_all_on_relation
author = Author.create!(name: "Jimmy")

assert_difference "author.books.count", +1 do
author.books.insert_all!([{ name: "My little book", isbn: "1974522598" }])
end
end

def test_insert_all_on_relation_precedence
author = Author.create!(name: "Jimmy")
second_author = Author.create!(name: "Bob")

assert_difference "author.books.count", +1 do
author.books.insert_all!([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }])
end
end

def test_insert_all_create_with
assert_difference "Book.where(format: 'X').count", +2 do
Book.create_with(format: "X").insert_all!([ { name: "A" }, { name: "B" } ])
end
end

def test_insert_all_has_many_through
book = Book.first
assert_raise(ArgumentError) { book.subscribers.insert_all!([ { nick: "Jimmy" } ]) }
end

def test_upsert_all_on_relation
author = Author.create!(name: "Jimmy")

assert_difference "author.books.count", +1 do
author.books.upsert_all([{ name: "My little book", isbn: "1974522598" }])
end
end

def test_upsert_all_on_relation_precedence
author = Author.create!(name: "Jimmy")
second_author = Author.create!(name: "Bob")

assert_difference "author.books.count", +1 do
author.books.upsert_all([{ name: "My little book", isbn: "1974522598", author_id: second_author.id }])
end
end

def test_upsert_all_create_with
assert_difference "Book.where(format: 'X').count", +2 do
Book.create_with(format: "X").upsert_all([ { name: "A" }, { name: "B" } ])
end
end

def test_upsert_all_has_many_through
book = Book.first
assert_raise(ArgumentError) { book.subscribers.upsert_all([ { nick: "Jimmy" } ]) }
end

private
def capture_log_output
output = StringIO.new
Expand Down