Skip to content

Commit c5c4315

Browse files
committed
Respect model.record_timestamps? in upsert_all
Typically, Active Record respects the record_timestamps config on models when deciding to automatically set timestamp columns: - {created,updated}_{at,on} are set on record creation - updated_{at,on} are set on record update Currently, the machinery for bulk insert/upsert only automatically sets timestamps - on update, regardless of record_timestamps config It does not set any timestamps on record creation. This teaches it how to set timestamps on creation, and respect record_timestamp, so the behavior matches typical record operations.
1 parent eba7595 commit c5c4315

File tree

2 files changed

+34
-14
lines changed

2 files changed

+34
-14
lines changed

activerecord/lib/active_record/insert_all.rb

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ def initialize(model, inserts, on_duplicate:, returning: nil, unique_by: nil)
1212

1313
@model, @connection, @inserts, @keys = model, model.connection, inserts, inserts.first.keys.map(&:to_s)
1414
@on_duplicate, @returning, @unique_by = on_duplicate, returning, unique_by
15+
@record_timestamps = model.record_timestamps
1516

1617
disallow_raw_sql!(returning)
1718
disallow_raw_sql!(on_duplicate)
@@ -64,15 +65,29 @@ def map_key_with_value
6465
inserts.map do |attributes|
6566
attributes = attributes.stringify_keys
6667
attributes.merge!(scope_attributes) if scope_attributes
68+
attributes.reverse_merge!(timestamps_for_create) if record_timestamps?
6769

6870
verify_attributes(attributes)
6971

70-
keys.map do |key|
72+
keys_including_timestamps.map do |key|
7173
yield key, attributes[key]
7274
end
7375
end
7476
end
7577

78+
def record_timestamps?
79+
@record_timestamps
80+
end
81+
82+
# TODO: Consider remaining this method, as it only conditionally extends keys, not always
83+
def keys_including_timestamps
84+
@keys_including_timestamps ||= if record_timestamps?
85+
keys + model.all_timestamp_attributes_in_model
86+
else
87+
keys
88+
end
89+
end
90+
7691
private
7792
attr_reader :scope_attributes
7893

@@ -134,7 +149,7 @@ def unique_by_columns
134149

135150

136151
def verify_attributes(attributes)
137-
if keys != attributes.keys.to_set
152+
if keys_including_timestamps != attributes.keys.to_set
138153
raise ArgumentError, "All objects being inserted must have the same keys"
139154
end
140155
end
@@ -148,10 +163,14 @@ def disallow_raw_sql!(value)
148163
"by wrapping them in Arel.sql()."
149164
end
150165

166+
def timestamps_for_create
167+
model.all_timestamp_attributes_in_model.index_with(connection.high_precision_current_timestamp)
168+
end
169+
151170
class Builder # :nodoc:
152171
attr_reader :model
153172

154-
delegate :skip_duplicates?, :update_duplicates?, :keys, to: :insert_all
173+
delegate :skip_duplicates?, :update_duplicates?, :keys, :keys_including_timestamps, :record_timestamps?, to: :insert_all
155174

156175
def initialize(insert_all)
157176
@insert_all, @model, @connection = insert_all, insert_all.model, insert_all.connection
@@ -162,9 +181,10 @@ def into
162181
end
163182

164183
def values_list
165-
types = extract_types_from_columns_on(model.table_name, keys: keys)
184+
types = extract_types_from_columns_on(model.table_name, keys: keys_including_timestamps)
166185

167186
values_list = insert_all.map_key_with_value do |key, value|
187+
next value if Arel::Nodes::SqlLiteral === value
168188
connection.with_yaml_fallback(types[key].serialize(value))
169189
end
170190

@@ -196,7 +216,7 @@ def updatable_columns
196216
end
197217

198218
def touch_model_timestamps_unless(&block)
199-
return "" unless update_duplicates?
219+
return "" unless update_duplicates? && record_timestamps?
200220

201221
model.timestamp_attributes_for_update_in_model.filter_map do |column_name|
202222
if touch_timestamp_attribute?(column_name)
@@ -219,7 +239,7 @@ def touch_timestamp_attribute?(column_name)
219239
end
220240

221241
def columns_list
222-
format_columns(insert_all.keys)
242+
format_columns(insert_all.keys_including_timestamps)
223243
end
224244

225245
def extract_types_from_columns_on(table_name, keys:)

activerecord/test/cases/insert_all_test.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -394,15 +394,15 @@ def test_upsert_all_uses_given_updated_on_over_implicit_updated_on
394394
assert_equal updated_on, Book.find(101).updated_on
395395
end
396396

397-
def test_upsert_all_does_not_implicitly_set_timestamps_on_create_even_when_model_record_timestamps_is_true
397+
def test_upsert_all_implicitly_sets_timestamps_on_create_when_model_record_timestamps_is_true
398398
with_record_timestamps(Ship, true) do
399399
Ship.upsert_all [{ id: 101, name: "RSS Boaty McBoatface" }]
400400

401401
ship = Ship.find(101)
402-
assert_nil ship.created_at
403-
assert_nil ship.created_on
404-
assert_nil ship.updated_at
405-
assert_nil ship.updated_on
402+
assert_equal Time.new.year, ship.created_at.year
403+
assert_equal Time.new.year, ship.created_on.year
404+
assert_equal Time.new.year, ship.updated_at.year
405+
assert_equal Time.new.year, ship.updated_on.year
406406
end
407407
end
408408

@@ -434,7 +434,7 @@ def test_upsert_all_implicitly_sets_timestamps_on_update_when_model_record_times
434434
end
435435
end
436436

437-
def test_upsert_all_implicitly_sets_timestamps_on_update_even_when_model_record_timestamps_is_false
437+
def test_upsert_all_does_not_implicitly_set_timestamps_on_update_when_model_record_timestamps_is_false
438438
skip unless supports_insert_on_duplicate_update?
439439

440440
with_record_timestamps(Ship, false) do
@@ -445,8 +445,8 @@ def test_upsert_all_implicitly_sets_timestamps_on_update_even_when_model_record_
445445
ship = Ship.find(101)
446446
assert_nil ship.created_at
447447
assert_nil ship.created_on
448-
assert_equal Time.now.year, ship.updated_at.year
449-
assert_equal Time.now.year, ship.updated_on.year
448+
assert_nil ship.updated_at
449+
assert_nil ship.updated_on
450450
end
451451
end
452452

0 commit comments

Comments
 (0)