Skip to content

Commit e0d1491

Browse files
committed
Don't recalculate GROUP BY in {update,delete}_all
These values are already calculated when building the SelectManager arel, so we can use them instead of recalculating/rebuilding them. Specifically, this prevents calling `arel_columns` after building the relation, which opens up the possibility to cache the built Arel ast. It couldn't be cached before because `arel_columns` may modify the relation. Going through the {Update,Delete}Manager's ast in `Crud` is necessary to prevent `Group(Group(SQL))` because `#group` will wrap its arguments in `Group` nodes.
1 parent 73761aa commit e0d1491

File tree

2 files changed

+6
-9
lines changed

2 files changed

+6
-9
lines changed

activerecord/lib/active_record/relation.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -628,14 +628,13 @@ def update_all(updates)
628628
arel = eager_loading? ? apply_join_dependency.arel : build_arel(c)
629629
arel.source.left = table
630630

631-
group_values_arel_columns = arel_columns(group_values.uniq)
632631
having_clause_ast = having_clause.ast unless having_clause.empty?
633632
key = if model.composite_primary_key?
634633
primary_key.map { |pk| table[pk] }
635634
else
636635
table[primary_key]
637636
end
638-
stmt = arel.compile_update(values, key, having_clause_ast, group_values_arel_columns)
637+
stmt = arel.compile_update(values, key, having_clause_ast)
639638
c.update(stmt, "#{model} Update All").tap { reset }
640639
end
641640
end
@@ -1045,14 +1044,13 @@ def delete_all
10451044
arel = eager_loading? ? apply_join_dependency.arel : build_arel(c)
10461045
arel.source.left = table
10471046

1048-
group_values_arel_columns = arel_columns(group_values.uniq)
10491047
having_clause_ast = having_clause.ast unless having_clause.empty?
10501048
key = if model.composite_primary_key?
10511049
primary_key.map { |pk| table[pk] }
10521050
else
10531051
table[primary_key]
10541052
end
1055-
stmt = arel.compile_delete(key, having_clause_ast, group_values_arel_columns)
1053+
stmt = arel.compile_delete(key, having_clause_ast)
10561054

10571055
c.delete(stmt, "#{model} Delete All").tap { reset }
10581056
end

activerecord/lib/arel/crud.rb

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@ def create_insert
1717
def compile_update(
1818
values,
1919
key = nil,
20-
having_clause = nil,
21-
group_values_columns = []
20+
having_clause = nil
2221
)
2322
um = UpdateManager.new(source)
2423
um.set(values)
@@ -28,19 +27,19 @@ def compile_update(
2827
um.wheres = constraints
2928
um.key = key
3029

31-
um.group(group_values_columns) unless group_values_columns.empty?
30+
um.ast.groups = @ctx.groups
3231
um.having(having_clause) unless having_clause.nil?
3332
um
3433
end
3534

36-
def compile_delete(key = nil, having_clause = nil, group_values_columns = [])
35+
def compile_delete(key = nil, having_clause = nil)
3736
dm = DeleteManager.new(source)
3837
dm.take(limit)
3938
dm.offset(offset)
4039
dm.order(*orders)
4140
dm.wheres = constraints
4241
dm.key = key
43-
dm.group(group_values_columns) unless group_values_columns.empty?
42+
dm.ast.groups = @ctx.groups
4443
dm.having(having_clause) unless having_clause.nil?
4544
dm
4645
end

0 commit comments

Comments
 (0)