Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

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

#2923

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 Make named_scopes remember the current scope when defined and #2346 named_scope doesn't override default_scope's :order key, 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...

Reported by Elliot Winkler · July 20th, 2009 @ 07:15 AM

State: stale
Milestone: 3.x
Assigned to: nobody
Importance: none

Activity

  1. Elliot Winkler
  2. Elliot Winkler
    Elliot Winkler
    • Tag changed from named_scope to named_scope, patch

    July 21st, 2009 @ 03:24 AM

  3. Reto
    Reto

    Had the same problem with the following constellation. The patch fixed it as well.

    class Employment < ActiveRecord::Base
      belongs_to :user
    
      has_many :silblings,
        :primary_key => :user_id,
        :foreign_key => :user_id,
        :class_name => 'Employment'
    end
    
    class User
      has_many :employments
    end
    

    (see also http://stackoverflow.com/questions/1623743/silblings-in-hasmany-rel...)

    October 26th, 2009 @ 09:24 AM

  4. Jon Druse
    Jon Druse

    I'm so glad that someone found this issue. Any word on when this will be fixed?

    November 6th, 2009 @ 10:21 PM

  5. Jeremy Kemper
    Jeremy Kemper
    • Milestone changed from 2.x to 3.x

    May 4th, 2010 @ 06:48 PM

  6. Rohit Arondekar
    Rohit Arondekar
    • State changed from new to stale
    • Importance changed from to

    Marking ticket as stale. If this is still an issue please leave a comment with suggested changes, creating a patch with tests, rebasing an existing patch or just confirming the issue on a latest release or master/branches.

    October 9th, 2010 @ 04:02 AM