diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 8 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 6 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 18 |
3 files changed, 31 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index cbbfad615d..167bd8c82c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Fixed inconsistency with `first(n)` when used with `limit()`. + The `first(n)` finder now respects the `limit()`, making it consistent + with `relation.to_a.first(n)`, and also with the behavior of `last(n)`. + + Fixes #23979. + + *Brian Christian* + * Use `count(:all)` in `HasManyAssociation#count_records` to prevent invalid SQL queries for association counting. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index e61cacf6a7..50d0f14b98 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -532,7 +532,11 @@ module ActiveRecord else relation = ordered_relation - if limit_value.nil? || index < limit_value + if limit_value + limit = [limit_value - index, limit].min + end + + if limit > 0 relation = relation.offset(offset_index + index) unless index.zero? relation.limit(limit).to_a else diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index c0485a7be7..95bd987551 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -687,6 +687,24 @@ class FinderTest < ActiveRecord::TestCase assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3) end + def test_first_on_relation_with_limit_and_offset + post = posts("sti_comments") + + comments = post.comments.order(id: :asc) + assert_equal comments.limit(2).to_a.first, comments.limit(2).first + assert_equal comments.limit(2).to_a.first(2), comments.limit(2).first(2) + assert_equal comments.limit(2).to_a.first(3), comments.limit(2).first(3) + + assert_equal comments.offset(2).to_a.first, comments.offset(2).first + assert_equal comments.offset(2).to_a.first(2), comments.offset(2).first(2) + assert_equal comments.offset(2).to_a.first(3), comments.offset(2).first(3) + + comments = comments.offset(1) + assert_equal comments.limit(2).to_a.first, comments.limit(2).first + assert_equal comments.limit(2).to_a.first(2), comments.limit(2).first(2) + assert_equal comments.limit(2).to_a.first(3), comments.limit(2).first(3) + end + def test_take_and_first_and_last_with_integer_should_return_an_array assert_kind_of Array, Topic.take(5) assert_kind_of Array, Topic.first(5) |