Skip to content

Commit

Permalink
Merge pull request rails#9969 from divineforest/fix-find-in-batches
Browse files Browse the repository at this point in the history
Fail early with "Primary key not included in the custom select clause" i...
  • Loading branch information
senny committed Jan 21, 2014
2 parents 9322e07 + 691709d commit 6a8a7f8
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 6 deletions.
8 changes: 8 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
* Fail early with "Primary key not included in the custom select clause"
in find_in_batches.

Before this patch find_in_batches raises this error only on second iteration.
So you will know about the problem only when you get the batch size threshold.

*Alexander Balashov*

* Ensure `second` through `fifth` methods act like the `first` finder.

The famous ordinal Array instance methods defined in ActiveSupport
Expand Down
7 changes: 2 additions & 5 deletions activerecord/lib/active_record/relation/batches.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,13 @@ def find_in_batches(options = {})
while records.any?
records_size = records.size
primary_key_offset = records.last.id
raise "Primary key not included in the custom select clause" unless primary_key_offset

yield records

break if records_size < batch_size

if primary_key_offset
records = relation.where(table[primary_key].gt(primary_key_offset)).to_a
else
raise "Primary key not included in the custom select clause"
end
records = relation.where(table[primary_key].gt(primary_key_offset)).to_a
end
end

Expand Down
4 changes: 3 additions & 1 deletion activerecord/test/cases/batches_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ def test_each_enumerator_should_execute_one_query_per_batch

def test_each_should_raise_if_select_is_set_without_id
assert_raise(RuntimeError) do
Post.select(:title).find_each(:batch_size => 1) { |post| post }
Post.select(:title).find_each(batch_size: 1) { |post|
flunk "should not call this block"
}
end
end

Expand Down

0 comments on commit 6a8a7f8

Please sign in to comment.