> But I'm curious if you've looked at the two possible patches I've proposed > and which (if either) you would support? >
Just went through your patches and relevant code in AR. And I'd personally go with your 1st patch. I think the current behavior could be very misleading to people who are not aware of AR internals and the results might not be what they expected. Also, I guess allowing conditions on eager loaded association can easily make people write bad code. It's very likely that those conditions deserves a separate association, at least in many cases. For other cases, explicit join should be justified. E.g. in Koz' example : @tickets_from_priority_customers = Ticket.find(:all, :conditions=>["tickets.open = ? and customers.priority = ?", true, true], :include=>[:customers]) I guess it'd make more sense if Ticket model looks has : has_many :customers has_many :priority_customers, :class_name => "Customer", :conditions => "customers.priority = 1" And then the query should like like : @tickets_from_priority_customers = Ticket.find(:all, :conditions=>["tickets.open = ?", true], :include=>[:priority_customers]) That'd be cleaner and more efficient. Having said that, in any case, the patch 2 is a must if most of the people don't agree with patch 1. So my votes are +2/+1 for your 2 patches, in order you uploaded them. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
