From 9673735074626a8e3920cc90c0d1c2f18bfffc8e Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 21 Sep 2012 19:42:14 -0300 Subject: start could be a string Related to 761bc751d31c22e2c2fdae2b4cdd435b68b6d783 and eb876c4d07130f15be2cac7be968cc393f959c62 Conflicts: activerecord/lib/active_record/relation/batches.rb activerecord/test/cases/batches_test.rb --- activerecord/lib/active_record/relation/batches.rb | 2 +- activerecord/test/cases/batches_test.rb | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 2fd89882ff..e7d916f2da 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -59,7 +59,7 @@ module ActiveRecord relation = apply_finder_options(finder_options) end - start = options.delete(:start).to_i + start = options.delete(:start) || 0 batch_size = options.delete(:batch_size) || 1000 relation = relation.reorder(batch_order).limit(batch_size) diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 660098b9ad..392898d080 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -136,4 +136,19 @@ 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 + old_primary_key = Post.primary_key + Post.primary_key = :title + title_order_posts = Post.order('title asc') + start_title = title_order_posts.second.title + + posts = [] + Post.find_in_batches(:batch_size => 1, :start => start_title) do |batch| + posts.concat(batch) + end + + assert_equal title_order_posts[1..-1].map(&:id), posts.map(&:id) + ensure + Post.primary_key = old_primary_key + end end -- cgit v1.2.3 From 7031e365d6a8c00b62354f4abe8dff4802b38adf Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Sat, 22 Sep 2012 10:57:54 -0300 Subject: Fix test_find_in_batches_should_use_any_column_as_primary_key --- activerecord/test/cases/batches_test.rb | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 392898d080..16a93d97c6 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") @@ -137,18 +138,14 @@ class EachTest < ActiveRecord::TestCase 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.second.title + nick_order_subscribers = Subscriber.order('nick asc') + start_nick = nick_order_subscribers.second.nick - posts = [] - Post.find_in_batches(:batch_size => 1, :start => start_title) do |batch| - posts.concat(batch) + subscribers = [] + Subscriber.find_in_batches(:batch_size => 1, :start => start_nick) do |batch| + subscribers.concat(batch) end - assert_equal title_order_posts[1..-1].map(&:id), posts.map(&:id) - ensure - Post.primary_key = old_primary_key + assert_equal nick_order_subscribers[1..-1].map(&:id), subscribers.map(&:id) end end -- cgit v1.2.3 From 16d98b2a41dec3619b9bd48b6b534406d9d07ef4 Mon Sep 17 00:00:00 2001 From: Alexis Bernard Date: Tue, 30 Oct 2012 15:54:16 +0100 Subject: Fix find_in_batches against string IDs when start option is not specified. Conflicts: activerecord/CHANGELOG.md activerecord/lib/active_record/relation/batches.rb --- activerecord/CHANGELOG.md | 4 ++++ activerecord/lib/active_record/relation/batches.rb | 4 ++-- activerecord/test/cases/batches_test.rb | 9 +++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3e071cfa01..483370de03 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,9 @@ ## 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 e7d916f2da..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) || 0 + 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 16a93d97c6..ad2a749ab4 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -148,4 +148,13 @@ class EachTest < ActiveRecord::TestCase 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 -- cgit v1.2.3