Skip to content

Commit

Permalink
Bring back the ability to provide :order for update_all.
Browse files Browse the repository at this point in the history
  • Loading branch information
thedarkone committed Jul 25, 2011
1 parent 02691d3 commit 1086358
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 9 deletions.
14 changes: 5 additions & 9 deletions activerecord/lib/active_record/relation.rb
Expand Up @@ -216,17 +216,13 @@ def update_all(updates, conditions = nil, options = {})
if conditions || options.present?
where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates)
else
limit = nil
order = []
# Apply limit and order only if they're both present
if @limit_value.present? == @order_values.present?
limit = arel.limit
order = arel.orders
stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)))

if limit = arel.limit
stmt.take limit
end

stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)))
stmt.take limit if limit
stmt.order(*order)
stmt.order(*arel.orders)
stmt.key = table[primary_key]
@klass.connection.update stmt.to_sql, 'SQL', bind_values
end
Expand Down
20 changes: 20 additions & 0 deletions activerecord/test/cases/persistence_test.rb
Expand Up @@ -29,6 +29,26 @@ def test_update_all_ignores_order_without_limit_from_association
end
end

def test_update_all_doesnt_ignore_order
assert_equal authors(:david).id + 1, authors(:mary).id # make sure there is going to be a duplicate PK error
test_update_with_order_succeeds = lambda do |order|
begin
Author.order(order).update_all('id = id + 1')
rescue ActiveRecord::ActiveRecordError
false
end
end

if test_update_with_order_succeeds.call('id DESC')
assert !test_update_with_order_succeeds.call('id ASC') # test that this wasn't a fluke and using an incorrect order results in an exception
else
# test that we're failing because the current Arel's engine doesn't support UPDATE ORDER BY queries is using subselects instead
assert_sql(/\AUPDATE .+ \(SELECT .* ORDER BY id DESC\)\Z/i) do
test_update_with_order_succeeds.call('id DESC')
end
end
end

def test_update_all_with_order_and_limit_updates_subset_only
author = authors(:david)
assert_nothing_raised do
Expand Down

0 comments on commit 1086358

Please sign in to comment.