From d681adbbb5e945994580a4e3b5103081888491b9 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 16 Sep 2018 05:02:40 +0900 Subject: Use table name qualified column name for update counters MySQL supports JOINs to UPDATE, so if column name isn't qualified by table name, it would cause an ambiguous error: ``` Mysql2::Error: Column 'integer' in field list is ambiguous: UPDATE `pets` INNER JOIN `toys` ON `toys`.`pet_id` = `pets`.`pet_id` SET `integer` = COALESCE(`integer`, 0) + 1 WHERE `toys`.`name` = ? ``` --- activerecord/lib/active_record/relation.rb | 16 +++++++--------- activerecord/test/cases/relation/update_all_test.rb | 8 ++++++++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 86e5d14c81..806f8a1cbb 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -349,7 +349,7 @@ module ActiveRecord stmt = Arel::UpdateManager.new - stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates)) + stmt.set Arel.sql(@klass.sanitize_sql_for_assignment(updates, table.name)) stmt.table(table) if has_join_values? || offset_value @@ -377,14 +377,14 @@ module ActiveRecord updates = counters.map do |counter_name, value| operator = value < 0 ? "-" : "+" - quoted_column = connection.quote_column_name(counter_name) + quoted_column = connection.quote_table_name_for_assignment(table.name, counter_name) "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{value.abs}" end if touch names = touch if touch != true touch_updates = klass.touch_attributes_with_time(*names) - updates << klass.sanitize_sql_for_assignment(touch_updates) unless touch_updates.empty? + updates << klass.sanitize_sql_for_assignment(touch_updates, table.name) unless touch_updates.empty? end update_all updates.join(", ") @@ -414,14 +414,12 @@ module ActiveRecord # Person.where(name: 'David').touch_all # # => "UPDATE \"people\" SET \"updated_at\" = '2018-01-04 22:55:23.132670' WHERE \"people\".\"name\" = 'David'" def touch_all(*names, time: nil) - updates = touch_attributes_with_time(*names, time: time) - if klass.locking_enabled? - quoted_locking_column = connection.quote_column_name(klass.locking_column) - updates = sanitize_sql_for_assignment(updates) + ", #{quoted_locking_column} = COALESCE(#{quoted_locking_column}, 0) + 1" + names << { time: time } + update_counters(klass.locking_column => 1, touch: names) + else + update_all klass.touch_attributes_with_time(*names, time: time) end - - update_all(updates) end # Destroys the records by instantiating each diff --git a/activerecord/test/cases/relation/update_all_test.rb b/activerecord/test/cases/relation/update_all_test.rb index e199f76197..09c365f31b 100644 --- a/activerecord/test/cases/relation/update_all_test.rb +++ b/activerecord/test/cases/relation/update_all_test.rb @@ -94,6 +94,14 @@ class UpdateAllTest < ActiveRecord::TestCase assert_equal posts(:welcome), comments(:greetings).post end + def test_update_counters_with_joins + assert_nil pets(:parrot).integer + + Pet.joins(:toys).where(toys: { name: "Bone" }).update_counters(integer: 1) + + assert_equal 1, pets(:parrot).reload.integer + end + def test_touch_all_updates_records_timestamps david = developers(:david) david_previously_updated_at = david.updated_at -- cgit v1.2.3