Touch updated_at when upsert_all modifies a record

boblail · kaspth · commit 3a8668bd7389 · 2020-02-10T01:00:03.000+01:00
- When a user passes `updated_at` to `upsert_all`, the given value is used.
- When a user omits `updated_at`, `upsert_all` touches the timestamp if (but only if) any upserted values differ.

Preserve Rails' ability to generate intelligent cache keys for ActiveRecord when using `upsert_all` frequently to sync imported data.
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -510,6 +510,7 @@ def build_insert_sql(insert) # :nodoc:
           sql << " ON DUPLICATE KEY UPDATE #{no_op_column}=#{no_op_column}"
         elsif insert.update_duplicates?
           sql << " ON DUPLICATE KEY UPDATE "
+          sql << insert.touch_model_timestamps_unless { |column| "#{column}<=>VALUES(#{column})" }
           sql << insert.updatable_columns.map { |column| "#{column}=VALUES(#{column})" }.join(",")
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -450,6 +450,7 @@ def build_insert_sql(insert) # :nodoc:
           sql << " ON CONFLICT #{insert.conflict_target} DO NOTHING"
         elsif insert.update_duplicates?
           sql << " ON CONFLICT #{insert.conflict_target} DO UPDATE SET "
+          sql << insert.touch_model_timestamps_unless { |column| "#{insert.model.quoted_table_name}.#{column} IS NOT DISTINCT FROM excluded.#{column}" }
           sql << insert.updatable_columns.map { |column| "#{column}=excluded.#{column}" }.join(",")
         end
 
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -319,6 +319,7 @@ def build_insert_sql(insert) # :nodoc:
           sql << " ON CONFLICT #{insert.conflict_target} DO NOTHING"
         elsif insert.update_duplicates?
           sql << " ON CONFLICT #{insert.conflict_target} DO UPDATE SET "
+          sql << insert.touch_model_timestamps_unless { |column| "#{column} IS excluded.#{column}" }
           sql << insert.updatable_columns.map { |column| "#{column}=excluded.#{column}" }.join(",")
         end
 
diff --git a/activerecord/lib/active_record/insert_all.rb b/activerecord/lib/active_record/insert_all.rb
@@ -153,9 +153,21 @@ def updatable_columns
           quote_columns(insert_all.updatable_columns)
         end
 
+        def touch_model_timestamps_unless(&block)
+          model.send(:timestamp_attributes_for_update_in_model).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 CURRENT_TIMESTAMP END),"
+            end
+          end.compact.join
+        end
+
         private
           attr_reader :connection, :insert_all
 
+          def touch_timestamp_attribute?(column_name)
+            update_duplicates? && !insert_all.updatable_columns.include?(column_name)
+          end
+
           def columns_list
             format_columns(insert_all.keys)
           end
diff --git a/activerecord/test/cases/insert_all_test.rb b/activerecord/test/cases/insert_all_test.rb
@@ -273,6 +273,46 @@ def test_upsert_all_does_not_perform_an_upsert_if_a_partial_index_doesnt_apply
     assert_equal ["Out of the Silent Planet", "Perelandra"], Book.where(isbn: "1974522598").order(:name).pluck(:name)
   end
 
+  def test_upsert_all_does_not_touch_updated_at_when_values_do_not_change
+    skip unless supports_insert_on_duplicate_update?
+
+    updated_at = Time.now.utc - 5.years
+    Book.insert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 1), updated_at: updated_at }]
+    Book.upsert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 1) }]
+
+    assert_in_delta updated_at, Book.find(101).updated_at, 1
+  end
+
+  def test_upsert_all_touches_updated_at_and_updated_on_when_values_change
+    skip unless supports_insert_on_duplicate_update?
+
+    Book.insert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 1), updated_at: 5.years.ago, updated_on: 5.years.ago }]
+    Book.upsert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 8) }]
+
+    assert_equal Time.now.year, Book.find(101).updated_at.year
+    assert_equal Time.now.year, Book.find(101).updated_on.year
+  end
+
+  def test_upsert_all_uses_given_updated_at_over_implicit_updated_at
+    skip unless supports_insert_on_duplicate_update?
+
+    updated_at = Time.now.utc - 1.year
+    Book.insert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 1), updated_at: 5.years.ago }]
+    Book.upsert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 8), updated_at: updated_at }]
+
+    assert_in_delta updated_at, Book.find(101).updated_at, 1
+  end
+
+  def test_upsert_all_uses_given_updated_on_over_implicit_updated_on
+    skip unless supports_insert_on_duplicate_update?
+
+    updated_on = Time.now.utc.to_date - 30
+    Book.insert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 1), updated_on: 5.years.ago }]
+    Book.upsert_all [{ id: 101, name: "Out of the Silent Planet", published_on: Date.new(1938, 4, 8), updated_on: updated_on }]
+
+    assert_equal updated_on, Book.find(101).updated_on
+  end
+
   def test_insert_all_raises_on_unknown_attribute
     assert_raise ActiveRecord::UnknownAttributeError do
       Book.insert_all! [{ unknown_attribute: "Test" }]
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
@@ -119,6 +119,10 @@
     t.datetime :published_on
     t.index [:author_id, :name], unique: true
     t.index :isbn, where: "published_on IS NOT NULL", unique: true
+
+    t.datetime :created_at
+    t.datetime :updated_at
+    t.date :updated_on
   end
 
   create_table :booleans, force: true do |t|
