From 231d7676f72947bae765b9bd885b134aaf949921 Mon Sep 17 00:00:00 2001 From: Josh Kalderimis Date: Sun, 9 May 2010 12:24:25 +0300 Subject: corrected AR find_each and find_in_batches to raise when the user uses select but does not specify the primary key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- activerecord/lib/active_record/relation/batches.rb | 9 ++++++++- activerecord/test/cases/batches_test.rb | 14 ++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 1c61e7d450..4649e3b376 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -67,11 +67,18 @@ module ActiveRecord relation = relation.except(:order).order(batch_order).limit(batch_size) records = relation.where(primary_key.gteq(start)).all + key_value = self.primary_key.name + while records.any? yield records break if records.size < batch_size - records = relation.where(primary_key.gt(records.last.id)).all + + last_value = records.last.send(key_value) + + raise "You must include the primary key if you define a select" unless last_value.present? + + records = relation.where(primary_key.gt(last_value)).all end end diff --git a/activerecord/test/cases/batches_test.rb b/activerecord/test/cases/batches_test.rb index 83deabb5b7..dcc49e12ca 100644 --- a/activerecord/test/cases/batches_test.rb +++ b/activerecord/test/cases/batches_test.rb @@ -17,6 +17,20 @@ class EachTest < ActiveRecord::TestCase end end + def test_each_should_raise_if_select_is_set_without_id + assert_raise(RuntimeError) do + Post.find_each(:select => :title, :batch_size => 1) { |post| post } + end + end + + def test_each_should_execute_if_id_is_in_select + assert_queries(4) do + Post.find_each(:select => "id, title, type", :batch_size => 2) do |post| + assert_kind_of Post, post + end + end + end + def test_each_should_raise_if_the_order_is_set assert_raise(RuntimeError) do Post.find_each(:order => "title") { |post| post } -- cgit v1.2.3