This project is archived and is in readonly mode.
Single Table Inheritance, Polymorphic association (self-referential and normal) bug
Reported by humanzz | May 2nd, 2009 @ 02:34 PM | in 2.x
I used single table inheritance and self-referential polymorphic association in some models and the wrong sql was generated. I have
class Feeling < ActiveRecord::Base
belongs_to :feelable, :polymorphic => true
belongs_to :feeler, :polymorphic => true
end
class PersonalFeeling < Feeling
has_many :feeling_comments, :as => :feelable, :dependent => :destroy
end
class FeelingComment < Feeling
end
and 'feelings' table in the database which has the type attribute. STI works just fine in the sense that if I create a Feeling type is set to Feeling, PersonalFeeling type is set to PersonalFeeling and so on.
If I run the following code
pf = PersonalFeeling.first
comments = pf.feeling_comments
The generated SQL is
SELECT * FROM `feelings` WHERE (`feelings`.feelable_id = 14 AND `feelings`.feelable_type = 'Feeling') AND ( (`feelings`.`type` = 'FeelingComment' ) )
which is not correct. The feelings
.feelable_type =
'Feeling' should have been feelings
.feelable_type =
'PersonalFeeling'.
I tracked the issue to the lib\active_record\associations\has_many_association.rb on line 91 in the construct_sql method. The issue is that the line "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" is trying setting the type to the base_class which is not correct in this case. I don't know why the behavior of using the base_class instead of the @owner's class itself.
After finding that cause, I also tried the case with non self-referential and the bug still exists.
Comments and changes to this ticket
-
Iain Adams May 8th, 2009 @ 05:49 PM
Am I right in thinking that the API documentation is wrong on this issue as well.
It claims that having a polymorphic_type=(sType) method should work, but it won't work for me.
From the API......
class Asset < ActiveRecord::Base
belongs_to :attachable, :polymorphic => true def attachable_type=(sType) raise "Setting type" end
end
class Post < ActiveRecord::Base
# because we store "Post" in attachable_type now :dependent => :destroy will work has_many :assets, :as => :attachable, :dependent => :destroy
end
class GuestPost < Post end
class MemberPost < Post end
The method attachable_type=(sType) is never even used otherwise I would get an error....right?
-
Matt Jones June 8th, 2009 @ 09:19 PM
A couple things:
-
does the "bad SQL" get the right answer?
-
in this case, you don't actually need a polymorphic association. If the models you're associating are all STI subclasses, you can just use regular has_many and friends. The polymorphic stuff comes in where you have objects in the association that aren't even part of the same DB table.
-
-
PommyTom November 19th, 2009 @ 03:10 AM
I'm also getting this error. For the exact same reasoning as the poster.
Forcibly overriding the _type field on the relationship to still have access to the has_many/named_scopes
-
Matt Jones November 19th, 2009 @ 09:34 PM
- State changed from new to wontfix
This is not a bug - the _type field essentially points to the base class. Note that the two statements below are equivalent:
Feeling.find(14) # => PersonalFeeling id 14 PersonalFeeling.find(14) # => PersonalFeeling id 14
Closing for now - if anyone can write a test where the existing code returns incorrect results (in other words, finds the wrong records) we can re-examine this.
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
Referenced by
- 2756 [patch] The polymorphic type field should be subclass name instead of base class name when using polymorphic association with single table inheritance I prepared a patch to resolve these situations. This patc...