Permalink
Browse files

Revert "Fix find_in_batches with customized primary_key"

This reverts commit 761bc75.

This commit wasn't fixing any issue just using the same table for
different models with different primary keys.
  • Loading branch information...
1 parent 008eaeb commit eb876c4d07130f15be2cac7be968cc393f959c62 @spastorino spastorino committed Sep 21, 2012
@@ -76,21 +76,6 @@
*kennyj*
-* Fix `find_in_batches` when primary_key is set other than id.
- You can now use this method with the primary key which is not integer-based.
-
- Example:
-
- class Post < ActiveRecord::Base
- self.primary_key = :title
- end
-
- Post.find_in_batches(start: 'My First Post') do |batch|
- batch.each { |post| post.author.greeting }
- end
-
- *Toshiyuki Kawanishi*
-
* You can now override the generated accessor methods for stored attributes
and reuse the original behavior with `read_store_attribute` and `write_store_attribute`,
which are counterparts to `read_attribute` and `write_attribute`.
@@ -36,12 +36,12 @@ def find_each(options = {})
# want multiple workers dealing with the same processing queue. You can
# make worker 1 handle all the records between id 0 and 10,000 and
# worker 2 handle from 10,000 and beyond (by setting the +:start+
- # option on that worker). You can also use non-integer-based primary keys
- # if start point is set.
+ # option on that worker).
#
# It's not possible to set the order. That is automatically set to
- # ascending on the primary key (e.g. "id ASC") to make the batch ordering
- # work. You can't set the limit either, that's used to control
+ # ascending on the primary key ("id ASC") to make the batch ordering
+ # work. This also mean that this method only works with integer-based
+ # primary keys. You can't set the limit either, that's used to control
# the batch sizes.
#
# Person.where("age > 21").find_in_batches do |group|
@@ -62,16 +62,15 @@ def find_in_batches(options = {})
ActiveRecord::Base.logger.warn("Scoped order and limit are ignored, it's forced to be batch order and batch size")
end
- start = options.delete(:start)
- start ||= 0
+ start = options.delete(:start).to_i
@spastorino
spastorino Sep 21, 2012 Member

start could be a string fixed here 5b99cf0

batch_size = options.delete(:batch_size) || 1000
relation = relation.reorder(batch_order).limit(batch_size)
records = relation.where(table[primary_key].gteq(start)).to_a
while records.any?
records_size = records.size
- primary_key_offset = records.last.send(primary_key)
+ primary_key_offset = records.last.id
yield records
@@ -125,14 +125,18 @@ def test_find_in_batches_should_not_ignore_the_default_scope_if_it_is_other_then
end
def test_find_in_batches_should_use_any_column_as_primary_key
+ old_primary_key = Post.primary_key
+ Post.primary_key = :title
title_order_posts = Post.order('title asc')
start_title = title_order_posts.first.title
posts = []
- PostWithTitlePrimaryKey.find_in_batches(:batch_size => 1, :start => start_title) do |batch|
+ Post.find_in_batches(:batch_size => 1, :start => start_title) do |batch|
posts.concat(batch)
end
assert_equal title_order_posts.map(&:id), posts.map(&:id)
+ ensure
+ Post.primary_key = old_primary_key
end
end
@@ -186,8 +186,3 @@ class SpecialPostWithDefaultScope < ActiveRecord::Base
self.table_name = 'posts'
default_scope { where(:id => [1, 5,6]) }
end
-
-class PostWithTitlePrimaryKey < ActiveRecord::Base
- self.table_name = 'posts'
- self.primary_key = :title
-end

0 comments on commit eb876c4

Please sign in to comment.