Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix find_nth with limit_value #25274

Merged
merged 1 commit into from
Feb 26, 2017

Conversation

kamipo
Copy link
Member

@kamipo kamipo commented Jun 4, 2016

If the index exceeds a limit, simply return an empty result without
querying the database.

@rails-bot
Copy link

r? @senny

(@rails-bot has picked a reviewer for you, use r? to override)

@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch from e7fddc3 to b6a4924 Compare June 4, 2016 12:58
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch 4 times, most recently from d14a26c to bc150ff Compare August 8, 2016 07:26
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch from bc150ff to ed4d3e0 Compare August 15, 2016 20:23
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch 3 times, most recently from 39b7ff2 to 988d3ec Compare September 12, 2016 19:31
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch 2 times, most recently from 11fb982 to 9e1b00b Compare December 29, 2016 10:02
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch from 9e1b00b to bfbc460 Compare January 16, 2017 14:06
If the `index` exceeds a `limit`, simply return an empty result without
querying the database.
@kamipo kamipo force-pushed the fix_find_nth_with_limit_value branch from bfbc460 to 84f4ab9 Compare February 26, 2017 12:20
@kamipo
Copy link
Member Author

kamipo commented Feb 26, 2017

@pixeltrix Could you review the fix?

@pixeltrix pixeltrix merged commit 385799a into rails:master Feb 26, 2017
@pixeltrix
Copy link
Contributor

@kamipo I was a little concerned that relation.limit(limit).to_a could result in loading large result sets but I don't think there was anyway it could be nil - took a while to check the various code paths though.

Thanks for fixing 👍

@kamipo kamipo deleted the fix_find_nth_with_limit_value branch February 27, 2017 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants