This project is archived and is in readonly mode.

#125 ✓resolved
Denis Barushev

Eager loading load association for every STI subclass when it is declared in superclass

Reported by Denis Barushev | May 6th, 2008 @ 07:10 PM

When association is declared in superclass eager loading of this association produces one query for each subclass. Example:

Update edge Rails and generate empty project: 

cd ~/dev/ruby/rails/
git pull
ruby railties/bin/rails ~/dev/test/eager
ln -s ~/dev/ruby/rails ~/dev/test/eager/vendor
cd ~/dev/test/eager/

Generate models: 

./script/generate model User name:string
./script/generate model Asset type:string user_id:integer

rake db:migrate

app/models: 

class User < ActiveRecord::Base
  has_many :assets
end

class Asset < ActiveRecord::Base
  belongs_to :user
end

class Image < Asset
end

class Audio < Asset
end

class Video < Asset
end

Console session:

Prepare data: 

>> User.create
>> User.create
>> Image.create :user => User.first
>> Image.create :user => User.last
>> Audio.create :user => User.first
>> Video.create :user => User.first

Find all Assets with user preloading: 

>> Asset.find :all, :include => :user

Produces queries: 

  Asset Load (0.002034)   SELECT * FROM "assets"
  User Load (0.001601)   SELECT * FROM "users" WHERE ("users".id IN ('6','5'))
  User Load (0.001147)   SELECT * FROM "users" WHERE ("users".id IN ('5'))
  User Load (0.001194)   SELECT * FROM "users" WHERE ("users".id IN ('5'))

I think when association is declared in superclass it must be only one query to preload it, not one for each subclass. Such as: 

  Asset Load (0.002034)   SELECT * FROM "assets"
  User Load (0.001601)   SELECT * FROM "users" WHERE ("users".id IN ('6','5'))

Comments and changes to this ticket

  • Frederick Cheung

    Frederick Cheung May 6th, 2008 @ 10:51 PM

    Well [http://dev.rubyonrails.org/ticke...][this ticket] is why the current behaviour is what it is. I can't think off the top of my head whether that the ticket and what you've noticed can easily be reconciled.

    Whatever it is, the fix is likely to be in preload_one_association where it does records.group_by(&:class); if it can be determined when this partitoning is not necessary and fall back.

  • Frederick Cheung

    Frederick Cheung May 7th, 2008 @ 12:16 AM

    I believe this does the trick. Would be good for those bitten by this to try it out

  • Frederick Cheung

    Frederick Cheung May 7th, 2008 @ 12:30 AM

    • no changes were found...
  • Denis Barushev

    Denis Barushev May 7th, 2008 @ 10:41 AM

    • no changes were found...
  • Tarmo Tänav

    Tarmo Tänav May 7th, 2008 @ 08:23 PM

    There seems to be an unused "class_to_reflection = {}" there, planning to cache the reflection for each cache?

  • Frederick Cheung

    Frederick Cheung May 7th, 2008 @ 12:30 AM

    D'oh, somehow manager to produce a patch with not the final version (and mix that in with a patch from another ticket I'd applied locally to test - no idea how that happened!). I think I have untangled my git-tardedness now

  • Denis Barushev

    Denis Barushev May 7th, 2008 @ 08:22 PM

    • Title changed from “Eager loading load association for every STI subclass when it is declared in superclass” to “[PATH] Eager loading load association for every STI subclass when it is declared in superclass”

    I'm not good familiar with Rails sources, but I fixed this problem. See path.

    Manual test: 

    class Asset < ActiveRecord::Base
  belongs_to :user
end

class Image < Asset
end

class Audio < Asset
end

class Video < Asset
end


class Asset1 < ActiveRecord::Base
  belongs_to :user
end

class Image1 < Asset1
end

class Audio1 < Asset1
  belongs_to :user
end

class Video1 < Asset1
end

    >> Asset.destroy_all
>> Asset1.destroy_all

>> Audio.create :user => User.first
>> Audio.create :user => User.last
>> Video.create :user => User.first
>> Image.create :user => User.first

>> Audio1.create :user => User.first
>> Audio1.create :user => User.last
>> Video1.create :user => User.first
>> Image1.create :user => User.first

>> Asset.all :include => :user
>> Asset1.all :include => :user

    SQL-queries: 

      Asset Load (0.003225)   SELECT * FROM "assets"
  User Load (0.001348)   SELECT * FROM "users" WHERE ("users".id IN ('1','2'))

  Asset1 Load (0.001352)   SELECT * FROM "asset1s"
  User Load (0.000919)   SELECT * FROM "users" WHERE ("users".id IN ('1','2'))
  User Load (0.000703)   SELECT * FROM "users" WHERE ("users".id IN ('1'))

    How test this behavior in Test::Unit?

  • Frederick Cheung

    Frederick Cheung May 7th, 2008 @ 10:51 AM

    See the patch I attached :-)

  • Denis Barushev

    Denis Barushev May 7th, 2008 @ 08:23 PM

    • Title changed from “Eager loading load association for every STI subclass when it is declared in superclass” to “[PATH] Eager loading load association for every STI subclass when it is declared in superclass”

    Oops, I haven't seen your patches. Lighthouse doesn't have intuitive interface.

    Sorry :)

  • Repository

    Repository May 11th, 2008 @ 10:16 PM

    • State changed from “new” to “resolved”
    • Title changed from “[PATH] Eager loading load association for every STI subclass when it is declared in superclass” to “Eager loading load association for every STI subclass when it is declared in superclass”

    (from [236f0bb67adecbc1e6dac5258e4a8cb310ffd7a4]) When preloading group by reflection rather than by class [#125 state:resolved]

    This avoids extra queries when several subclasses inherit the association

    from their parent class, while still coping with subclasses redefining

    associations.

    Signed-off-by: Pratik Naik

    http://github.com/rails/rails/co...

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Attachments

Tags

Referenced by

Pages