This project is archived and is in readonly mode.
Query built wrong for preloading associations nested under polymorphic belongs_to
Reported by Matt D | September 19th, 2009 @ 12:36 AM
Relevant model code:
class Subscription < ActiveRecord::Base
belongs_to :subscribable, :polymorphic => true
end
class Article < ActiveRecord::Base
has_many :notifications, :as => :notifiable, :dependent => :destroy
has_many :subscriptions, :as => :subscribable, :dependent => :destroy
end
class Planner < ActiveRecord::Base
has_many :notifications, :as => :notifiable, :dependent => :destroy
has_many :subscriptions, :as => :subscribable, :dependent => :destroy
end
class Notification < ActiveRecord::Base
belongs_to :notifiable, :polymorphic => true
end
As such, the following sample code acts as expected to get all notifications, but is query-heavy:
all_notifications = Subscription.all.inject([]) { |n, s| n + s.notifications }
However, the following seems ideal for reducing query count, but the queries are incorrect:
some_notifications = Subscription.all(:include => {:subscribable => :notifications}).inject([]) { |n, s| n + s.notifications }
That call returns some notifications, but not all the ones the previous call did. Turns out, with test data of 2 articles and 6 planners, the following queries are run:
Notification Load (0.2ms) SELECT `notifications`.* FROM `notifications` WHERE (`notifications`.`notifiable_id` IN (1,2) and `notifications`.`notifiable_type` = 'Article')
Notification Load (0.2ms) SELECT `notifications`.* FROM `notifications` WHERE (`notifications`.`notifiable_id` IN (1,2,3,4,5,6) and `notifications`.`notifiable_type` = 'Article')
As you can see, the notifiable (polymorphic association) class does not change. I'd be tempted to think I'm doing something wrong, except I can't imagine anything I could do that would cause something like that to happen at the very core of ActiveRecord...
The two things about this setup that look like they might cause an issue are the fact that I'm preloading what's nested under a polymorphic association, and that the "middle-man" classes are polymorphic in both directions (they have a polymorphic relationship with both subscriptions, and notifications).
On Rails version 2.3.4, Ruby 1.8.
Comments and changes to this ticket
-
Maxim Chernyak June 1st, 2010 @ 02:27 AM
- Tag changed from 2.3.4, polymorphic, preload, sql to 2.3.x, patch, polymorphic, preload, sql, test
Here's a patch with a test for this, for rails 2-3-stable. This is probably irrelevant to Rails3.
-
Maxim Chernyak June 12th, 2010 @ 10:54 PM
I should probably clarify that this is a patch with both a test and a fix. Not just test.
-
José Valim June 23rd, 2010 @ 08:09 AM
Applied patch for 2-3-stable. Can someone please investigate if this is an issue in Rails 3 or not?
-
Repository June 23rd, 2010 @ 08:17 AM
- State changed from new to resolved
(from [844da12ba6acfb71a691ba32901d7acf4cbe37b4]) Fix eager loading of polymorphic has_one associations nested-included under polymorphic belongs_to associations. [#3233 state:resolved]
Signed-off-by: José Valim jose.valim@gmail.com
http://github.com/rails/rails/commit/844da12ba6acfb71a691ba32901d7a... -
Neeraj Singh June 23rd, 2010 @ 10:01 PM
@Maxim looks like rails3 is not behaving as expected. Can you verify that?
def self.lab1 puts Subscription.all.inject([]) { |n, s| n + s.subscribable.notifications } end def self.lab2 puts Subscription.all(:include => {:subscribable => :notifications}).inject([]) { |n, s| n + s.subscribable.notifications } end
I am getting three records with lab1 and two records with lab2.
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
Referenced by
- 3233 Query built wrong for preloading associations nested under polymorphic belongs_to (from [844da12ba6acfb71a691ba32901d7acf4cbe37b4]) Fix eag...