collection_singular_ids ignores association :include option #925

Closed
lighthouse-import opened this Issue May 16, 2011 · 3 comments

Comments

Projects
None yet
1 participant

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/6569
Created by rob g - 2011-03-13 20:20:23 UTC

We have a set of models similar to the following:

class User < ActiveRecord::Base
  has_many :taggings, :include => :tag, :order => 'tags.name'
end

class Tagging < ActiveRecord::Base
  belongs_to :user
  belongs_to :tag
end

class Tag < ActiveRecord::Base
  has_many :taggings
end

When attempting to get the set of tagging_ids:

User.first.tagging_ids

we get a StatementInvalid error

     ActiveRecord::StatementInvalid:
       Mysql2::Error: Unknown column 'tags.name' in 'order clause': SELECT `taggings`.id FROM `taggings` WHERE (`taggings`.user_id = 227792459) ORDER BY tags.name

I think I've tracked this to https://github.com/rails/rails/blob/3-0-stable/activerecord/lib/active_record/associations.rb#L1510 :

          redefine_method("#{reflection.name.to_s.singularize}_ids") do
            if send(reflection.name).loaded? || reflection.options[:finder_sql]
              send(reflection.name).map { |r| r.id }
            else
              if reflection.through_reflection && reflection.source_reflection.belongs_to?
                through = reflection.through_reflection
                primary_key = reflection.source_reflection.primary_key_name
                send(through.name).select("DISTINCT #{through.quoted_table_name}.#{primary_key}").map! { |r| r.send(primary_key) }
              else
                send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id } # <===================
              end
            end
          end

I've tried both removing the call to except(:includes) from line 1510 as well as expanding the exceptions to include the association order:

  send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes, :order).map! { |r| r.id } 

and either approach will result in valid SQL. Obviously, the latter approach generates a more performant query but the ids returned will no longer be in the order specified on the association which seems to be a reasonable tradeoff.

Imported from Lighthouse.
Comment by Anatoliy Lysenko - 2011-03-27 13:56:16 UTC

I created simple test for this bug.

Imported from Lighthouse.
Comment by Anatoliy Lysenko - 2011-04-03 11:45:56 UTC

Rob was right,
This commit will revert fix from 3436fdf , but tests is ok.

Attachments saved to Gist: http://gist.github.com/971814

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