Skip to content

Commit

Permalink
find_in_batches should not mutate its argument
Browse files Browse the repository at this point in the history
  • Loading branch information
marcandre committed Jan 30, 2014
1 parent fec1028 commit 642106e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
4 changes: 2 additions & 2 deletions activerecord/lib/active_record/relation/batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ def find_in_batches(options = {})
logger.warn("Scoped order and limit are ignored, it's forced to be batch order and batch size")
end

start = options.delete(:start)
batch_size = options.delete(:batch_size) || 1000
start = options[:start]
batch_size = options[:batch_size] || 1000

relation = relation.reorder(batch_order).limit(batch_size)
records = start ? relation.where(table[primary_key].gteq(start)).to_a : relation.to_a
Expand Down
6 changes: 6 additions & 0 deletions activerecord/test/cases/batches_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,12 @@ def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then
assert_equal special_posts_ids, posts.map(&:id)
end

def test_find_in_batches_should_not_modify_passed_options
assert_nothing_raised do
Post.find_in_batches({ batch_size: 42, start: 1 }.freeze){}
end
end

def test_find_in_batches_should_use_any_column_as_primary_key
nick_order_subscribers = Subscriber.order('nick asc')
start_nick = nick_order_subscribers.second.nick
Expand Down

0 comments on commit 642106e

Please sign in to comment.