Skip to content

Commit fa1f7b8

Browse files
committed
Modify update_all to use a sub select query in some cases:
- In rails@216b37b, we attempted a fix let `update_all` use a subselect query instead of the `UPDATE FROM` scheme when a LEFT JOIN is used. This fix doesn't apply however for leading joings that get produced by a `through :table` which first creates a INNER JOIN, and then other joins given by the user. So such code `Post.through_comments.left_joins(:author)` will continue to try using the `UPDATE FROM` scheme and this won't work (see the commit mentioned above why). The fix in this commit checks whether there is a left join on the target, no matter whether it's the first join, and in such case makes `update_all` uses a subselect query.
1 parent fe25c50 commit fa1f7b8

File tree

3 files changed

+16
-3
lines changed

3 files changed

+16
-3
lines changed

activerecord/lib/arel/visitors/postgresql.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def prepare_update_statement(o)
6464
# The PostgreSQL dialect isn't flexible enough to allow anything other than a inner join
6565
# for the first join:
6666
# UPDATE table SET .. FROM joined_table WHERE ...
67-
(o.relation.right.first.is_a?(Arel::Nodes::InnerJoin))
67+
(o.relation.right.all? { |join| join.is_a?(Arel::Nodes::InnerJoin) || join.right.expr.right.relation != o.relation.left })
6868
o
6969
else
7070
super

activerecord/lib/arel/visitors/sqlite.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def prepare_update_statement(o)
6464
# The SQLite3 dialect isn't flexible enough to allow anything other than a inner join
6565
# for the first join:
6666
# UPDATE table SET .. FROM joined_table WHERE ...
67-
(o.relation.right.first.is_a?(Arel::Nodes::InnerJoin))
67+
(o.relation.right.all? { |join| join.is_a?(Arel::Nodes::InnerJoin) || join.right.expr.right.relation != o.relation.left })
6868
o
6969
else
7070
super

activerecord/test/cases/relation/update_all_test.rb

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,19 @@
1313
require "models/post"
1414
require "models/person"
1515
require "models/pet"
16+
require "models/pet_treasure"
17+
require "models/ship"
1618
require "models/toy"
1719
require "models/topic"
20+
require "models/treasure"
1821
require "models/tag"
1922
require "models/tagging"
2023
require "models/warehouse_thing"
2124
require "models/cpk"
2225

2326
class UpdateAllTest < ActiveRecord::TestCase
2427
fixtures :authors, :author_addresses, :comments, :companies, :developers, :owners, :posts, :people, :pets, :toys, :tags,
25-
:taggings, "warehouse-things", :cpk_orders, :cpk_order_agreements
28+
:taggings, :treasures, "warehouse-things", :cpk_orders, :cpk_order_agreements
2629

2730
class TopicWithCallbacks < ActiveRecord::Base
2831
self.table_name = :topics
@@ -105,6 +108,16 @@ def test_dynamic_update_all_with_one_joined_table
105108
end
106109
end
107110

111+
def test_dynamic_update_all_with_a_through_join
112+
pet = pets(:parrot)
113+
treasure = treasures(:diamond)
114+
115+
PetTreasure.create(pet: pet, treasure: treasure)
116+
117+
assert_operator pet.treasures.left_joins(:ship).update_all(name: "Gold"), :>, 0
118+
assert_equal("Gold", treasure.reload.name)
119+
end
120+
108121
def test_dynamic_update_all_with_one_join_on_the_target_and_one_indirect_join
109122
update_fragment = if current_adapter?(:TrilogyAdapter, :Mysql2Adapter)
110123
"toys.name = owners.name"

0 commit comments

Comments
 (0)