Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• 

  Andrew White June 28th, 2008 @ 03:50 PM

  On the surface this appears to fix things but what you have to remember is that the eager loading may be for more than one parent so if you just limit the query the result will be incorrect unless the associated records are the same for each parent.

  For example if you do a Category.find(:all, :include => :most_recent_movies) then limiting the query will not give you want you want. The only way to fix it is to discard any extra records after they have been preloaded.

  However in your example there may be irregular distribution of movies between categories so I wouldn't recommend eager loading - you may end up loading all of the movie records just to fill the sparser categories.

  The best way to proceed I think would be to patch the preloading code so that extra records are discarded and add a warning in the documentation for eager loading of limited associations.

• 

  Frederick Cheung June 28th, 2008 @ 04:29 PM

  Eager loading and limits on the association don't really match. As Andrew says, your patch will break most of the time when you're loading (in your example) more than one category.

  Limiting after load is a bit messy (and sort of creates the illusion that we can eager load such associations efficiently). We already do that for has_one though. If you do go down that road, add_preloaded_records_to_collection is probably the place to enforce that.

• You flagged this item as spam.
  

  MatthewRudy June 29th, 2008 @ 01:21 AM

  and then one could argue the following;

  class A
    has_many :bs, :limit => 2
  end
  
  >> a.bs
  => [#< B:1 >, #<B:2 >]
  >> a.bs.create()
  (irb):10:in `irb_binding': limit of association :bs exceeded (HasManyLimitException)
  
  or
  
  >> a.bs.create()
  => #< B:new >
  >> a.bs
  => [#< B:2 >, #< B:new >]
  

  but then which object do you push off...

  and what if you actually wanted to maintain a limit, but allowed a create, so it'd be a hard remove of one of them...

  I dunno...

  just drunkenly (without drinking anything 'cept tea) imagining the craziness that could be involved!

• 

  Pratik July 12th, 2008 @ 02:22 PM

  • State changed from “new” to “wontfix”

  What everyone else said.

• You flagged this item as spam.
  

  Luca Guidi July 12th, 2008 @ 02:37 PM

  I tried to figure out how to fix it, but an unnecessary complexity would be added to the code.

  I agree to wontfix status.

  It would be nice to specify this behavior into the documentation, in order to avoid unexpected results.

• 

  Pratik July 12th, 2008 @ 02:52 PM

  Hey jodosha,

  If you message me (lifo) at github, I'll add you to docrails. So that you can commit the docpatch yourself anytime you wish.

  Thanks.

• You flagged this item as spam.
  

  Luca Guidi November 3rd, 2008 @ 09:19 AM

  This is just a reminder: the case was documented in http://github.com/lifo/docrails/...