Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Query built wrong for preloading associations nested under polymorphic belongs_to

#3233

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.

Reported by Matt D · September 19th, 2009 @ 12:36 AM

State: resolved
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Maxim Chernyak
    Maxim Chernyak

    Still happens on 2.3.5 with ruby 1.8

    May 31st, 2010 @ 06:08 AM

  2. Maxim Chernyak
    Maxim Chernyak
    • 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.

    June 1st, 2010 @ 02:27 AM

  3. Maxim Chernyak
    Maxim Chernyak

    I should probably clarify that this is a patch with both a test and a fix. Not just test.

    June 12th, 2010 @ 10:54 PM

  4. José Valim
    José Valim

    Applied patch for 2-3-stable. Can someone please investigate if this is an issue in Rails 3 or not?

    June 23rd, 2010 @ 08:09 AM

  5. Repository
    Repository
    • State changed from new to resolved

    (from [844da12ba6acfb71a691ba32901d7acf4cbe37b4]) Fix eager loading of polymorphic has_one associations nested-included under polymorphic belongs_to associations. [#3233 Query built wrong for preloading associations nested under polymorphic belong... state:resolved]

    Signed-off-by: José Valim jose.valim@gmail.com
    http://github.com/rails/rails/commit/844da12ba6acfb71a691ba32901d7a...

    June 23rd, 2010 @ 08:17 AM

  6. Neeraj Singh
    Neeraj Singh

    @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.

    June 23rd, 2010 @ 10:01 PM