Hi guys,

There's two regressions in the 2.1.x series (including current edge)
blocking one of my clients from upgrading from 2.0.

The first one (lighthouse #1101, includes fix) is pretty easy,
whenever you have an association with :conditions => {:some =>
'hash'}, it'll work fine if you use the association without eager
loading, it'll work fine if you use the old eager loading system, but
assuming you fall onto the new preloading path as usual, the
conditions get sanitized against the parent table rather than the
child and so the fully-qualified column names in the SQL have the
wrong table name.

The patch for that's really trivial (diffs for edge on the ticket -
it's also really simple to backport to 2.1, I've attached it for
frozen-in 2.1 rails on the ticket as well for ppl's convenience, let
me know if anyone would like it against rails git for 2.1).

The second bug (lighthouse #1110, testcase but no fix) seems much
messier to fix.  I was having weird problems with records showing up
twice in some of the application's associations when loaded.  It
turned out that you can actually reproduce the problem relatively
simply, if you have say a has_many which is defined with
some :includes on it (Client has_many :projects, :include => :goals),
and you later do a find that does the same include
(Client.find(:all, :include => {:project => {:goals => :tasks}}) or
even just :include => {:projects => :goals} the same as in the
association itself), then the associations get loaded twice - and you
get two of every object in the target array.

Suddenly, all our hours and total invoice prices double... :)

I need some help fixing this one.  Merging duplicate :includes in
AssociationPreload's preload_associations works for the above example
(and the test case on the ticket) but not the general problem, so it
looks to me like checking to see if we've already loaded the
association as we go is probably a better approach.  Doing it that way
for say has_many is pretty easy, you could even just:

--- a/activerecord/lib/active_record/association_preload.rb
+++ b/activerecord/lib/active_record/association_preload.rb
@@ -129,6 +129,7 @@ module ActiveRecord
       end

       def preload_has_many_association(records, reflection,
preload_options={})
+        return if records.first.send(reflection.name).loaded?
         id_to_record_map, ids = construct_id_map(records)
         records.each {|record| record.send(reflection.name).loaded}
         options = reflection.options

But fixing it for has_many :throughs and has_ones is harder.

Thoughts?

Cheers,
Will

--~--~---------~--~----~------------~-------~--~----~
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