salsify/goldiloader

Auto eager loading broken for has many through queries that specify select or order

dvandersluis opened this issue · 12 comments

I'm not sure if this is the same bug as #12, but a has many through association that specifies select or order referring to the source table crashes and needs the workaround/fix as well.

For example:

has_many :template_questions, dependent: :delete_all
has_many :questions, through: :template_questions, source: :question,
    select: "questions.*, template_questions.sequence AS sequence"

#=> Mysql2::Error: Unknown column 'template_questions.sequence' in 'field list'

The Rails code that eager loads has_many through associations is unfortunately a bit buggy. Consider the following models:

class Blog < ActiveRecord::Base
  has_many :posts
  has_many :authors, through: :posts
end

class Post < ActiveRecord::Base
  belongs_to :author
  belongs_to :blog
end

class Author < ActiveRecord::Base
  has_many :posts
end

Rails generates the following queries when we use the Blog#authors associations (without Goldiloader in the picture):

> Blog.first.authors.to_a
# SELECT "blogs".* FROM "blogs" LIMIT 1
# SELECT "authors".* FROM "authors" INNER JOIN "posts" ON "authors"."id" = "posts"."author_id" WHERE "posts"."blog_id" = 1

> Blog.includes(:authors).first.authors.to_a
# SELECT "blogs".* FROM "blogs" LIMIT 1
# SELECT "posts".* FROM "posts" WHERE "posts"."blog_id" IN (1)
# SELECT "authors".* FROM "authors" WHERE "authors"."id" IN (1, 2)

In the authors lazy loading case Rails joins in the posts table which means you can select or order by columns from either table. In the eager loading case Rails doesn't generate this join so your options are more limited.

There are a few related Rails bugs to look out for:

  • Eager loading in situations like this doesn't work even if you specify an explicit join on the association - see #11.
  • The context for the order and select in a has_many through association are the "through" association which is not really what you'd expect. See #14 but this means the following:
# Doesn't work with eager loading
has_many :authors, through: :posts, order: 'authors.name'

# Works with eager loading
has_many :authors, through: :posts, order: 'posts.created_at'

As @dvandersluis noted the workaround is to set auto_include = false on this association. Goldiloader could potentially detect this scenario and automatically disable auto eager loading.

+1 For this bug. On the latest version of Goldiloader (0.0.9) for Rails 3.2.12 I get an error when trying to process an :order in a :through:

class Clinic < ActiveRecord::Base
  has_many   :clinic_speaks, :dependent => :destroy
  has_many   :languages, :through => :clinic_speaks, :order => "name ASC"
end

In rails console with this Model, when I do:

@clinic = Clinic.first
@clinic.languages.present?

I get this error:

ClinicSpeak Load (1.3ms) SELECT "clinic_speaks".* FROM "clinic_speaks" WHERE "clinic_speaks"."clinic_id" IN (324) ORDER BY name ASC

_ActiveRecord::StatementInvalid: PG::UndefinedColumn: ERROR: column "name" does not exist_
LINE 1: ... WHERE "clinic_speaks"."clinic_id" IN (3) ORDER BY name ASC
^
: SELECT "clinic_speaks".* FROM "clinic_speaks" WHERE "clinic_speaks"."clinic_id" IN (324) ORDER BY name ASC
from ... /.rbenv/versions/2.1.2/lib/ruby/gems/2.1.0/gems/activerecord-3.2.21/lib/active_record/connection_adapters/postgresql_adapter.rb:1163:in `async_exec'

Originally thought it was my default_scope on Clinic that was affecting it, but it's definitely the order clause. Is there a way for the gem to gracefully default ActiveRecord to :auto_include => false if that's what is needed to fix this?

Ran into a slightly different, but similar variation of this bug that prevents using auto_include: true as a general patch to this issue. In associations looking like this:

  has_many :specializations, :through => :procedure_specializations, :uniq => true, :conditions => { "procedure_specializations.mapped" => true }, :order => 'name ASC', :auto_include => true

connected to a default scope ordering:

  default_scope order('specializations.name')

When using this association, e.g. by running:

    @procedure = Procedure.find(params[:id])
    @specializations = @procedure.specializations
    Specialization.all.each_with_index do |specialization, index|
      @specializations.include?(specialization)
    end

I get this error:

An ActionView::Template::Error occurred in procedures#edit:
PG::InvalidColumnReference: ERROR:  for SELECT DISTINCT, ORDER BY expressions must appear in select list
LINE 1: ...ocedure_specializations"."mapped" = 't') ORDER BY specializa...
                                                             ^
: SELECT  DISTINCT 1 AS one FROM "specializations" INNER JOIN "procedure_specializations" ON "specializations"."id" = "procedure_specializations"."specialization_id" WHERE "procedure_specializations"."procedure_id" = 32 AND "procedure_specializations"."mapped" = 't' AND "specializations"."id" = 23 AND ("procedure_specializations"."mapped" = 't') ORDER BY specializations.name LIMIT 1
  vendor/bundle/ruby/2.1.0/gems/activerecord-3.2.21/lib/active_record/connection_adapters/postgresql_adapter.rb:1163:in `async_exec'

I'll take a look next week to see what we can do here while still supporting has many through associations that eager load properly in the various version of Rails.

In the meantime, one possibility is changing the default for auto_include for has many through associations and only enabling auto_include for has many through associations that are eager loadable. The default can be changed with the following setting:

ActiveRecord::Associations::HasManyThroughAssociation.default_auto_include = false

This bug seems to have been fixed in Rails 4.2+ (assuming explicit joins are added to the necessary tables). Reproducible test case can be found here: https://gist.github.com/jturkel/447257f08372a80e6893. For earlier versions of Rails the potential workarounds are:

  1. Change the auto include behavior for the problematic association by specifying auto_include: false as an association option.
  2. Change the default auto include behavior for has many through associations by setting
    ActiveRecord::Associations::HasManyThroughAssociation.default_auto_include = false
  3. Use the auto_include query option (which I'll add shortly) for default scope cases e.g. order('specializations.name').auto_include(false)

@KelseyDH - I've been thinking about your use case with a default_scope that specifies an order by, can you just specify auto_include: false on your specializations association to workaround the Rails bug?

@jturkel I tried ActiveRecord::Associations::HasManyThroughAssociation.default_auto_include = false and got issue

NoMethodError: undefined method `default_auto_include' for ActiveRecord::Associations::HasManyThroughAssociation:Class

Then got issue SystemStackError: stack level too deep when tried add auto_include to association

has_many :photos, -> { auto_include(false) }, class_name: 'Photo', through: :practice_locations

@kingdomheartdl - The auto_include association option was removed in 2.0 in favor of an ActiveRecord query method that is usable in more situations. See #52 and the upgrade section of the README for more details. Is your stack overflow being caused by #54?

I used uncope to bypass this issue

class Asset < ActiveRecord::Base
end

class Photo < Asset
end

class Location < ActiveRecord::Base
  has_many :photos
end

class People < ActiveRecord::Base
  has_many :locations
  has_many :photos, through: locations
end

We will got issue when includes photo for people People.includes(:photos)

People Load (0.9ms)  SELECT  "peoples".* FROM "peoples"
Location Load (1.2ms)  SELECT "locations".* FROM "locations" WHERE "assets"."type" IN ('Photo') AND "locations"."people_id" IN (512, 910)  ORDER BY "locations"."created_at" ASC, "locations"."id" ASC
PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "assets"
LINE 1: ...locations".* FROM "locations" WHERE "assets"."...
                                                             ^
: SELECT "locations".* FROM "locations" WHERE "assets"."type" IN ('Photo') AND "locations"."people_id" IN (512, 910)  ORDER BY "locations"."created_at" ASC, "locations"."id" ASC
ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "assets"
LINE 1: ...locations".* FROM "locations" WHERE "assets"."...
                                                             ^
: SELECT "locations".* FROM "locations" WHERE "assets"."type" IN ('Photo') AND "locations"."people_id" IN (512, 910)  ORDER BY "locations"."created_at" ASC, "locations"."id" ASC

Use unscope for scope photos model People will help fix it

class People < ActiveRecord::Base
  has_many :locations
  has_many :photos, -> { unscope(where: :type) }, through: locations
end

Now, when execute People.includes(:photos). It's work fine.

People Load (1.7ms)  SELECT  "peoples".* FROM "peoples"
Location Load (0.6ms)  SELECT "locations".* FROM "locations" WHERE "locations"."people_id" IN (512, 910)  ORDER BY "locations"."created_at" ASC, "locations"."id" ASC
Photo Load (0.6ms)  SELECT "assets".* FROM "assets" WHERE "assets"."type" IN ('Photo') AND "assets"."assetable_type" = 'Location' AND "assets"."assetable_id" IN (617, 1097)

But I have another issue:

  • When I load photos first then load locations.
p = People.limit(2)
p.map {p.photos}
p.map {p.locations}

=> It's work fine.

  • When I load locations first then load photos.
p = People.limit(2)
p.map {p.locations}
p.map {p.photos}

=> I got issue SystemStackError: stack level too deep. Here is full description for error

SystemStackError: stack level too deep
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/associations/through_association.rb:6:in `rescue in through_reflection'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activerecord-4.2.3/lib/active_record/associations/through_association.rb:6:in `through_reflection'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:160:in `auto_include?'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:112:in `load_with_auto_include'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/bullet-4.14.7/lib/bullet/active_record42.rb:117:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `block in load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:116:in `load_with_auto_include'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/bullet-4.14.7/lib/bullet/active_record42.rb:117:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `block in load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:116:in `load_with_auto_include'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/bullet-4.14.7/lib/bullet/active_record42.rb:117:in `load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:146:in `block in load_target'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/goldiloader-2.0.1/lib/goldiloader/active_record_patches.rb:116:in `load_with_auto_include'
... 9838 levels...
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:268:in `load'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:268:in `block in load'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:240:in `load_dependency'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/activesupport-4.2.3/lib/active_support/dependencies.rb:268:in `load'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/commands/rails.rb:6:in `call'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/command_wrapper.rb:38:in `call'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:183:in `block in serve'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:156:in `fork'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:156:in `serve'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:131:in `block in run'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:125:in `loop'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application.rb:125:in `run'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/gems/2.2.0/gems/spring-1.3.6/lib/spring/application/boot.rb:18:in `<top (required)>'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
	from /usr/local/rbenv/versions/2.2.1/lib/ruby/2.2.0/rubygems/core_ext/kernel_require.rb:54:in `require'
	from -e:1:in `<main>'

Trying research about it. Many thank if you have a look on it.

I found the issue. The reason is association photos is not loaded but through association locations loaded. So when run function Goldiloader::AssociationPatch.load_with_auto_include, it's call Goldiloader::ThroughAssociationPatch.auto_include? to check through_association.auto_include?. But locations loaded so through_association.auto_include? will return false and make loop forever. Just override it as below

Goldiloader::ThroughAssociationPatch.module_eval do
  def auto_include?
    # Only auto include through associations if the target association is auto-loadable
    through_association = owner.association(through_reflection.name)
    (through_association.loaded? || through_association.auto_include?) && super
  end
end

@HienNguyenAsnet - The issue with the invalid SQL when doing People.includes(:photos) is caused by #12 (which is really a bug in the Rails eager loader). The issue with the SystemStackError is caused by conflicts between goldiloader and bullet. Using bullet 5.6.0 seems to fix the problem but only when using Rails 5+. Assuming you want to stay with Rails 4.2, you should either remove bullet from your project (out of curiosity what's the motivation for having both goldiloader and bullet?) or use the latest goldiloader 1.x release.

This appears to have been fixed in Rails 5.2 (and possibly earlier versions of Rails).