mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Merge pull request #2251 from thedarkone/update-all-order
Bring back the ability to provide :order for update_all
This commit is contained in:
commit
a850cf7058
2 changed files with 25 additions and 9 deletions
|
@ -216,17 +216,13 @@ module ActiveRecord
|
||||||
if conditions || options.present?
|
if conditions || options.present?
|
||||||
where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates)
|
where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates)
|
||||||
else
|
else
|
||||||
limit = nil
|
stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)))
|
||||||
order = []
|
|
||||||
# Apply limit and order only if they're both present
|
if limit = arel.limit
|
||||||
if @limit_value.present? == @order_values.present?
|
stmt.take limit
|
||||||
limit = arel.limit
|
|
||||||
order = arel.orders
|
|
||||||
end
|
end
|
||||||
|
|
||||||
stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates)))
|
stmt.order(*arel.orders)
|
||||||
stmt.take limit if limit
|
|
||||||
stmt.order(*order)
|
|
||||||
stmt.key = table[primary_key]
|
stmt.key = table[primary_key]
|
||||||
@klass.connection.update stmt.to_sql, 'SQL', bind_values
|
@klass.connection.update stmt.to_sql, 'SQL', bind_values
|
||||||
end
|
end
|
||||||
|
|
|
@ -29,6 +29,26 @@ class PersistencesTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
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
|
def test_update_all_with_order_and_limit_updates_subset_only
|
||||||
author = authors(:david)
|
author = authors(:david)
|
||||||
assert_nothing_raised do
|
assert_nothing_raised do
|
||||||
|
|
Loading…
Reference in a new issue