Skip to content

Commit

Permalink
Merge pull request #2855 from arunagw/fix_find_in_batches_master
Browse files Browse the repository at this point in the history
We need to recorder here. Need to drop the order from default scope.
  • Loading branch information
jonleighton committed Sep 4, 2011
2 parents e865d12 + 9066e34 commit 6c46506
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/relation/batches.rb
Expand Up @@ -62,7 +62,7 @@ def find_in_batches(options = {})
start = options.delete(:start).to_i start = options.delete(:start).to_i
batch_size = options.delete(:batch_size) || 1000 batch_size = options.delete(:batch_size) || 1000


relation = relation.except(:order).order(batch_order).limit(batch_size) relation = relation.reorder(batch_order).limit(batch_size)
records = relation.where(table[primary_key].gteq(start)).all records = relation.where(table[primary_key].gteq(start)).all


while records.any? while records.any?
Expand Down
20 changes: 20 additions & 0 deletions activerecord/test/cases/batches_test.rb
Expand Up @@ -113,7 +113,27 @@ def test_find_in_batches_should_not_use_records_after_yielding_them_in_case_orig
batch.map! { not_a_post } batch.map! { not_a_post }
end end
end end
end


def test_find_in_batches_should_ignore_the_order_default_scope
# First post is with title scope
first_post = PostWithDefaultScope.first
posts = []
PostWithDefaultScope.find_in_batches do |batch|
posts.concat(batch)
end
# posts.first will be ordered using id only. Title order scope should not apply here
assert_not_equal first_post, posts.first
assert_equal posts(:welcome), posts.first
end

def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then_order
special_posts_ids = SpecialPostWithDefaultScope.all.map(&:id)
posts = []
SpecialPostWithDefaultScope.find_in_batches do |batch|
posts.concat(batch)
end
assert_equal special_posts_ids, posts.map(&:id)
end end


end end
10 changes: 10 additions & 0 deletions activerecord/test/models/post.rb
Expand Up @@ -171,4 +171,14 @@ class PostWithDefaultInclude < ActiveRecord::Base
self.table_name = 'posts' self.table_name = 'posts'
default_scope includes(:comments) default_scope includes(:comments)
has_many :comments, :foreign_key => :post_id has_many :comments, :foreign_key => :post_id
end

class PostWithDefaultScope < ActiveRecord::Base
self.table_name = 'posts'
default_scope :order => :title
end

class SpecialPostWithDefaultScope < ActiveRecord::Base
self.table_name = 'posts'
default_scope where(:id => [1, 5,6])
end end

0 comments on commit 6c46506

Please sign in to comment.