Ensure ActiveRecord::Base.find_in_batches fires doesnt fire an extra query unless needed

3 files changed, 17 insertions, 2 deletions

diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb
index 4ea932f34d..03bd4f9f93 100644
--- a/activerecord/lib/active_record/batches.rb
+++ b/activerecord/lib/active_record/batches.rb
@@ -49,12 +49,15 @@ module ActiveRecord
         raise "You can't specify a limit, it's forced to be the batch_size"  if options[:limit]
 
         start = options.delete(:start).to_i
+        batch_size = options.delete(:batch_size) || 1000
 
-        with_scope(:find => options.merge(:order => batch_order, :limit => options.delete(:batch_size) || 1000)) do
+        with_scope(:find => options.merge(:order => batch_order, :limit => batch_size)) do
           records = find(:all, :conditions => [ "#{table_name}.#{primary_key} >= ?", start ])
 
           while records.any?
             yield records
+
+            break if records.size < batch_size
             records = find(:all, :conditions => [ "#{table_name}.#{primary_key} > ?", records.last.id ])
           end
         end
diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb
index 705da9f88e..5009a90846 100644
--- a/activerecord/test/cases/batches_test.rb
+++ b/activerecord/test/cases/batches_test.rb
@@ -46,4 +46,16 @@ class EachTest < ActiveRecord::TestCase
       end
     end
   end
+
+  def test_find_in_batches_shouldnt_excute_query_unless_needed
+    post_count = Post.count
+
+    assert_queries(2) do
+      Post.find_in_batches(:batch_size => post_count) {|batch| assert_kind_of Array, batch }
+    end
+
+    assert_queries(1) do
+      Post.find_in_batches(:batch_size => post_count + 1) {|batch| assert_kind_of Array, batch }
+    end
+  end
 end
\ No newline at end of file
diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb
index a441a8f6e3..8045b136c2 100644
--- a/activerecord/test/cases/named_scope_test.rb
+++ b/activerecord/test/cases/named_scope_test.rb
@@ -324,7 +324,7 @@ class NamedScopeTest < ActiveRecord::TestCase
       Topic.approved.find_each(:batch_size => 1) {|t| assert t.approved? }
     end
 
-    assert_queries(3) do
+    assert_queries(2) do
       Topic.approved.find_in_batches(:batch_size => 2) do |group|
         group.each {|t| assert t.approved? }
       end