Bug with polymorphic has_one :as pointing to STI record

+1

(from [93c557828e1873004911acfd25d3b3903210bc40]) Fixed bug with polymorphic has_one :as pointing to an STI record

[#2594 Bug with polymorphic has_one :as pointing to STI record state:committed]

Signed-off-by: Jeremy Kemper jeremy@bitsweat.net http://github.com/rails/rails/co...

I didn't see that this breaks existing behavior. No tests fail, unfortunately. Reverting.

(from [ddbeb15a5e7e0c3c5f316ccf65b557bc5311a6c4]) Revert "Fixed bug with polymorphic has_one :as pointing to an STI record"

[#2594 Bug with polymorphic has_one :as pointing to STI record state:open]

This reverts commit 99c103be1165da9c8299bc0977188ecf167e06a5. http://github.com/rails/rails/co...

Darn - i thought that was too easy ;)

Do you have a patch to add some failing test units for me mr. Knowles?

Hi Ruy,

I will try and whip together a test which show-cases this behavior this evening. Regardless of the missing test, switching the defaults is a bit concerning to me as anyone with an existing dataset is left in the cold.

Well from my POV the existing behaviour just seemed like a flat out bug so anyone that actually ran into this issue would have had to work around it in some way. I suspected that it was possible the change would affect cases other then this particular issue, but I figured the unit tests should have caught anything really bad... guess not!

This is related to several other reported issues with STI and polymorphic associations, and they all have a similar issue: expecting that the type field match the real type of the target object. This is not the case in the current code - the type field is used to figure out which table to query, not the type of the result. Note that, in this example, that this will work:

Organization.find(SponsorableOrganization.first.id) # => a SponsorableOrganization object

We are still seeing this happening on version 3.0.3. We have the following code:

Coverage < ActiveRecord::Base
has_one :contact, :as => :contactable end

ContactInfo < ActiveRecord::Base
belongs_to :contactable, :polymorphic => true end

Here's the console output:

irb(main):001:0> Coverage.first.contact
ArgumentError: wrong number of arguments (2 for 1)

    from /usr/lib/ruby/gems/1.8/gems/activerecord-3.0.3/lib/active_record/associations/has_one_association.rb:88:in `find'
    from /usr/lib/ruby/gems/1.8/gems/activerecord-3.0.3/lib/active_record/associations/has_one_association.rb:88:in `find_target'
    from /usr/lib/ruby/gems/1.8/gems/activerecord-3.0.3/lib/active_record/associations/association_proxy.rb:237:in `load_target'
    from /usr/lib/ruby/gems/1.8/gems/activerecord-3.0.3/lib/active_record/associations/association_proxy.rb:118:in `reload'
    from /usr/lib/ruby/gems/1.8/gems/activerecord-3.0.3/lib/active_record/associations.rb:1451:in `contact'
    from (irb):1

Correction. The code we had was:
Coverage < ActiveRecord::Base
has_one :contact, :as => :contactable end
Contact < ActiveRecord::Base
belongs_to :contactable, :polymorphic => true end

This is definitely still broken in Rails 3.

Xac, Rohit Arondekar As i see it is not broken in Rails 3 because to the sponsors table will be added a sponsor with sponsorable_type set to Organization (to the base_class) and so organizations(:sponsorable).sponsor will work correct.

Rails writes the base class to the #{reflection.options[:as]}type column to give an ability to call something like a tagging.posts. Here we need to return all posts of type Post and of type SpecialPost (which is STI from Post). If we will write a class into #{reflection.options[:as]}type instead of class.base_class we won't get posts of class SpecialPost which is not correct behavior.

I think this ticket could be closed for Rails3.

@2kan, I disagree.

Storing the base_class in the type field of a polymorphic association does indeed allow the use case you give, but it does not allow efficient selection of a subset of STI types from a polymorphic association.

For example, extending your example, let's assume we also have @NiceSpecialPost@, NastySpecialPost as additional subclasses of @SpecialPost@, plus GoalPost as an additional subclass of Post. Now consider these use cases:

• you want to get the set of NiceSpecialPosts.
• you want to get the set of SpecialPosts and it's subclasses.
• you want to get the set of GoalPosts and NiceSpecialPosts.

These are all terribly inefficient if the polymorphic type does not hold the subclass.

On the other hand, if the polymorphic type does have the subclass, then queries such as this are possible, e.g. for the second case above:
Tags.find(:all, :include => :posts, :conditions => {:posts_type => [SpecialPosts, SpecialPosts.subclasses]})

If you can do such a query using find then you can generally achieve the same thing in an association.

Apologies if this is all a bit Rails2 - we haven't upgraded yet. I have actually made these changes to our Rails2 installation and all of our polymorphic associations work this way because it enables queries in ActiveRecord that would otherwise be a pain to write.

Although my opinions are based on Rails2, I don't see why this doesn't affect Rails3 - I'd have thought the same use cases could only be solved in the same way in Rails3 too, but the generally approved style and syntax would be different.

I agree with Jonathan. There are non-trivial performance gains by storing the actual STI class in the polymorphic type column ... effectively removing the need to make extra joins.

I've first stumbled onto this "problem" in Rails 1.x and have been applying patches as I've been upgrading Rails. Now I'm looking to upgrade to Rails 3 and oh no ... Rails 3 still stores the base class in polymorphic type columns and no there is no patch :(.

Going from the existing Rails behavior to what's proposed will require a migration to update the polymorphic columns and simple code changes (i.e. instead of parent_type = 'Person' you need parent_type = [Person, Person.subclasses]).

Going the other way also requires a migration but the code changes are potentially much more complex as additional joins have to be worked into the code.

BTW, this seems to be same as #5617 Polymorphic type field stores the base_class of the associated object, not it.... No?

louis vuitton watches