aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJosé Valim <jose.valim@gmail.com>2011-07-25 05:03:28 -0700
committerJosé Valim <jose.valim@gmail.com>2011-07-25 05:03:28 -0700
commita850cf705892a7f2062dd3740f9161f7be49e338 (patch)
treed13b261f5b4b5cb51c5c89880af5546923cc4db7
parentdffd920432380143417a0e9ac80de9d017ab223e (diff)
parent10863580aa14c7456f10c1fe9b4e93fcd18e0ef8 (diff)
downloadrails-a850cf705892a7f2062dd3740f9161f7be49e338.tar.gz
rails-a850cf705892a7f2062dd3740f9161f7be49e338.tar.bz2
rails-a850cf705892a7f2062dd3740f9161f7be49e338.zip
Merge pull request #2251 from thedarkone/update-all-order
Bring back the ability to provide :order for update_all
-rw-r--r--activerecord/lib/active_record/relation.rb14
-rw-r--r--activerecord/test/cases/persistence_test.rb20
2 files changed, 25 insertions, 9 deletions
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 2d0861d5c9..fff0ad1b83 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -216,17 +216,13 @@ module ActiveRecord
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
diff --git a/activerecord/test/cases/persistence_test.rb b/activerecord/test/cases/persistence_test.rb
index 57d1441128..9cd07fa8a5 100644
--- a/activerecord/test/cases/persistence_test.rb
+++ b/activerecord/test/cases/persistence_test.rb
@@ -29,6 +29,26 @@ class PersistencesTest < ActiveRecord::TestCase
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