Merge pull request #7987 from alexisbernard/3-2_find_in_batches_compatible_with_strings

Fix find_in_batches with customized primary_key on 3-2-stable

3 files changed, 28 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a1a2977520..27c4839ac4 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -29,6 +29,10 @@
 
 ## Rails 3.2.9 (unreleased)
 
+*   Fix `find_in_batches` crashing when IDs are strings and start option is not specified.
+
+    *Alexis Bernard*
+
 *   Fix issue with collection associations calling first(n)/last(n) and attempting
     to set the inverse association when `:inverse_of` was used. Fixes #8087.
 
diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb
index 2fd89882ff..14701f668c 100644
--- a/activerecord/lib/active_record/relation/batches.rb
+++ b/activerecord/lib/active_record/relation/batches.rb
@@ -59,11 +59,11 @@ module ActiveRecord
         relation = apply_finder_options(finder_options)
       end
 
-      start = options.delete(:start).to_i
+      start = options.delete(:start)
       batch_size = options.delete(:batch_size) || 1000
 
       relation = relation.reorder(batch_order).limit(batch_size)
-      records = relation.where(table[primary_key].gteq(start)).all
+      records = start ? relation.where(table[primary_key].gteq(start)).to_a : relation.to_a
 
       while records.any?
         records_size = records.size
diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb
index 660098b9ad..ad2a749ab4 100644
--- a/activerecord/test/cases/batches_test.rb
+++ b/activerecord/test/cases/batches_test.rb
@@ -1,8 +1,9 @@
 require 'cases/helper'
 require 'models/post'
+require 'models/subscriber'
 
 class EachTest < ActiveRecord::TestCase
-  fixtures :posts
+  fixtures :posts, :subscribers
 
   def setup
     @posts = Post.order("id asc")
@@ -136,4 +137,24 @@ class EachTest < ActiveRecord::TestCase
     assert_equal special_posts_ids, posts.map(&:id)
   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
+
+    subscribers = []
+    Subscriber.find_in_batches(:batch_size => 1, :start => start_nick) do |batch|
+      subscribers.concat(batch)
+    end
+
+    assert_equal nick_order_subscribers[1..-1].map(&:id), subscribers.map(&:id)
+  end
+
+  def test_find_in_batches_should_use_any_column_as_primary_key_when_start_is_not_specified
+    Subscriber.count('nick') # preheat arel's table cache
+    assert_queries(Subscriber.count + 1) do
+      Subscriber.find_each(:batch_size => 1) do |subscriber|
+        assert_kind_of Subscriber, subscriber
+      end
+    end
+  end
 end