[Rails 4] Association Unscoped does not work #10643

Closed
achiinto opened this Issue May 16, 2013 · 18 comments

Comments

Projects
None yet
@achiinto

I am not sure if this is a bug. Back in Rails 3. I believe I could do this...

belongs_to :creator, scope: unscoped, class_name: "User"

But this in Rails 4 does not work...

belongs_to :creator, -> { unscoped }, class_name: "User"

And I had to do it this way...

belongs_to :creator, -> { where is_destroyed: [true, false] }, class_name: "User"

But this is still working...

User.where(is_destroyed: false).unscoped.find 2
# User 2 is_destroyed: true
@rafaelfranca

This comment has been minimized.

Show comment
Hide comment
Member

rafaelfranca commented May 16, 2013

@neerajdotname

This comment has been minimized.

Show comment
Hide comment
@neerajdotname

neerajdotname May 17, 2013

Member

I looked at it. This is what's happening.

In order to get scope this code is executed.

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association.rb#L84

which in turn executes https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association.rb#L125

So the code that gets executed is something like

User.all.merge(User.unscoped)

Now you can see why unscoped is not removing the default_scope because default_scope has already been applied to the target_scope.

Initially I was thinking that in order to fix this issue association_scope should accept target_scope as parameter. But the association_scope does not directly act on it. It adds constraints. https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association_scope.rb#L17-L19

It means unscoped will not be invoked on the passed target. Rather the constraints will be added which does not bring the desired result.

Member

neerajdotname commented May 17, 2013

I looked at it. This is what's happening.

In order to get scope this code is executed.

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association.rb#L84

which in turn executes https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association.rb#L125

So the code that gets executed is something like

User.all.merge(User.unscoped)

Now you can see why unscoped is not removing the default_scope because default_scope has already been applied to the target_scope.

Initially I was thinking that in order to fix this issue association_scope should accept target_scope as parameter. But the association_scope does not directly act on it. It adds constraints. https://github.com/rails/rails/blob/master/activerecord/lib/active_record/associations/association_scope.rb#L17-L19

It means unscoped will not be invoked on the passed target. Rather the constraints will be added which does not bring the desired result.

@neerajdotname

This comment has been minimized.

Show comment
Hide comment
@neerajdotname

neerajdotname May 28, 2013

Member

Spent sometime working on the theory outlined above that instead of merging the relation the block should act on target_relation.

This is where I stand as of now. neerajdotname/rails@master...10643

4 tests are still failing.

It seems that my theory is not going to work because in case of nested has_many the scope_chain needs to act on different on different scopes and then those scopes will have to be merged. So there is no getting around to merging of scopes. And as long as there is merging of scopes, I do not see a way to remove default_scope that has already been applied.

Member

neerajdotname commented May 28, 2013

Spent sometime working on the theory outlined above that instead of merging the relation the block should act on target_relation.

This is where I stand as of now. neerajdotname/rails@master...10643

4 tests are still failing.

It seems that my theory is not going to work because in case of nested has_many the scope_chain needs to act on different on different scopes and then those scopes will have to be merged. So there is no getting around to merging of scopes. And as long as there is merging of scopes, I do not see a way to remove default_scope that has already been applied.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Jun 3, 2013

Member

@neerajdotname, we definitely need a way to remove the entire default scope in a another scope.

@jonleighton, do you have any pointers here?

Member

dhh commented Jun 3, 2013

@neerajdotname, we definitely need a way to remove the entire default scope in a another scope.

@jonleighton, do you have any pointers here?

@ghost ghost assigned jonleighton Jun 3, 2013

@neerajdotname

This comment has been minimized.

Show comment
Hide comment
@neerajdotname

neerajdotname Jun 3, 2013

Member

@jonleighton lemme know if you have any pointers.

In the meantime I'll revisit the four failing tests to see if anything can done to fix them.

Member

neerajdotname commented Jun 3, 2013

@jonleighton lemme know if you have any pointers.

In the meantime I'll revisit the four failing tests to see if anything can done to fix them.

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Jun 7, 2013

Member

I don't really understand how this is a regression. The following is not valid in 3.2:

belongs_to :creator, scope: unscoped, class_name: "User"

The :scope option does not exist.

Could somebody please provide some working code that works in 3.2 but is broken in 4.0?

Member

jonleighton commented Jun 7, 2013

I don't really understand how this is a regression. The following is not valid in 3.2:

belongs_to :creator, scope: unscoped, class_name: "User"

The :scope option does not exist.

Could somebody please provide some working code that works in 3.2 but is broken in 4.0?

@jonleighton

This comment has been minimized.

Show comment
Hide comment
@jonleighton

jonleighton Jun 7, 2013

Member

@dhh I'm removing the milestone on this as I don't think the issue as reported is a regression (see my comment above)

In Campfire you told me of a related issue. You gave the following example:

irb(main):012:0> Project.where(trashed: [ 1, 0]).to_sql
=> "SELECT `projects`.* FROM `projects`  WHERE `projects`.`trashed` IN (1, 0)"
irb(main):013:0> Project.where(trashed: [ 1, 0]).unscope(where: :trashed).to_sql
=> "SELECT `projects`.* FROM `projects`  WHERE `projects`.`trashed` = 0"

I have repro'd with the following test script:

require "active_record"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do
  create_table :projects do |t|
    t.boolean :trashed
  end
end

class Project < ActiveRecord::Base
  default_scope -> { where trashed: false }
  scope :including_trashed, -> { unscoped }
end

puts Project.including_trashed.to_sql        # works as intended
puts Project.unscope(where: :trashed).to_sql # doesn't work
puts Project.except(:where).to_sql           # doesn't work

However, this is not a regression, for a start because unscope doesn't exist in 3.2. However a similar problem does exist in 3.2 with except (as demonstrated above).

Unfortunately this is not something we can really solve in 4.0. It stems from the way that default scopes work - they are only applied at the very end. In 4.0 I deprecated all "eager" forms of scopes, like this:

default_scope where(trashed: false)
scope :trashed, where(trashed: true)

The reason to do that was actually in part to fix this issue. So we can solve it in 4.1 but not in 4.0 since that's where the deprecations are added.

Member

jonleighton commented Jun 7, 2013

@dhh I'm removing the milestone on this as I don't think the issue as reported is a regression (see my comment above)

In Campfire you told me of a related issue. You gave the following example:

irb(main):012:0> Project.where(trashed: [ 1, 0]).to_sql
=> "SELECT `projects`.* FROM `projects`  WHERE `projects`.`trashed` IN (1, 0)"
irb(main):013:0> Project.where(trashed: [ 1, 0]).unscope(where: :trashed).to_sql
=> "SELECT `projects`.* FROM `projects`  WHERE `projects`.`trashed` = 0"

I have repro'd with the following test script:

require "active_record"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do
  create_table :projects do |t|
    t.boolean :trashed
  end
end

class Project < ActiveRecord::Base
  default_scope -> { where trashed: false }
  scope :including_trashed, -> { unscoped }
end

puts Project.including_trashed.to_sql        # works as intended
puts Project.unscope(where: :trashed).to_sql # doesn't work
puts Project.except(:where).to_sql           # doesn't work

However, this is not a regression, for a start because unscope doesn't exist in 3.2. However a similar problem does exist in 3.2 with except (as demonstrated above).

Unfortunately this is not something we can really solve in 4.0. It stems from the way that default scopes work - they are only applied at the very end. In 4.0 I deprecated all "eager" forms of scopes, like this:

default_scope where(trashed: false)
scope :trashed, where(trashed: true)

The reason to do that was actually in part to fix this issue. So we can solve it in 4.1 but not in 4.0 since that's where the deprecations are added.

@dhh

This comment has been minimized.

Show comment
Hide comment
@dhh

dhh Jun 12, 2013

Member

Sounds good about solving in 4.1.

On Jun 7, 2013, at 5:24 PM, Jon Leighton notifications@github.com wrote:

@dhh I'm removing the milestone on this as I don't think the issue as reported is a regression (see my comment above)

In Campfire you told me of a related issue. You gave the following example:

irb(main):012:0> Project.where(trashed: [ 1, 0]).to_sql
=> "SELECT projects.* FROM projects WHERE projects.trashed IN (1, 0)"
irb(main):013:0> Project.where(trashed: [ 1, 0]).unscope(where: :trashed).to_sql
=> "SELECT projects.* FROM projects WHERE projects.trashed = 0"

I have repro'd with the following test script:

require "active_record"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do

create_table :projects do |t|

t.boolean :trashed

end
end

class Project < ActiveRecord::Base

default_scope -> { where trashed: false }

scope :including_trashed, -> { unscoped }
end

puts Project.including_trashed.to_sql # works as intended
puts Project.unscope(where: :trashed).to_sql # doesn't work
puts Project.except(:where).to_sql # doesn't work
However, this is not a regression, for a start because unscope doesn't exist in 3.2. However a similar problem does exist in 3.2 with except (as demonstrated above).

Unfortunately this is not something we can really solve in 4.0. It stems from the way that default scopes work - they are only applied at the very end. In 4.0 I deprecated all "eager" forms of scopes, like this:

default_scope where(trashed: false)
scope :trashed, where(trashed: true)

The reason to do that was actually in part to fix this issue. So we can solve it in 4.1 but not in 4.0 since that's where the deprecations are added.


Reply to this email directly or view it on GitHub.

Member

dhh commented Jun 12, 2013

Sounds good about solving in 4.1.

On Jun 7, 2013, at 5:24 PM, Jon Leighton notifications@github.com wrote:

@dhh I'm removing the milestone on this as I don't think the issue as reported is a regression (see my comment above)

In Campfire you told me of a related issue. You gave the following example:

irb(main):012:0> Project.where(trashed: [ 1, 0]).to_sql
=> "SELECT projects.* FROM projects WHERE projects.trashed IN (1, 0)"
irb(main):013:0> Project.where(trashed: [ 1, 0]).unscope(where: :trashed).to_sql
=> "SELECT projects.* FROM projects WHERE projects.trashed = 0"

I have repro'd with the following test script:

require "active_record"

ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")

ActiveRecord::Schema.define do

create_table :projects do |t|

t.boolean :trashed

end
end

class Project < ActiveRecord::Base

default_scope -> { where trashed: false }

scope :including_trashed, -> { unscoped }
end

puts Project.including_trashed.to_sql # works as intended
puts Project.unscope(where: :trashed).to_sql # doesn't work
puts Project.except(:where).to_sql # doesn't work
However, this is not a regression, for a start because unscope doesn't exist in 3.2. However a similar problem does exist in 3.2 with except (as demonstrated above).

Unfortunately this is not something we can really solve in 4.0. It stems from the way that default scopes work - they are only applied at the very end. In 4.0 I deprecated all "eager" forms of scopes, like this:

default_scope where(trashed: false)
scope :trashed, where(trashed: true)

The reason to do that was actually in part to fix this issue. So we can solve it in 4.1 but not in 4.0 since that's where the deprecations are added.


Reply to this email directly or view it on GitHub.

@davejachimiak

This comment has been minimized.

Show comment
Hide comment
@davejachimiak

davejachimiak Jun 23, 2013

Contributor

@dhh, @jonleighton, between #10966 and #11061, all issues noted here should be fixed.

With both pulls, .unscope and .except should also act on the association's default scope when using them on association scopes, e.g.

class User
  has_many :projects, -> { unscope(:order) }
end
Contributor

davejachimiak commented Jun 23, 2013

@dhh, @jonleighton, between #10966 and #11061, all issues noted here should be fixed.

With both pulls, .unscope and .except should also act on the association's default scope when using them on association scopes, e.g.

class User
  has_many :projects, -> { unscope(:order) }
end
@anazar

This comment has been minimized.

Show comment
Hide comment
@anazar

anazar Jun 27, 2013

Any chance of also making this work when including a polymorphic association? So that all queries there ignore any default scopes.

anazar commented Jun 27, 2013

Any chance of also making this work when including a polymorphic association? So that all queries there ignore any default scopes.

@schneems

This comment has been minimized.

Show comment
Hide comment
@schneems

schneems Jun 27, 2013

Member

Sounds like a variation of the original issue is being worked on for 4.1. Is this the best issue to track the state of that progress?

Member

schneems commented Jun 27, 2013

Sounds like a variation of the original issue is being worked on for 4.1. Is this the best issue to track the state of that progress?

fluxusfrequency added a commit to fluxusfrequency/rails that referenced this issue Dec 4, 2013

Fix ActiveRecord::Relation#unscope
I'm pretty confused about the addition of this method. The documentation
says that it was intended to allow the removal of values from the
default scope (in contrast to #except). However it behaves exactly the
same as except: https://gist.github.com/jonleighton/7537008 (other than
having a slightly enhanced syntax).

The removal of the default scope is allowed by
94924dc, which was not a change we
could make until 4.1 due to the need to deprecate things. However after
that change #unscope still gives us nothing that #except doesn't already
give us.

However there *is* a desire to be able to unscope stuff in a way that
persists across merges, which would allow associations to be defined
which unscope stuff from the default scope of the associated model. E.g.

  has_many :comments, -> { unscope where: :trashed }

So that's what this change implements. I've also corrected the
documentation. I removed the guide references to #except as I think
unscope really supercedes #except now.

While we're here, there's also a potential desire to be able to write
this:

  has_many :comments, -> { unscoped }

However, it doesn't make sense and would not be straightforward to
implement. While with #unscope we're specifying exactly what we want to
be removed from the relation, with "unscoped" we're just saying that we
want it to not have some things which were added earlier on by the
default scope. However in the case of an association, we surely don't
want *all* conditions to be removed, otherwise the above would just
become "SELECT * FROM comments" with no foreign key constraint.

To make the above work, we'd have to somehow tag the relation values
which get added when evaluating the default scope in order to
differentiate them from other relation values. Which is way too much
complexity and therefore not worth it when most use cases can be
satisfied with unscope.

Closes #10643, #11061.
@glampr

This comment has been minimized.

Show comment
Hide comment
@glampr

glampr Jan 5, 2014

Can we make sure that this solution works for JOINs as well?
This is the scenario that is failing for me in AR 4.0.2

class Product
  default_scope deleted_at: nil
end

class OrderItem
  belongs_to  :product
  belongs_to  :unscoped_product, -> { unscoped }, foreign_key: :product_id, class_name: "Product"
end

OrderItem.joins(:unscoped_product).group(:product_id).count

generates this:

SELECT COUNT(*) AS count_all, product_id AS product_id
FROM "order_items"
INNER JOIN "products" ON "products"."id" = "order_items"."product_id"

AND "products"."deleted_at" IS NULL  # This should NOT be here

GROUP BY product_id

glampr commented Jan 5, 2014

Can we make sure that this solution works for JOINs as well?
This is the scenario that is failing for me in AR 4.0.2

class Product
  default_scope deleted_at: nil
end

class OrderItem
  belongs_to  :product
  belongs_to  :unscoped_product, -> { unscoped }, foreign_key: :product_id, class_name: "Product"
end

OrderItem.joins(:unscoped_product).group(:product_id).count

generates this:

SELECT COUNT(*) AS count_all, product_id AS product_id
FROM "order_items"
INNER JOIN "products" ON "products"."id" = "order_items"."product_id"

AND "products"."deleted_at" IS NULL  # This should NOT be here

GROUP BY product_id
@JoshDoody

This comment has been minimized.

Show comment
Hide comment
@JoshDoody

JoshDoody Mar 17, 2014

+1 for @clarkgr - I am having exactly this problem now.

+1 for @clarkgr - I am having exactly this problem now.

@claptimes5

This comment has been minimized.

Show comment
Hide comment
@claptimes5

claptimes5 Jul 15, 2014

Looks like there is an open issue for the joins problem #13775

Looks like there is an open issue for the joins problem #13775

@cwjenkins

This comment has been minimized.

Show comment
Hide comment
@cwjenkins

cwjenkins Aug 17, 2014

@achiinto @jonleighton @dhh The following PR #16531 allows one to remove the default_scope on associations.

@achiinto @jonleighton @dhh The following PR #16531 allows one to remove the default_scope on associations.

@MarkMurphy

This comment has been minimized.

Show comment
Hide comment
@MarkMurphy

MarkMurphy Jan 29, 2015

Can someone summarize the conclusion to this issue thread for me?

Can someone summarize the conclusion to this issue thread for me?

@dimitarvp

This comment has been minimized.

Show comment
Hide comment
@dimitarvp

dimitarvp Feb 18, 2015

This is still present as a behaviour as of Rails 4.2.0.

I have Users that can be marked as deleted, and the User's default scope is deleted_at: nil. Users have associated objects. Fetching any of these objects by ID and referencing their parent User yields nil (obviously, only if the User is marked as deleted). Very counter-intuitive behaviour.

Is this issue with WONTFIX status? If it is, what's the recommended workaround by the Rails devs?

This is still present as a behaviour as of Rails 4.2.0.

I have Users that can be marked as deleted, and the User's default scope is deleted_at: nil. Users have associated objects. Fetching any of these objects by ID and referencing their parent User yields nil (obviously, only if the User is marked as deleted). Very counter-intuitive behaviour.

Is this issue with WONTFIX status? If it is, what's the recommended workaround by the Rails devs?

@cassidyclawson

This comment has been minimized.

Show comment
Hide comment
@cassidyclawson

cassidyclawson Feb 19, 2015

+1

Just wanted to cast another vote that we as Rails devs need a strategy for dealing with "soft deletes" implemented via default_scopes. We often need to access deleted objects via associations.

+1

Just wanted to cast another vote that we as Rails devs need a strategy for dealing with "soft deletes" implemented via default_scopes. We often need to access deleted objects via associations.

@rails rails locked and limited conversation to collaborators Feb 19, 2015

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