Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested eager loading on polymorphic association #8005

Closed
unixcharles opened this issue Oct 21, 2012 · 15 comments
Closed

Nested eager loading on polymorphic association #8005

unixcharles opened this issue Oct 21, 2012 · 15 comments

Comments

@unixcharles
Copy link

The current implementation of the preloader prevent you from eager loading inexistent association.

preloader.rb#L144

def records_by_reflection(association)
  records.group_by do |record|
    reflection = record.class.reflections[association]

    unless reflection || record.class.reflections[association].options[:polymorphic]
      raise ActiveRecord::ConfigurationError, "Association named '#{association}' was not found; " \
                                              "perhaps you misspelled it?"
    end

    reflection
  end
end

Which seem reasonable but then it as the side effect of preventing nested eager loading on certain type of polymorphic association. Keep reading!

Here an exemple involving space exploration and experimentation on animals.

class Vehicule < ActiveRecord::Base
  belongs_to :pilot, :polymorphic => true
end

class Astronaut < ActiveRecord::Base
  has_one :rank
  belongs_to :vehicule
end

class Rank < ActiveRecord::Base
  belongs_to :astronaut
end

class Animal < ActiveRecord::Base
  belongs_to :vehicule
end

buzz = Astronaut.create(:name => 'Buzz Aldrin')
Rank.create(:name => 'Colonel', :astronaut => buzz)

laika = Animal.create(:name => 'Laika')

Vehicule.create(:name => 'Lunar module', :pilot => buzz)
Vehicule.create(:name => 'Sputnik 2', :pilot => laika)

Vehicule.all :include => {:pilot => :rank} # Boom. 
> "Association named 'Rank' was not found; perhaps you misspelled it?"

As you can see, ActiveRecord seem concern that you try to retrieve the army rank of an animal.

This is sad because as a side effect, it prevent you from eager loading the legit rank association of humanoid astronaut.

Given that polymorphic association can point to any model, records that don't have the association could be simply ignored since there is no warranty that the association is not valid for the other records.

I suppose this also apply to STI.

I would suggest be to simply remove that check, filter out records with missing association like this and leave it up to the programmer to not eager load fantasy associations.

Please advice, thanks for your time! 😄

@thegrubbsian
Copy link

I just ran into this today and it is kinda frustrating. There is probably a downside to just removing the error entirely but there should be some mechanism for handling this case in my opinion.

@unixcharles
Copy link
Author

I agree with you @thegrubbsian.

That's why I didn't made it a pull request.

Ideally, when the Preloader know that the record came from a STI/Polymorphic association, it should assume that therefore nested missing relationships should be ignore since they are not predictable.

This look like bit of a changes since at the moment the Preloader is not aware of where the query/parent object.

Meanwhile, I made a small gem that monkey patch the preloader within a block. I hope you find it useful.

@desireco
Copy link

Hey, thanks @unixcharles, I just ran into this issue and your gem seems to be what I need.

@unixcharles
Copy link
Author

Hey @desireco, I'm glad it was helpful for you.

@parndt
Copy link
Contributor

parndt commented Mar 4, 2013

Anyone at rails core: should this turn into a pull request against rails?

@senny
Copy link
Member

senny commented Apr 3, 2013

I'm not quite sure about this one. It would be good to hear what @jonleighton thinks.

@jonleighton
Copy link
Member

I think this could be a reasonable feature, but my enthusiasm will be affected by the complexity of the implementation. If someone wants to take a stab at a patch, go ahead.

@bogardon
Copy link

bogardon commented Oct 6, 2013

any progress here? :(

@jackpipe
Copy link

Me too.
Thank you for the gem @unixcharles, it makes a substantial difference for me.

@drn
Copy link
Contributor

drn commented Nov 4, 2013

+1 - I would really like this to be included in rails as well. For now, monkey-patching it will work for my purposes...

EDIT: Looks like the gem is no longer working with rails 3.2.13. Would love to hear of any other solutions to this. Thanks!

@marcferna
Copy link

+1

1 similar comment
@robertomiranda
Copy link
Contributor

+1

@rafaelfranca
Copy link
Member

I agree with @jonleighton. If someone want to open a pull request and we will discuss the implementation.

I'm closing since this one is a feature request.

seanlinsley added a commit to seanlinsley/rails that referenced this issue Nov 1, 2014
This allows for nested eager loading of polymorphic associations

Resolves rails#8005
@seanlinsley
Copy link
Contributor

Hi everyone, I submitted a PR for this: #17479. Give it a test!

@ttosch
Copy link

ttosch commented Jan 19, 2015

@seanlinsley I love it, does exactly what I need. This has been on my "open issues" list for such a long time. Thanks so much for dealing with it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet