diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb index 0e3de9c453..7efd17d208 100644 --- a/activerecord/lib/active_record/insert_all.rb +++ b/activerecord/lib/active_record/insert_all.rb @@ -7,11 +7,12 @@ class InsertAll # :nodoc: attr_reader :model, :connection, :inserts, :keys attr_reader :on_duplicate, :returning, :unique_by, :update_sql - def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil) + def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil, record_timestamps: 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) @on_duplicate, @returning, @unique_by = on_duplicate, returning, unique_by + @record_timestamps = record_timestamps.nil? ? model.record_timestamps : record_timestamps disallow_raw_sql!(returning) disallow_raw_sql!(on_duplicate) @@ -64,15 +65,29 @@ def map_key_with_value inserts.map do |attributes| attributes = attributes.stringify_keys attributes.merge!(scope_attributes) if scope_attributes + attributes.reverse_merge!(timestamps_for_create) if record_timestamps? verify_attributes(attributes) - keys.map do |key| + keys_including_timestamps.map do |key| yield key, attributes[key] end end end + def record_timestamps? + @record_timestamps + end + + # TODO: Consider remaining this method, as it only conditionally extends keys, not always + def keys_including_timestamps + @keys_including_timestamps ||= if record_timestamps? + keys + model.all_timestamp_attributes_in_model + else + keys + end + end + private attr_reader :scope_attributes @@ -134,7 +149,7 @@ def unique_by_columns def verify_attributes(attributes) - if keys != attributes.keys.to_set + if keys_including_timestamps != attributes.keys.to_set raise ArgumentError, "All objects being inserted must have the same keys" end end @@ -148,10 +163,14 @@ def disallow_raw_sql!(value) "by wrapping them in Arel.sql()." end + def timestamps_for_create + model.all_timestamp_attributes_in_model.index_with(connection.high_precision_current_timestamp) + end + class Builder # :nodoc: attr_reader :model - delegate :skip_duplicates?, :update_duplicates?, :keys, to: :insert_all + delegate :skip_duplicates?, :update_duplicates?, :keys, :keys_including_timestamps, :record_timestamps?, to: :insert_all def initialize(insert_all) @insert_all, @model, @connection = insert_all, insert_all.model, insert_all.connection @@ -162,9 +181,10 @@ def into end def values_list - types = extract_types_from_columns_on(model.table_name, keys: keys) + types = extract_types_from_columns_on(model.table_name, keys: keys_including_timestamps) values_list = insert_all.map_key_with_value do |key, value| + next value if Arel::Nodes::SqlLiteral === value connection.with_yaml_fallback(types[key].serialize(value)) end @@ -196,6 +216,8 @@ def updatable_columns end def touch_model_timestamps_unless(&block) + return "" unless update_duplicates? && record_timestamps? + model.timestamp_attributes_for_update_in_model.filter_map do |column_name| if touch_timestamp_attribute?(column_name) "#{column_name}=(CASE WHEN (#{updatable_columns.map(&block).join(" AND ")}) THEN #{model.quoted_table_name}.#{column_name} ELSE #{connection.high_precision_current_timestamp} END)," @@ -213,11 +235,11 @@ def raw_update_sql attr_reader :connection, :insert_all def touch_timestamp_attribute?(column_name) - update_duplicates? && !insert_all.updatable_columns.include?(column_name) + insert_all.updatable_columns.exclude?(column_name) end def columns_list - format_columns(insert_all.keys) + format_columns(insert_all.keys_including_timestamps) end def extract_types_from_columns_on(table_name, keys:) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 26d2b31f40..9d04901a69 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -63,8 +63,8 @@ def create!(attributes = nil, &block) # go through Active Record's type casting and serialization. # # See ActiveRecord::Persistence#insert_all for documentation. - def insert(attributes, returning: nil, unique_by: nil) - insert_all([ attributes ], returning: returning, unique_by: unique_by) + def insert(attributes, returning: nil, unique_by: nil, record_timestamps: nil) + insert_all([ attributes ], returning: returning, unique_by: unique_by, record_timestamps: record_timestamps) end # Inserts multiple records into the database in a single SQL INSERT @@ -131,8 +131,8 @@ def insert(attributes, returning: nil, unique_by: nil) # { id: 1, title: "Rework" }, # { id: 2, title: "Eloquent Ruby" } # ]) - def insert_all(attributes, returning: nil, unique_by: nil) - InsertAll.new(self, attributes, on_duplicate: :skip, returning: returning, unique_by: unique_by).execute + def insert_all(attributes, returning: nil, unique_by: nil, record_timestamps: nil) + InsertAll.new(self, attributes, on_duplicate: :skip, returning: returning, unique_by: unique_by, record_timestamps: record_timestamps).execute end # Inserts a single record into the database in a single SQL INSERT @@ -141,8 +141,8 @@ def insert_all(attributes, returning: nil, unique_by: nil) # go through Active Record's type casting and serialization. # # See ActiveRecord::Persistence#insert_all! for more. - def insert!(attributes, returning: nil) - insert_all!([ attributes ], returning: returning) + def insert!(attributes, returning: nil, record_timestamps: nil) + insert_all!([ attributes ], returning: returning, record_timestamps: record_timestamps) end # Inserts multiple records into the database in a single SQL INSERT @@ -188,8 +188,8 @@ def insert!(attributes, returning: nil) # { id: 1, title: "Rework", author: "David" }, # { id: 1, title: "Eloquent Ruby", author: "Russ" } # ]) - def insert_all!(attributes, returning: nil) - InsertAll.new(self, attributes, on_duplicate: :raise, returning: returning).execute + def insert_all!(attributes, returning: nil, record_timestamps: nil) + InsertAll.new(self, attributes, on_duplicate: :raise, returning: returning, record_timestamps: record_timestamps).execute end # Updates or inserts (upserts) a single record into the database in a @@ -198,8 +198,8 @@ def insert_all!(attributes, returning: nil) # go through Active Record's type casting and serialization. # # See ActiveRecord::Persistence#upsert_all for documentation. - def upsert(attributes, on_duplicate: :update, returning: nil, unique_by: nil) - upsert_all([ attributes ], on_duplicate: on_duplicate, returning: returning, unique_by: unique_by) + def upsert(attributes, on_duplicate: :update, returning: nil, unique_by: nil, record_timestamps: nil) + upsert_all([ attributes ], on_duplicate: on_duplicate, returning: returning, unique_by: unique_by, record_timestamps: record_timestamps) end # Updates or inserts (upserts) multiple records into the database in a @@ -261,8 +261,8 @@ def upsert(attributes, on_duplicate: :update, returning: nil, unique_by: nil) # ], unique_by: :isbn) # # Book.find_by(isbn: "1").title # => "Eloquent Ruby" - def upsert_all(attributes, on_duplicate: :update, returning: nil, unique_by: nil) - InsertAll.new(self, attributes, on_duplicate: on_duplicate, returning: returning, unique_by: unique_by).execute + def upsert_all(attributes, on_duplicate: :update, returning: nil, unique_by: nil, record_timestamps: nil) + InsertAll.new(self, attributes, on_duplicate: on_duplicate, returning: returning, unique_by: unique_by, record_timestamps: record_timestamps).execute end # Given an attributes hash, +instantiate+ returns a new instance of diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb index 56cca2e1a5..8b511c7ab5 100644 --- a/activerecord/test/cases/insert_all_test.rb +++ b/activerecord/test/cases/insert_all_test.rb @@ -4,6 +4,8 @@ require "models/author" require "models/book" require "models/cart" +require "models/developer" +require "models/ship" require "models/speedometer" require "models/subscription" require "models/subscriber" @@ -393,6 +395,148 @@ def test_upsert_all_uses_given_updated_on_over_implicit_updated_on assert_equal updated_on, Book.find(101).updated_on end + def test_upsert_all_implicitly_sets_timestamps_on_create_when_model_record_timestamps_is_true + with_record_timestamps(Ship, true) do + Ship.upsert_all [{ id: 101, name: "RSS Boaty McBoatface" }] + + ship = Ship.find(101) + assert_equal Time.new.year, ship.created_at.year + assert_equal Time.new.year, ship.created_on.year + assert_equal Time.new.year, ship.updated_at.year + assert_equal Time.new.year, ship.updated_on.year + end + end + + def test_upsert_all_does_not_implicitly_set_timestamps_on_create_when_model_record_timestamps_is_true_but_overridden + with_record_timestamps(Ship, true) do + Ship.upsert_all [{ id: 101, name: "RSS Boaty McBoatface" }], record_timestamps: false + + ship = Ship.find(101) + assert_nil ship.created_at + assert_nil ship.created_on + assert_nil ship.updated_at + assert_nil ship.updated_on + end + end + + def test_upsert_all_does_not_implicitly_set_timestamps_on_create_when_model_record_timestamps_is_false + with_record_timestamps(Ship, false) do + Ship.upsert_all [{ id: 101, name: "RSS Boaty McBoatface" }] + + ship = Ship.find(101) + assert_nil ship.created_at + assert_nil ship.created_on + assert_nil ship.updated_at + assert_nil ship.updated_on + end + end + + def test_upsert_all_implicitly_sets_timestamps_on_create_when_model_record_timestamps_is_false_but_overridden + with_record_timestamps(Ship, false) do + Ship.upsert_all [{ id: 101, name: "RSS Boaty McBoatface" }], record_timestamps: true + + ship = Ship.find(101) + assert_equal Time.now.year, ship.created_at.year + assert_equal Time.now.year, ship.created_on.year + assert_equal Time.now.year, ship.updated_at.year + assert_equal Time.now.year, ship.updated_on.year + end + end + + def test_upsert_all_respects_created_at_precision_when_touched_implicitly + skip unless supports_datetime_with_precision? + + Book.upsert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 8) }] + + assert_not_predicate Book.find(101).created_at.usec, :zero?, "created_at should have sub-second precision" + end + + def test_upsert_all_implicitly_sets_timestamps_on_update_when_model_record_timestamps_is_true + skip unless supports_insert_on_duplicate_update? + + with_record_timestamps(Ship, true) do + travel_to(Date.new(2016, 4, 17)) { Ship.create! id: 101, name: "RSS Boaty McBoatface" } + + Ship.upsert_all [{ id: 101, name: "RSS Sir David Attenborough" }] + + ship = Ship.find(101) + assert_equal 2016, ship.created_at.year + assert_equal 2016, ship.created_on.year + assert_equal Time.now.year, ship.updated_at.year + assert_equal Time.now.year, ship.updated_on.year + end + end + + def test_upsert_all_does_not_implicitly_set_timestamps_on_update_when_model_record_timestamps_is_true_but_overridden + skip unless supports_insert_on_duplicate_update? + + with_record_timestamps(Ship, true) do + travel_to(Date.new(2016, 4, 17)) { Ship.create! id: 101, name: "RSS Boaty McBoatface" } + + Ship.upsert_all [{ id: 101, name: "RSS Sir David Attenborough" }], record_timestamps: false + + ship = Ship.find(101) + assert_equal 2016, ship.created_at.year + assert_equal 2016, ship.created_on.year + assert_equal 2016, ship.updated_at.year + assert_equal 2016, ship.updated_on.year + end + end + + def test_upsert_all_does_not_implicitly_set_timestamps_on_update_when_model_record_timestamps_is_false + skip unless supports_insert_on_duplicate_update? + + with_record_timestamps(Ship, false) do + Ship.create! id: 101, name: "RSS Boaty McBoatface" + + Ship.upsert_all [{ id: 101, name: "RSS Sir David Attenborough" }] + + ship = Ship.find(101) + assert_nil ship.created_at + assert_nil ship.created_on + assert_nil ship.updated_at + assert_nil ship.updated_on + end + end + + def test_upsert_all_implicitly_sets_timestamps_on_update_when_model_record_timestamps_is_false_but_overridden + skip unless supports_insert_on_duplicate_update? + + with_record_timestamps(Ship, false) do + Ship.create! id: 101, name: "RSS Boaty McBoatface" + + Ship.upsert_all [{ id: 101, name: "RSS Sir David Attenborough" }], record_timestamps: true + + ship = Ship.find(101) + assert_nil ship.created_at + assert_nil ship.created_on + assert_equal Time.now.year, ship.updated_at.year + assert_equal Time.now.year, ship.updated_on.year + end + end + + def test_upsert_all_implicitly_sets_timestamps_even_when_columns_are_aliased + skip unless supports_insert_on_duplicate_update? + + Developer.upsert_all [{ id: 101, name: "Alice" }] + alice = Developer.find(101) + + assert_not_nil alice.created_at + assert_not_nil alice.created_on + assert_not_nil alice.updated_at + assert_not_nil alice.updated_on + + alice.update!(created_at: nil, created_on: nil, updated_at: nil, updated_on: nil) + + Developer.upsert_all [{ id: alice.id, name: alice.name, salary: alice.salary * 2 }] + alice.reload + + assert_nil alice.created_at + assert_nil alice.created_on + assert_not_nil alice.updated_at + assert_not_nil alice.updated_on + end + def test_insert_all_raises_on_unknown_attribute assert_raise ActiveRecord::UnknownAttributeError do Book.insert_all! [{ unknown_attribute: "Test" }] @@ -515,4 +659,12 @@ def capture_log_output ActiveRecord::Base.logger = old_logger end end + + def with_record_timestamps(model, value) + original = model.record_timestamps + model.record_timestamps = value + yield + ensure + model.record_timestamps = original + end end