On 9/13/07, Trevor Squires <[EMAIL PROTECTED]> wrote:
>
>
> Thanks for the reply, I believe I understand now.
>
> So the issue is that once it decides that *any* of the :include
> tables are referenced in :conditions it joins *all* of them - even
> though it's possible that some of the includes are not referenced.
>
> Personally I've not been bitten by performance issues with the
> prequery but if your non-destructive-patch improves that situation
> I'd say it was a good idea.


But what do you think of the destructive patch?  In your example statement
below I don't like the fact that the conditions string is used both to A)
find albums with at least one errored song upload and B) fetch only the
errored uploads.

If you just wanted A then you have to do an explicit join.  If you wanted B
you would be better off defining a custom association with that condition
directly attached.  With the existing code A&B are conflated, which I think
greatly diminishes the utility of adding eager joins to the pre-query.

Is your patch intelligent enough to cope with this?:
>
> Album.find(:all, :limit => 10, :include => [{:songs
> => :uploads}, :artists],
>    :conditions => 'uploads.errored = 0')


Yes, it is just comparing the join string when the query is compiles,
doesn't matter what kind of nested includes you use.

where the prequery joins songs and uploads but not artists?
>
> I bring it up because that strikes me as a reason why it just does
> all of the joins - bit of a pain to tease apart exactly which joins
> are absolutely required (not impossible - just a pain).
>

Check the patch, it's actually quite simple.  The include_eager_order? and
include_eager_conditions? methods already parse their respective strings for
table names, so I just refactored those methods to capture the table names
in arrays.

Then I add one reject method in the method chain that adds the joins.
 Essentially there is only half a line of new code required to do this.  But
again, this patch is my second choice.  I'm trying to drum up support for
the first patch.

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at 
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to