*_ids methods are only available on associations #5812

Closed
ahawkins opened this Issue Apr 11, 2012 · 18 comments

Projects

None yet

9 participants

@ahawkins

For example, if you have an association you can call association_ids. However if you apply a scope like: association.scoped you cannot get the ID's back without writing more code.

I propose that all ActiveRecord::Base objects have an ids method. This method will be available to all generated relations for that class and thus associations and base classes.

Here is the code I use in my app:

module ActiveRecordPluckIds
  extend ActiveSupport::Concern

  module ClassMethods
    def ids
      pluck "#{table_name}.id"
    end
  end
end

class ActiveRecord::Base
  include ActiveRecordPluckIds
end

If there is support for this, then I can make a pull request to update ActiveRecord. Thoughts?

@rolfb
Contributor
rolfb commented Apr 11, 2012

What is the use case?

@epochwolf

Is it really so hard to write association.scoped.map(&:id)?

@ahawkins

@epochwolf that example would load all the records then call the id method.

@rolfb I stated it in the description. association.scoped.ids or whenever you need ids for anything in the db without loading all the AR objects.

@epochwolf

Okay, I didn't know about pluck before. http://api.rubyonrails.org/classes/ActiveRecord/Calculations.html#method-i-pluck

I don't see why we need ids when you can do pluck(:field) on any relation already.

@radar
Contributor
radar commented Apr 11, 2012

I like this idea. If you want to get a subset of records for instance and return just their ids and then do something with them, like queue them up to be processed or something.

@ahawkins

@epochwolf because pluck(:field) does not take into account which table you want. I think this is simple enough that could be a part of ActiveRecord

@epochwolf

I do see one use case. If you have a primary key that isn't named id, this could save you from having to manually look up what that is. Though the code presented doesn't handle that right now.

@rolfb
Contributor
rolfb commented Apr 11, 2012

@twinturbo: that's not a "use case" :)

The code will have to account :id not being the primary column, but i regard this as a convenience method as .pluck(:id) will work. I like the consistency between .ids and .association_ids though.

@ahawkins

@rolfb my point is that pluck(:id) does not work in all uses cases because it doesn't take the table name into account.

@jonpaul
jonpaul commented Apr 11, 2012

I like this concept. I'd actually prefer *_ids to .pluck(:id) and I like the utility that it'll provide. Although MySQL doesn't seem to have the same issue as above on associations, it definitely blows up on something like a join. +1

@rolfb
Contributor
rolfb commented Apr 11, 2012

@twinturbo: didn't know that was a problem with pluck. why not fix pluck? either way, if you supply a pull request for this you'll probably have to use primary_key instead of id. Example: https://github.com/rails/rails/blob/90d96353e6dcd962b182e03f53f2214acde00907/activerecord/lib/active_record/relation/finder_methods.rb#L251

@ahawkins

@drogus what do you think?

@drogus
Member
drogus commented Apr 16, 2012

@twinturbo I like that idea and I think that changing pluck to work like this can be tricky. If we added table_name to pluck, there would be no easy way to use it with other tables (unless we treated symbol as default with table name implicitly attached and string as something that should go as is, but that would be confusing).

@ahawkins

@drogus you don't have to change pluck. Just pass a complete table.column_name string for the argument. I am not proposing any changes to pluck, just to use pluck to extend AR.

@drogus
Member
drogus commented Apr 16, 2012

@twinturbo sure, I was just referring to the idea of "just changing pluck to work well when using pluck(:id) instead of adding ids method".

To clarify - I would add ids and I would not change pluck to add table_name by default.

@ahawkins

@jeremy what do you think of this idea?

@Jonesy
Jonesy commented Apr 17, 2012

+1, this will make it easier to program stuff with ember and friends because it will serve ID's faster.

@stefanpenner
Contributor

+1

potential improvement would be default to primary_key, but allow override via passed in argument.
(Im assuming primary_key is available within this scope)

  def ids(column=primary_key)
    pluck "#{table_name}.#{column}"
  end
@wycats wycats closed this in 69c2307 Apr 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment