Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scope in belongs_to is not respected when used in join context #20679

Closed
kenn opened this issue Jun 24, 2015 · 7 comments
Closed

scope in belongs_to is not respected when used in join context #20679

kenn opened this issue Jun 24, 2015 · 7 comments

Comments

@kenn
Copy link
Contributor

kenn commented Jun 24, 2015

In particular, I pass scope argument to belongs_to to reverse the effect of default_scope in the parent model.

Here, I'm seeing an inconsistent behavior - the scope is respected when it's called on the child instance, but not when it's called on the relation using joins.

Here's models:

class User
  default_scope { where(disabled_at: nil) }
  has_one :profile
end

class Profile
  belongs_to :user, -> { unscope(where: :disabled_at) }
end

Now, given User:1 has disabled_at set:

# Fetches the user correctly
Profile.find(1).user
 => #<User:0x007f830af64728 ...

# Fails to fetch the user!
Profile.joins(:user).merge(User.where(id: 1))
 => []

# default_scope is not removed
Profile.joins(:user).merge(User.where(id: 1)).to_sql
 => "SELECT `profiles`.* FROM `profiles` INNER JOIN `users` ON `users`.`id` = `profiles`.`id` AND `users`.`disabled_at` IS NULL WHERE `users`.`disabled_at` IS NULL AND `users`.`id` = 1"

I think this is fairly confusing, and believe that the scope should be respected with the join relation as well.

Thoughts?

@everton
Copy link

everton commented Jun 25, 2015

I found the same problem with #includes when updating to 4.2.3.rc1 (from 4.2.2) @rafaelfranca

This gist follows the bug report template:

https://gist.github.com/everton/7eed6c00c42712cc0a1f

@everton
Copy link

everton commented Jun 27, 2015

This problem occurs only with "regular" belongs_to, if you create a second "belongs_to" statement like:

class Profile
  belongs_to :user, -> { unscope(where: :disabled_at) }
  belongs_to :user_even_if_deleted, -> { unscope(where: :disabled_at) }, 
    class_name: 'User', foreign_key: :user_id
end

and use this new relation on scope it works.

I tested with stable 4.2.3 and it still with problems.. This seems to me as a regression @rafaelfranca since it was working on 4.2.2.

Anyways, this usage of another belongs_to can be used as a workaround.

@kenn
Copy link
Contributor Author

kenn commented Jun 27, 2015

This problem occurs only with "regular" belongs_to

Not true in my case. I've been using foreign_key and class_name and it still doesn't work.

This seems to me as a regression @rafaelfranca since it was working on 4.2.2.

I'm using 4.2.1 so you seem to have a different problem than mine.

In my case, the crux of the issue is that there are two places that takes default_scope from association - INNER JOIN and WHERE. See:

Profile.joins(:user).merge(User.where(id: 1)).to_sql

=> "SELECT `profiles`.* FROM `profiles`
    INNER JOIN `users` ON `users`.`id` = `profiles`.`id`
      AND `users`.`disabled_at` IS NULL
    WHERE `users`.`disabled_at` IS NULL AND `users`.`id` = 1"

Profile.joins(:user).merge(User.unscoped.where(id: 1)).to_sql

=> "SELECT `profiles`.* FROM `profiles`
    INNER JOIN `users` ON `users`.`id` = `profiles`.`id`
      AND `users`.`disabled_at` IS NULL
    WHERE `users`.`id` = 1"

So, User.unscoped removes default_scope from the WHERE clause, but has no effect on INNER JOIN ... ON clause.

There should be a way to remove default_scope from the INNER JOIN ... ON clause, and to me, it is a bug that the same scope specified is not applied to INNER JOIN ... ON as both INNER JOIN and WHERE are scopes and can be used interchangeably.

At this moment, I had to resort to Arel for a workaround:

class Profile
  def self.joins_unscoped(key)
    source = arel_table
    target = key.to_s.classify.constantize.arel_table
    join = source
      .join(target, Arel::Nodes::InnerJoin)
      .on(source[:id].eq(target[:id])).join_sources
    joins(join)
  end
end

Profile.joins_unscoped(:user).merge(User.unscoped.where(id: 1)).to_sql

=> "SELECT `profiles`.* FROM `profiles`
    INNER JOIN `users` ON `profiles`.`id` = `users`.`id`
    WHERE `users`.`id` = 1"

@rafaelfranca
Copy link
Member

@everton could you open a new issue for your problem? Thanks

@rails-bot
Copy link

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-2-stable, 4-1-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@kenn
Copy link
Contributor Author

kenn commented Jun 14, 2016

I've come up with a robust monkey patch that fixes this issue.

module ActiveRecordExtension
  extend ActiveSupport::Concern

  module ClassMethods
    def joins_unscoped(name)
      reflection = reflections[name.to_s]
      source = arel_table
      target = reflection.class_name.constantize.arel_table
      join = source
        .join(target, Arel::Nodes::InnerJoin)
        .on(source[reflection.foreign_key].eq(target[:id])).join_sources
      joins(join)
    end
  end
end

ActiveRecord::Base.send(:include, ActiveRecordExtension)

Now foreign_key is inferred from association reflection.

> Picture.joins_unscoped(:user).to_sql
=> "SELECT `pictures`.* FROM `pictures` INNER JOIN `users` ON `pictures`.`user_id` = `users`.`id`"

Related issues #13775 and #18109 but it's not quite the same - when I join, it's highly likely to do joins(:user).merge(User.some.scope), and I don't want to wrap it with the User.unscoped { } block, which is confusing AND unnecessarily broad (no finer control in a complex query). I just want to unscope in the context of the particular join, that's it.

Maybe we need a new relation method joins_unscoped that works exactly like the code above?

@tomdracz
Copy link

tomdracz commented Oct 3, 2016

@kenn your PR makes absolute sense. For me when I do:

belongs_to :placement, -> { unscope(where: :archived) }, class_name: "Placements::Placement"

The join should override the default scope (which is archived: false).

The monkey patch works for simple cases, but something like:
joins(placement: {application_container: { programme: :programme_groups }})

Will go kaboom, alas.

Still waiting for a good solution to this one. Using unscoped with a block is not really feasible and using pure sql joins creates issues with non-unique table names, which then need to be aliased, creating massive headache.

kamipo added a commit to kamipo/rails that referenced this issue Jun 28, 2017
Unscoping `default_scope` in associations has already supported (rails#17360
for preloading, c9cf8b8 for eager loading).

Fixes rails#20679.
Closes rails#16531.
kamipo added a commit that referenced this issue Sep 4, 2017
Unscoping `default_scope` in associations has already supported (#17360
for preloading, c9cf8b8 for eager loading).

Fixes #20679.
Closes #16531.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants