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

Already on GitHub? Sign in to your account

Add limit functionality to find first and last #649

Closed
lighthouse-import opened this Issue May 16, 2011 · 14 comments

Comments

Projects
None yet
1 participant

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/3565
Created by Stephen Celis - 2011-02-22 08:32:35 UTC

The convenience methods @.first@ and @.last@ on @ActiveRecord::Base@ have had me itching to pass in an integer limit as you can with @array#first@ and @#last@. E.g.,

array = [1, 2, "buckle", :my, Shoe]
array.first    # => 1
array.first(2) # => [1, 2]
array.first(1) # => [1]
array.last(2)  # => [:my, Shoe]

Attached is a patch that adds this functionality.

Person.first    # => #<Person id: 1>
Person.first(2) # => [#<Person id: 1>, #<Person id: 2>]
Person.first(1) # => [#<Person id: 1>]
Person.last(2)  # => [#<Person id: 49>, #<Person id: 50>]

Considerations:

ActiveRecord::Base subclasses are not kinds of Array, but named scopes and association proxies come closer. Consider the following:

>> Person.scoped({}).first    # SELECT * FROM `people` LIMIT 1
=> #<Person>
>> Person.scoped({}).first(2) # SELECT * FROM `people`
=> [#<Person>, #<Person>]

The first gets special treatment. The second does not. In order to make the second consistent, it only makes sense to add the behavior to @ActiveRecord::Base@ as well. (Named scope functionality requires additions to this patch, but I'd prefer a consensus before making any more changes.)

Additionally, it made the most sense to add logic through to @ActiveRecord.find_initial@, rather than merely to the convenience, surface methods, but this obviously could cause problems for those who have been blindly passing options hashes to @find(:first)@ and @:last@ that include a @:limit@. I'm open to the idea of a less-invasive approach that merely adds the functionality to the convenience methods, but this seemed less desirable and messier.

If this is of interest to others, it could also be added to @activeresource::Base@.

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-11 11:09:20 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Anil Wadghule - 2010-10-12 08:10:32 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Aditya Sanghi - 2010-10-12 09:10:45 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Aditya Sanghi - 2010-10-12 09:13:51 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Aditya Sanghi - 2010-10-13 17:13:26 UTC

@stephen, this functionality is now already in place in Rails 3.0.0. Marking this ticket as resolved. Feel free to comment, if you think otherwise.

Imported from Lighthouse.
Comment by Stephen Celis - 2010-10-13 22:43:22 UTC

Does not appear to be in place in 3.0.0:

% rails c
Loading development environment (Rails 3.0.0)
ruby-1.9.2-p0 > User.first 2
  User Load (0.2ms)  SELECT `users`.* FROM `users`
 => []
ruby-1.9.2-p0 > User.last 2
  User Load (0.3ms)  SELECT `users`.* FROM `users`
 => []

Imported from Lighthouse.
Comment by Stephen Celis - 2010-10-13 22:44:33 UTC

That is, the SQL query is not limited. Please re-open or change to a different state than "resolved."

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-13 22:47:36 UTC

I can verify this issue exists on Rails 3.

Imported from Lighthouse.
Comment by Aditya Sanghi - 2010-10-14 05:23:11 UTC

@stephen, patch also does not apply cleanly anymore since there has been much refactoring around ARel.

I misread the second half of your OP and looked on the net result ( which works ) but the query does not since it does not use limit but gets all and then pick n from array.

Feel free to submit another patch.

Imported from Lighthouse.
Comment by Elad Meidar - 2010-10-14 14:26:41 UTC

Makes sense that the Rails default would fetch all and select back n accordingly, proper SQL query should be implemented on the adapter level since it involves different syntax for each DBMS.

Imported from Lighthouse.
Comment by Stephen Celis - 2010-10-15 18:58:45 UTC

I'm happy to fix the patch if there's interest in getting this into Rails.

@elad, I'm not sure what you're saying. The Rails default for ActiveRecord::Base.first is to fetch one record from the database (with :limit => 1). As soon as you add a numeric argument, however, it fetches all, which is not what I would have expected. The only case I can think of where .first(n) would fetch all is ActiveRecord::Base.all.first(n), because .all returns an Array.

Imported from Lighthouse.
Comment by Ryan Bigg - 2010-10-19 07:33:09 UTC

Automatic cleanup of spam.

Imported from Lighthouse.
Comment by Piotr Sarnacki - 2010-12-17 11:10:20 UTC

I would add such functionality, anyone have anything against it?

Attachments saved to Gist: http://gist.github.com/971652

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment