From 04309aee82468fa4c4b3d92a533e84a96533f236 Mon Sep 17 00:00:00 2001 From: Matthew Draper Date: Fri, 18 Dec 2015 15:54:04 +1030 Subject: Implement limit & offset for ourselves We know the query will return exactly one row for each entry in the `ids` array, so we can do all the limit/offset calculations on that array, in advance. I also split our new ordered-ids behaviour out of the existing `find_some` method: especially with this change, the conditionals were overwhelming the actual logic. --- .../lib/active_record/relation/finder_methods.rb | 25 ++++++++++++++++------ activerecord/test/cases/finder_test.rb | 4 ++-- 2 files changed, 21 insertions(+), 8 deletions(-) (limited to 'activerecord') diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 3aa38dcf78..fc4e891b02 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -444,6 +444,10 @@ module ActiveRecord end def find_some(ids) + return find_some_ordered(ids) unless order_values.present? + + result = where(primary_key => ids).to_a + expected_size = if limit_value && ids.size > limit_value limit_value @@ -454,18 +458,27 @@ module ActiveRecord # 11 ids with limit 3, offset 9 should give 2 results. if offset_value && (ids.size - offset_value < expected_size) expected_size = ids.size - offset_value + end + + if result.size == expected_size + result else - ids = ids.first(expected_size) if order_values.empty? + raise_record_not_found_exception!(ids, result.size, expected_size) end + end - result = where(primary_key => ids).to_a + def find_some_ordered(ids) + ids = ids.slice(offset_value || 0, limit_value || ids.size) || [] + + result = except(:limit, :offset).where(primary_key => ids).to_a + + if result.size == ids.size + pk_type = @klass.type_for_attribute(primary_key) - if result.size == expected_size - return result if order_values.present? records_by_id = result.index_by(&:id) - ids.collect { |id| records_by_id[id.to_i] }.compact + ids.map { |id| records_by_id.fetch(pk_type.cast(id)) } else - raise_record_not_found_exception!(ids, result.size, expected_size) + raise_record_not_found_exception!(ids, result.size, ids.size) end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 47059fe9e4..3c3f3284ac 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -109,7 +109,7 @@ class FinderTest < ActiveRecord::TestCase assert_equal 'The Fifth Topic of the day', records[2].title end - def test_find_with_ids_and_offset # failing with offset + def test_find_with_ids_and_offset records = Topic.offset(2).find([3,2,5,1,4]) assert_equal 3, records.size assert_equal 'The Fifth Topic of the day', records[0].title @@ -261,7 +261,7 @@ class FinderTest < ActiveRecord::TestCase assert_equal topics(:second).title, Topic.find([2]).first.title end - def test_find_by_ids_with_limit_and_offset # failing with offset + def test_find_by_ids_with_limit_and_offset assert_equal 2, Entrant.limit(2).find([1,3,2]).size entrants = Entrant.limit(3).offset(2).find([1,3,2]) assert_equal 1, entrants.size -- cgit v1.2.3