[PATCH] Eager Loading Duplicate Table Alias with has_many :through

The problem seems to be that has_many_through association constructs the inner join without letting the JoinDependency class know that the joined tables must be aliased.

I can't see a simple fix because the only communication between has_many_through and JoinDependency is through normal find options (options[:joins], and thus SQL snippets).

To fix this JoinDependency class must either parse the joins manually to know which table to alias (probably not a good idea) or the association code should somehow communicate the automatically generated join to the JoinDependency code which would then know to alias the table (this would probably require quite large changes to associations code).

A bit hackish approach to fix this would be to allow find() to accept a parameter which indicates that a specific table name is already used in a join and must thus be aliased if an :include OUTER JOIN is generated for a table with the same name.

The query

@people = Post.find(id).people.find(:all, :include => :posts)

is identical to

@people = People.find(:all, 
:include => :posts, 
:joins => 'inner join readers on readers.person_id = people.person_id', 
:conditions => ['readers.post_id = ?',id])

but this also does not work. The workaround is to manually alias the join table

@people = People.find(:all, 
:include => :posts, 
:joins => 'inner join readers as readers2 on readers2.person_id = people.person_id', 
:conditions => ['readers2.post_id = ?',id])

I've had a similar problem with has_and_belongs_to_many. I have a Country model and a NewsStory model in my project, and a view that shows the latest news for a country. Each story shows links to the other countries it's linked to, and it should be possible to eager load them. I'll upload a patch with an exception-throwing unit test to show what I mean.

Also, I've found that intercepting the finder methods in the case shown in my patch doesn't work. I'm doing the following to convert HABTM includes to joins (I'll only include one of the finder methods:

def find_every_with_default_includes(options)
  add_default_includes(options)
  find_every_without_default_includes(options)
rescue ActiveRecord::StatementInvalid
  convert_problematic_includes_to_joins(options)
  find_every_without_default_includes(options)
end
alias_method_chain(:find_every, :default_includes)

def convert_problematic_includes_to_joins(options)
  includes, joins, i = [], [options[:joins].to_s], 0
  options[:include].to_a.each do |inc|
    if inc.is_a?(Hash)
      includes << inc
      next
    end
    association = reflect_on_association(inc)
    unless association.macro.to_s == 'has_and_belongs_to_many'
      includes << inc
      next
    end
    i += 1
    assoc_class = Kernel.const_get(association.class_name)
    opts = association.options
    joins << <<-end_of_sql
      LEFT OUTER JOIN #{opts[:join_table]} AS ibd_join_table_#{i}
      ON ibd_join_table_#{i}.#{opts[:foreign_key]} =
          #{table_name}.#{primary_key}
      LEFT OUTER JOIN #{assoc_class.table_name} AS ibd_assoc_table_#{i}
      ON ibd_assoc_table_#{i}.#{assoc_class.primary_key} =
          ibd_join_table_#{i}.#{opts[:association_foreign_key]}
    end_of_sql
  end
  options[:include] = includes.blank? ? nil : includes
  options[:joins] = joins * ' '
end

It's from a plugin I'm working on, which lets you specify a default set of includes in your model. The joins it creates are passed to the finder method after the rescue, but the SQL generated does not contain the join instructions. I've tried putting convert_problematic_includes_to_joins before the rescue so it always gets fired, and the join statements are included in no-association and has_many situations, e.g.

NewsStory.find(:all)
NewsSource.find(8).news_stories.find(:all)

where the NewsSource class has_many :news_stories. Both these statements include my join instructions (NewsStory is set to include its HABTM association with :countries by default). In a HABTM situation the stories are fetched using an INNER JOIN with the join table - could this be overwriting the :joins option I'm passing in? Is there a workaround for this or is it a genuine bug?

Re: my last comment: I'm posting this :joins issue as a separate ticket with a suggested fix: #8937.

I've managed to get this working in the plugin I'm developing (files here). It's a terrible hack, but it works. Not sure how I'd change the framework if I were to build this in directly, though. How it works:

• Let find_every run as normal. If StatementInvalid is raised, catch it and rewrite any troublesome :includes as :joins.
• Store information about the joined classes and table aliases in an instance variable for use by other methods.
• Overwrite add_joins! to allow :joins on many-to-many scoped finds.
• Wrap using_limitable_reflections? so it remembers that you were using non-limitable reflections before you removed them from :include. This is so that correct result sets are returned if you have duplicate many-to-many links in your DB.
• Wrap column_aliases so that it writes out column aliases for your new :joins tables. Without this the data won't be fetched from the DB.
• Overwrite find_with_associations so that it passes information about the joined tables to JoinDependency.
• Wrap JoinDependency#instantiate so that it accepts this information and uses it to pull row data into objects so that eager loading works properly.

Like I said, terrible hack, but thought some might be interested. It works for me with habtm and has_many :through associations.

Okay, apologies for the convoluted workaround above. I found a simpler solution that only overwrites one method - one that my 'workaround' itself overwrote rather than intercepting it. I've included the tests from the orignial patches in the fix patch, with a couple of corrections. It works by checking the SQL supplied to add_joins! and the join fragment for naming clashes, then goes through and renames any trouble spots. It might not hold up if you're using string comparison to do joins on, but that's not really a common use case. It will work just fine with numeric comparison based joins. It tests OK against MySQL.

Not sure why the patch duplicate_join_table_names_fix_with_tests.2.diff is still around, I meant to replace it. It doesn't work - use the one without the '2'.

This is the latest version of the patch, not the '.2' file

thanks get a cool air force ones

Changzhou Sunpower Solar Water Heater Co., Ltd is a private company, professional producing solar water heater, and having the most advanced production line in Chinese, like automatic foaming machine imported from Italy, automatic argon arc welding machine from America, and numerical control punch. We use Germany Bayer's polyurethane and SUS304 stainless steel from Korea and Taiwan. Our company is one of the earliest companies engaging in exporting, having many years experience in exporting, and 90% of our products are exported to overseas market.

Sunpower Solar Energy Industry Co., Ltd, covers an area of nearly 150000 square metre, Now it has more than 300 workers and staff members among which the primary. Medium and high grade technicians share 65% of the total personnel. It is of abumdant technical force, advanced craft testing means. Its annual productive capacity reaches to 300000 sets. Our factory is the first to pass the ISO9001: 2000 quality system attestation among the same trade. Our products have get EN12975: 2 certificate, and all of the products are strictly complied with its standard. The goods reach to the first-class international standard. Now already export 62 counties. Our gain is "Where is sunlight, where is Sunpower". OEM also welcome.

http://www.solar-water-heaters.com.cn

Thanks Air Force One