Eager load of has_one :through with conditions on the :through table fails

No, because the two associations are loaded completely separately in the eager loading code, there is no way to apply the conditions on the :through association. I think the only way to fix it is to rewrite it so it loads the two associations in a single query like it does everywhere else.

I've been wanting for it to be that way (and for has many through) for a long long time.

I think a precondition is some refactoring some of the condition, join etc. generation for hmt/hot so that it is shared between eager loading and 'normal' loading (you could duplicate most of it but that wouldn't be very nice.

When I last looked at this the bit where it got non obvious was when you had

a has_many :bars a has_many :foos, :through => bars, :include => :yet_another_association

and the include of yet_another_association required falling back to the old join code

(because some preload stuff requires you to be able to add to the select clause, but the old include overwrites select, and we decided against allowing old include to support a limited form of select that would have sufficed here)

This may have gotten better because one change that went into 2.3 reduced the number of occasions we need to fall back to the old code because we now scan the joins clause. Unfortunately I don't really have time to investigate right now.

Here is some of what I monkey-patched into association_join() in associations.rb:

                      " #{join_type} %s ON (%s.%s = %s.%s%s%s%s%s%s) " % [
                        table_alias_for(through_reflection.klass.table_name, aliased_join_table_name),
                        connection.quote_table_name(parent.aliased_table_name),
                        connection.quote_column_name(parent.primary_key),
                        connection.quote_table_name(aliased_join_table_name),
                        connection.quote_column_name(jt_foreign_key),
                        jt_as_extra, jt_source_extra, jt_sti_extra,
                        through_reflection ? reflection_conditions(through_reflection.options[:conditions], aliased_join_table_name, through_reflection.klass) : '',
                        reflection ? reflection_conditions(reflection.options[:through_conditions], aliased_join_table_name, through_reflection.klass) : ''
                      ] +
                      " #{join_type} %s ON (%s.%s = %s.%s%s) " % [
                        table_name_and_alias,
                        connection.quote_table_name(aliased_table_name),
                        connection.quote_column_name(first_key),
                        connection.quote_table_name(aliased_join_table_name),
                        connection.quote_column_name(second_key),
                        as_extra
                      ]

which puts the through reflections conditions on the right join, as well as putting my new :through_conditions from the outer association on there too.

Also, at the bottom of join_association(), I modified the code that attaches the conditions to not include the conditions from the :through reflection (having included them directly in the join above:

# Only include reflection conditions here, not the through_reflection conditions
# as they have already been rolled into the join clause above.
# Also, use appropriate aliasing and handle Hash-style conditions.
#   Previous lines:
#             [through_reflection, reflection].each do |ref|
#               join << "AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} " if ref && ref.options[:conditions]
#             end
              [reflection].each do |ref|
                join << reflection_conditions(ref.options[:conditions]) if ref
              end

And, finally, to handle table aliasing correctly, and to support hash conditions, I abstracted the reflection condition handling:

              # Generates reflection condition clause with appropriate table aliasing.
              # By default, it uses aliased_table_name on reflection condition Hashes to avoid
              # errors such as:
              # Mysql::Error: Unknown column 'table_name.column_name' in 'on clause'.
              def reflection_conditions(conditions, reflection_aliased_table_name = aliased_table_name, reflection_klass = active_record)
                if conditions
                  if conditions.is_a?(Hash)
                    return " AND #{interpolate_sql(reflection_klass.send(:sanitize_sql_hash_for_conditions, conditions, reflection_aliased_table_name))} "
                  else
                    return " AND #{interpolate_sql(reflection_klass.send(:sanitize_sql, conditions))} "
                  end
                else
                  return ""
                end
              end

(Obviously there's also the minor changes to allow :through_conditions as an attribute on has_one and has_many).

I see that 2.3 has added support for hash conditions in associations, but it does not cope with aliasing when you join to the same table several times, which I think I've fixed above by passing the alias to sanitize_sql_hash_for_conditions().

I think we may be talking at cross purposes I (and I think Joseph) are talking about the other :include mechanism (in association_preload.rb)

Ah, OK. In that case, do my changes make any sense to you? Do they relate to another ticket that already exists perhaps? If not, should I raise a new ticket for it?