Make ActiveRecord::Base.exists? invoke find_initial to support :include scopes

Thanks Michael! I think the count approach sounds like a really good. I uploaded a new patch.

Cheers

Peter

Back when the current implementation of exists? came into existance using count == 1 was discussed. Using count has performance implications since it means the database has to find all rows (and count them) rather than just a row

Frederick, thanks for the input on performance implications of count. I just wanted to give a recap of the options discussed here. We have:

1) The current implementation of exists? that does a connection.select_all table_name.primary_key with :limit => 1 within scopes where eager loaded tables are referenced.

2) The current patch that uses count(table_name.primary_key) and potentially performs worse.

3) My original patch that does a find_initial table_name.primary_key. The downside here is that we instantiate AR objects unnecessarily. The upside is that scopes with references to eager loaded tables work, thanks to this piece of logic in find_every:

      if include_associations.any? && references_eager_loaded_tables?(options)
        records = find_with_associations(options)
      else
        records = find_by_sql(construct_finder_sql(options))
        if include_associations.any?
          preload_associations(records, include_associations)
        end
      end

Would it make sense to make it possible to do a find_every(:instantiate => false) that skips instantiation? Or would that maybe just be conceptually odd and convolute the code? If we had such an option then exists? using find_initial would be identical to the current implementation (connection.select_all) except when eager loaded tables are referenced in which case those would be included properly and the query wouldn't break.

Just to clarify, the implementation of exists? from my original patch looked like this:

def exists?(id_or_conditions = {})
  find_initial(
    :select => "#{quoted_table_name}.#{primary_key}",
    :conditions => expand_id_conditions(id_or_conditions)) ? true : false
end

So my idea was to add an :instantiate => false to the find_initial call.

Don't worry about skipping instantiation, that's just me being
paranoid about performance.

Upload a version with find_initial (the original patch is gone?) and
I'm happy to apply it.

Hi Michael!
I agree with you that the find_initial approach seems like the best at this point. I've attached a patch file.

Thanks!

Peter

(from [0380e9ca5fa872e56d0920e6255a2f20b6e01030]) Changed ActiveRecord::Base#exists? to invoke find_initial so that it is compatible with, and doesn't lose, :include scopes (references to eager loaded tables)

Signed-off-by: Michael Koziarski michael@koziarski.com
[#2543 Make ActiveRecord::Base.exists? invoke find_initial to support :include scopes state:committed] http://github.com/rails/rails/commit/0380e9ca5fa872e56d0920e6255a2f...