This project is archived and is in readonly mode.

#2923 ✓stale
Elliot Winkler

named_scope through a has_many (or has_many through) generates redundant conditions

Reported by Elliot Winkler | July 20th, 2009 @ 07:15 AM | in 3.x

Setup:

class Shop < ActiveRecord::Base
  has_many :dogs
end
class Dog < ActiveRecord::Base
  named_scope :attractive, :conditions => { "color" => %w(yellow black) }
end

Calling shop.dogs.attractive.to_a generates this SQL:

SELECT * FROM "dogs" WHERE ("dogs".shop_id = 1) AND (("dogs"."color" IN ('yellow','black')) AND ("dogs".shop_id = 1))

when it should just be:

SELECT * FROM "dogs" WHERE (("dogs"."color" IN ('yellow','black')) AND ("dogs".shop_id = 1))

The reason this is happening is in part due to #1960 and #2346, but also just a consequence of association proxy magic. What is happening here is essentially this:

Dog.with_scope(:find => {:conditions => {:color => %w(yellow black)}}) do
  Dog.with_scope(:find => {:conditions => {:shop_id => 1}}) do
    Dog.find(:all, :conditions => { :shop_id => 1 })
  end
end

when this should be happening:

Dog.with_scope(:find => {:conditions => {:color => %w(yellow black)}}) do
  Dog.find(:all, :conditions => { :shop_id => 1 })
end

That is, the second scope is being created when it should not be. The key is this line in Scope#method_missing:

if current_scoped_methods_when_defined && !scoped_methods.include?(current_scoped_methods_when_defined)

That seems harmless, right? If the Scope object was created within a scope, apply the scope, unless it's already been applied. Except that here the Scope object wasn't created within a scope, since we went through an association, so the second scope shouldn't be applied, right? And yet it is. What gives?

The culprit is this line in Scope.new:

@current_scoped_methods_when_defined = proxy_scope.send(:current_scoped_methods)

Turns out that since proxy_scope is an association here, when it forwards current_scoped_methods to Dog, it also wraps that in a scope. So this variable ends up storing a scope that will already be applied later.

All that to say, the attached patch+test (tested against 2-3-stable and master) simply fixes AssociationCollection to not wrap calls to current_scoped_methods in a scope. Kind of a hack, but it works...

Comments and changes to this ticket

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

Attachments

Tags

Referenced by

Pages