Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merge pull request #2257 from thedarkone/udpate-all-order-3-1

Backport #2251 into 3-1-stable
  • Loading branch information...
commit 5bc0020bb60e48089cf457a419f256355ce38893 2 parents 1d97a7c + 787194e
Santiago Pastorino spastorino authored
14 activerecord/lib/active_record/relation.rb
View
@@ -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
20 activerecord/test/cases/persistence_test.rb
View
@@ -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
Please sign in to comment.
Something went wrong with that request. Please try again.