51

I am using polymorphic associations to track Comments in my project. All very straight forward stuff.

The problem I have is in querying based on the polymorphic association and joining from the Comment model back to it's owner.

So ...

I have a Comment model

class Comment < ActiveRecord::Base
  belongs_to :commentable, :polymorphic => true
end

And a ForumTopics mode:

class ForumTopic < ActiveRecord::Base
  has_many :comments, :as => :commentable
end

I have several other "commentable" models that aren't important right now. All of this works.

What I am trying to do is find all of the Comments that belong to a ForumTopic with a specified condition (in this case, 'featured' == true).

When I try and use a finder to join the models:

@comments = Comment.find(:all 
            :joins => :commentable
            :conditions => ["forum_topics.featured = ? ", true] 
            )

I receive the following error:

Can not eagerly load the polymorphic association :commentable

Using the AR "include syntax":

@comments = Comment.find(:all 
            :include => :forum_topics
            :conditions => ["forum_topics.featured = ? ", true] 
            )

returns:

Association named 'forum_topics' was not found; perhaps you misspelled it?

If I try and join with a table name instead of the association name (string instead of symbol):

@comments = Comment.find(:all,
            :joins => "forum_topics",
            :conditions => ["forum_topics.featured = ? ", true] 
            )

I see:

Mysql::Error: Unknown table 'comments': SELECT comments. FROM comments forum_topics WHERE (forum_topics.featured = 1 )*

(You can see here that the syntax of the underlying query is totally off and the join is missing altogether).

Not sure if what I am doing is even possible, and there are other ways to achieve the required result but it seems like it should be doable.

Any ideas? Anything I am missing?

Undistraction
  • 42,754
  • 56
  • 195
  • 331
Toby Hede
  • 36,755
  • 28
  • 133
  • 162
  • Oh, and I realise the conditions don't make so much sense ... but this was all originally in named_scopes and being passed parameters – Toby Hede Mar 25 '09 at 03:45

8 Answers8

36

Argh!

I think I found the problem.

When joining via:

@comments = Comment.find(:all,
        :joins => "forum_topics",
        :conditions => ["forum_topics.featured = ? ", true] 
        )

You need the whole join!

:joins => "INNER JOIN forum_topics ON forum_topics.id = comments.commentable_id",

See the ever-awesome: http://guides.rubyonrails.org/active_record_querying.html#joining-tables

Toby Hede
  • 36,755
  • 28
  • 133
  • 162
  • 5
    Don't you hate it when the error is so mind-numblingly simple? – Toby Hede Mar 25 '09 at 04:07
  • 1
    At least you posted the solution. Kudos. – srboisvert Mar 25 '09 at 13:00
  • 31
    I think you also need to check that `comments.commentable_type` is the right type (in your case `"ForumTopic"`) otherwise you are doing a join with ALL the `comments.commentable_id` entries matching `forum_topics.id` even when they aren't forum topics. – Jits Jul 12 '11 at 10:07
  • 1
    @Jits can you clarify how you would alter the query to check for that? – yoshyosh Nov 15 '13 at 01:27
  • 2
    Look at the answer by Chris barnes below for clarification on Jit's comment. Basically you would add a 'AND comments.commentable_type = "ForumTopic"' – yoshyosh Nov 15 '13 at 01:36
  • 1
    This is not the correct answer. Watchout it will fail as soon as a comment is added to another Commentable polymorphic class that matches the same ID as a forum_topic – tommyalvarez Jul 12 '17 at 16:49
35

An old question, but there is a cleaner way of achieving this by setting up a direct association for the specific type along with the polymorphic:

#comment.rb
class Comment < ActiveRecord::Base

  belongs_to :commentable, polymorphic: true
  belongs_to :forum_topics, -> { where( comments: { commentable_type: 'ForumTopic' } ).includes( :comments ) }, foreign_key: 'commentable_id'

  ...

end

You are then able to pass :forum_topics to includes getting rid of the need for a messy join:

@comments = Comment
  .includes( :forum_topics )
  .where( :forum_topics => { featured: true } )

You could then further clean this up by moving the query into a scope:

#comment.rb
class Comment < ActiveRecord::Base

  ...

  scope :featured_topics, -> { 
    includes( :forum_topics )
    .where( :forum_topics => { featured: true } ) 
  }

  ...

end

Leaving you to be able to simply do

@comments = Comment.featured_topics
Sam Peacey
  • 5,964
  • 1
  • 25
  • 27
  • I tried exactly what you said, and it didn't work for me. https://gist.github.com/aruprakshit/edf202e12b073df838b0 . :/ No idea why. – Arup Rakshit Jan 15 '16 at 08:03
  • 3
    @ArupRakshit, have you tried declaring your specific association as :review instead of :reviews? I think "belongs_to" should always be used with a singular relation name. – Seth Kingsley Mar 22 '16 at 05:32
  • 1
    Scopes on the polymorphic model is definitely the cleanest way to go as it's DRY - the scopes can be used in controllers, views, etc while only being defined once in the model. In fact, that's probably why scopes were introduced in the first place! – Chris Cirefice May 25 '16 at 17:46
  • I've just tried this on a Rails 5 project, and it does work, however I had to change the `includes(:forum_topics)` to `joins(:forum_topics)`, I think joins would have been better in the original answer to be honest. What problem are you running into? – Sam Peacey Jul 21 '17 at 00:46
  • What is the purpose of the join within the association scope? I don't see how this would return nil for a comment that actually does not belong to a forum topic. As a side note, it's somehow fundamentally wrong to declare a polymorphic association and then try to establish a direct connection to another model. This connection may not exist, that's why the polymorphic association is there in the first place. – petkov.np Aug 09 '17 at 09:15
  • 1
    It doesn't return nil for a comment not belonging to a forum topic, it simply doesn't return any comments not belonging to a forum topic. The join (the one back to comments I assume you mean?) is because the association is on forum_topics, it doesn't include comments by default. As for the legitimacy, the polymorphic relationship is saying the comment can belong to any model, but the association is just a convenience to say "give me all the forum topic comments". – Sam Peacey Aug 10 '17 at 12:21
  • To be fair, a polymorphic relationship is just a convenience and probably not 'best practice' as you can't enforce referential integrity at a database level. If you want to go for best practice I'd say you're better off with an intersection table (per associated model). – Sam Peacey Aug 10 '17 at 12:24
  • If this helps, it works for me in Rails 6, and I do not need to change to joins(:forum_topics). However, I have to use singular form `belongs_to :forum_topic, ...` – Linh Dam Oct 02 '19 at 06:00
23

You Need a Conditional, Plus Rails 3+

A lot of people alluded to it in the answers and comments but I felt that people, including myself, would get tripped up if they landed here and didn't read thoroughly enough.

So, here's the proper answer, including the conditional that is absolutely necessary.

@comments = Comment.joins( "INNER JOIN forum_topics ON comments.commentable_id = forum_topics.id" )
                   .where( comments:     { commentable_type: 'ForumTopic' } )
                   .where( forum_topics: { featured:         true         } )

Thanks to all, especially @Jits, @Peter, and @prograils for their comments.

Joshua Pinter
  • 45,245
  • 23
  • 243
  • 245
  • If you joins a `forum_topics`, why the `commentable_type` matters since it is the `forum_topics` type like you sad on `inner join`? – Washington Botelho Oct 18 '16 at 03:09
  • 2
    @WashingtonBotelho It matters because `Comment` is polymorphic and that `INNER JOIN` is only comparing the `id` of the `Comment` model. So, if `Comment` belongs to multiple models, like `ForumPost`, then that `INNER JOIN` will mistakenly include `Comment` records that belong to `ForumPost` as well. – Joshua Pinter Oct 18 '16 at 13:17
  • 1
    This is by far the most informative answer on here and saved me hours of headache joining across multiple polymorphic associations. Thank you! – Travis May 18 '17 at 17:56
  • @Travis Thanks for the comment, Travis! Happy that it saved you time, that's what it's all about! :) – Joshua Pinter May 18 '17 at 20:56
  • Doesn't work at least under Rails 5: no backticks are allowed here. – prograils Jul 20 '17 at 13:30
  • @prograils Our company isn't on Rails 5 yet. if you remove the back ticks in the SQL does it work? – Joshua Pinter Jul 20 '17 at 20:19
  • @Josh Pinter Yes it does. See http://guides.rubyonrails.org/active_record_querying.html#joining-tables – prograils Jul 21 '17 at 06:58
  • @prograils I've updated the answer to include a section for Rails 5+ without the back ticks. Thanks for the tip! – Joshua Pinter Jul 21 '17 at 15:41
  • 1
    Really good explanation and great answer on the topic! Thanks! – Mathieu Rios Nov 04 '20 at 13:08
22

Checked to work under Rails 5:

Solution 1:

@comments = Comment
              .where(commentable_type: "ForumTopic")
              .joins("INNER JOIN forum_topics ON comments.commentable_id = forum_topics.id")
              .where(forum_topics: {featured: true})
              .all

or

Solution 2:

@comments = Comment
              .joins("INNER JOIN forum_topics ON comments.commentable_id = forum_topics.id AND comments.commentable_type = 'ForumTopic'")
              .where(forum_topics: {featured: true}).all

Pay attention to the raw SQL syntax: no backticks are allowed. See http://guides.rubyonrails.org/active_record_querying.html#joining-tables .

I personally prefer Solution 1 as it contains fewer raw SQL syntax.

zishe
  • 10,665
  • 12
  • 64
  • 103
prograils
  • 2,248
  • 1
  • 28
  • 45
17

The accepted solution does not work once you introduce another model that has an association using "commentable". commentable_id is not unique and therefore you'll start retrieving the wrong comments.

For example:

You decide to add a news model that accepts comments...

class News < ActiveRecord::Base
   has_many :comments, :as => :commentable
end

Now you may get two records back if you made a comment on a forum_topic with an id of 1 and a news article with an id of 1 using your query:

:joins => "INNER JOIN forum_topics ON forum_topics.id = comments.commentable_id"

You could probably solve the problem by supplying a commentable_type as one of your conditions, but I don't think that's the best way to approach this issue.

  • 2
    Actually I think you're suggestion of supplying a commentable_type as one of the conditions is the right way to go... the [`act_as_taggable_on`](https://github.com/mbleigh/acts-as-taggable-on) plugin does exactly that with their `taggings` table containing (amongst others) the columns: `tag_id`, `taggable_id` **and `taggable_type`** – AJP Aug 23 '11 at 01:43
  • This absolutely an astute observation. I've added an answer that prevents people from making this mistake like I did. – Joshua Pinter Apr 10 '14 at 17:45
10

I came across this post and it lead me to my solution. Using the commentable_type as one of my conditions but using a LEFT OUTER JOIN instead. That way forum topics without comments will be included.

LEFT OUTER JOIN `comments` ON `comments`.`commentable_id` = `forum_topics`.`id` AND `comments`.`commentable_type` = 'ForumTopic'
Chris Barnes
  • 143
  • 1
  • 9
  • Doesn't work at least under Rails 5: no backticks are allowed here. – prograils Jul 20 '17 at 13:51
  • For the benefit of anyone finding the old above comment, the backticks are a MySQL thing, not a Rails thing. ANSI standard SQL would use double quotes, but in this case no quoting of the table/column names should be necessary, so you could just remove the backticks. – Wodin Sep 26 '19 at 06:11
3

Here is an example to adapt to your purpose.

class AdwordsCampaign < ApplicationRecord
  has_many :daily_campaign_reports, as: :ga_campaign
end

class DailyCampaignReport < ApplicationRecord

  belongs_to :ga_campaign, polymorphic: true

  # scope
  def self.joins_ga_campaign(model)
    models = model.to_s.pluralize
    sql = <<~SQL
      INNER JOIN #{models}
      ON #{models}.id = daily_campaign_reports.ga_campaign_id
    SQL
    joins(sql).where(ga_campaign_type: model.to_s.camelize)
  end
end

Then I can use it wherever needed like this:

DailyCampaignReport.joins_ga_campaign(:adwords_campaign)
Max Press
  • 111
  • 1
  • 6
0

Rails does not include a polymorphic join by default but this gem would help you to joins your polymorphic relationship with ease. https://github.com/jameshuynh/polymorphic_join

Damian Sia
  • 479
  • 1
  • 4
  • 14