17477 1 false 2009-01-09T20:22:37+00:00 32604 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 1033 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T23:29:15+00:00 51960 Jonathan Monahan Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x The first problem is that has_one :through eager loading doesn't pass through :conditions or :order like a has_many :through does. But once you pass those down, you have the problem that the through association is preloaded separately from the base association, so at that point the conditions throw a SQL error because the table can't be found. has_many :through has the same problem. The patch has a test that shows the broken functionality. The first problem is that has_one :through eager loading doesn't pass through :conditions or :order like a has_many :through does. But once you pass those down, you have the problem that the through association is preloaded separately from the base association, so at that point the conditions throw a SQL error because the table can't be found. has_many :through has the same problem. The patch has a test that shows the broken functionality. <div><p>The first problem is that has_one :through eager loading doesn't pass through :conditions or :order like a has_many :through does. But once you pass those down, you have the problem that the through association is preloaded separately from the base association, so at that point the conditions throw a SQL error because the table can't be found.</p> <p>has_many :through has the same problem.</p> <p>The patch has a test that shows the broken functionality.</p></div> 0 The first problem is that has_one :through eager loading doesn't pass through :conditions or :order like a has_many :through does. But once you pass those down, you have the problem that the through association is preloaded separately from the base association, so at that point the conditions throw a SQL error because the table can't be found. has_many :through has the same problem. The patch has a test that shows the broken functionality. <div><p>The first problem is that has_one :through eager loading doesn't pass through :conditions or :order like a has_many :through does. But once you pass those down, you have the problem that the through association is preloaded separately from the base association, so at that point the conditions throw a SQL error because the table can't be found.</p> <p>has_many :through has the same problem.</p> <p>The patch has a test that shows the broken functionality.</p></div> false 2009-01-09T20:22:37+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include eager_loading has_one_through 2009-01-09T20:22:38+00:00 32604 Joseph Palermo Joseph Palermo http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 false 2009-03-10T12:19:14+00:00 32604 --- :assigned_user: 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include eager_loading has_one_through 2009-03-10T12:19:20+00:00 1366 Pratik Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 Is this possibly related to the fact that there is no way to apply conditions to the :through association from the source association? I have monkey-patched this on 2.2.2 by adding support for :through_conditions, i.e. conditions that are applied to the join table join rather than to the target table join. Does that make sense? <div><p>Is this possibly related to the fact that there is no way to apply conditions to the :through association from the source association?</p> <p>I have monkey-patched this on 2.2.2 by adding support for :through_conditions, i.e. conditions that are applied to the join table join rather than to the target table join. Does that make sense?</p></div> false 2009-03-26T10:27:01+00:00 32604 --- :tag: ":include eager_loading has_one_through" 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T10:27:06+00:00 51960 Jonathan Monahan Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 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. <div><p>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.</p></div> false 2009-03-26T16:22:18+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T16:22:22+00:00 32604 Joseph Palermo Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 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. <div><p>I've been wanting for it to be that way (and for has many through) for a long long time.</p> <p>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.</p> <p>When I last looked at this the bit where it got non obvious was when you had</p> <p>a has_many :bars a has_many :foos, :through =&gt; bars, :include =&gt; :yet_another_association</p> <p>and the include of yet_another_association required falling back to the old join code</p> <p>(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)</p> <p>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.</p></div> false 2009-03-26T16:39:47+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T16:39:50+00:00 17477 Frederick Cheung Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 Here is some of what I monkey-patched into association_join() in associations.rb: @@@ ruby " #{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: @@@ ruby # 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: @@@ ruby # 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(). <div><p>Here is some of what I monkey-patched into association_join() in associations.rb:</p> <pre><code class="ruby"> &quot; #{join_type} %s ON (%s.%s = %s.%s%s%s%s%s%s) &quot; % [ 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) : '' ] + &quot; #{join_type} %s ON (%s.%s = %s.%s%s) &quot; % [ 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 ] </code></pre> <p>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.</p> <p>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:</p> <pre><code class="ruby"> # 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 &lt;&lt; &quot;AND #{interpolate_sql(sanitize_sql(ref.options[:conditions]))} &quot; if ref &amp;&amp; ref.options[:conditions] # end [reflection].each do |ref| join &lt;&lt; reflection_conditions(ref.options[:conditions]) if ref end </code></pre> <p>And, finally, to handle table aliasing correctly, and to support hash conditions, I abstracted the reflection condition handling:</p> <pre><code class="ruby"> # 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 &quot; AND #{interpolate_sql(reflection_klass.send(:sanitize_sql_hash_for_conditions, conditions, reflection_aliased_table_name))} &quot; else return &quot; AND #{interpolate_sql(reflection_klass.send(:sanitize_sql, conditions))} &quot; end else return &quot;&quot; end end </code></pre> <p>(Obviously there's also the minor changes to allow :through_conditions as an attribute on has_one and has_many).</p> <p>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().</p></div> false 2009-03-26T16:58:54+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T16:58:59+00:00 51960 Jonathan Monahan Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 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) <div><p>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)</p></div> false 2009-03-26T18:39:36+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T18:39:38+00:00 17477 Frederick Cheung Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 17477 1 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? <div><p>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?</p></div> false 2009-03-26T23:29:11+00:00 32604 --- {} 9903 1723 eager-load-of-has_one-through-with-conditions-on-the-through-table-fails 0 8994 new :include :through_conditions eager_loading has_one_through 2009-03-26T23:29:15+00:00 51960 Jonathan Monahan Joseph Palermo Frederick Cheung http://rails.lighthouseapp.com/projects/8994/tickets/1723 2.x 4640d9c1e08cae6999e763fa81f86e9cdc68a7e6 text/plain 2009-01-09T20:22:37+00:00 has_one_through_with_conditions_on_through.patch 76970 1918 32604 http://rails.lighthouseapp.com/attachments/76970/has_one_through_with_conditions_on_through.patch