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

Association using unscope: NoMethodError: undefined method `children' for nil:NilClass #38811

Closed
dvandersluis opened this issue Mar 24, 2020 · 8 comments · Fixed by #38929
Closed
Assignees

Comments

@dvandersluis
Copy link
Contributor

Steps to reproduce

Have the following models:

class Post < ApplicationRecord
  has_many :tags, -> { unscope(:where) }
end

class Tag < ApplicationRecord
  default_scope { where(locale: :en) }
end

The following code will crash (even if the default_scope is removed):

Post.joins(:tags).to_sql

Expected behavior

Join tags to posts without the default scope being applied. Should generate a query something like:

SELECT "posts".* FROM "posts" INNER JOIN "tags" ON "tags"."post_id" = "posts"."id"

This was the behaviour in Rails 5.2.3.

Actual behavior

Exception: NoMethodError: undefined method `children' for nil:NilClass
--
 0: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency/join_association.rb:39:in `block in join_constraints'
 1: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency/join_association.rb:30:in `reverse_each'
 2: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency/join_association.rb:30:in `with_index'
 3: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency/join_association.rb:30:in `join_constraints'
 4: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:162:in `make_constraints'
 5: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:155:in `block in make_join_constraints'
 6: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:154:in `each'
 7: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:154:in `flat_map'
 8: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:154:in `make_join_constraints'
 9: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/associations/join_dependency.rb:85:in `join_constraints'
10: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation/query_methods.rb:1179:in `build_join_query'
11: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation/query_methods.rb:1164:in `build_joins'
12: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation/query_methods.rb:1044:in `build_arel'
13: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation/query_methods.rb:1016:in `arel'
14: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:810:in `block in exec_queries'
15: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:828:in `skip_query_cache_if_necessary'
16: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:797:in `exec_queries'
17: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:615:in `load'
18: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:250:in `records'
19: /Users/daniel/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/activerecord-6.0.2.2/lib/active_record/relation.rb:245:in `to_ary'

System configuration

Rails version:
Works in 5.2.3, broken in 5.2.4.2 and 6.0.2.2.

Ruby version:
2.6.2

@kamipo
Copy link
Member

kamipo commented Mar 24, 2020

Can you provide a reproduction script with this template https://github.com/rails/rails/blob/master/guides/bug_report_templates/active_record_master.rb?

@dvandersluis
Copy link
Contributor Author

@kamipo kamipo self-assigned this Mar 24, 2020
@imrubyist
Copy link

imrubyist commented Apr 1, 2020

@kamipo - Is this already fixed? Just run that test and I confirmed it works with ruby 2.6.3p62

Update: that test installed rails 6.1.0.alpha. Sorry. I'll try again.

More update: still unable to reproduce it with this updated test: https://gist.github.com/kevinhq/3c5f9d000d6f3ddc648ba87d35cffc89.

Ruby 2.6.3p62 and Rails 6.0.2.2

@mechanicles
Copy link
Contributor

I also tested this issue but found that it does work perfectly with Ruby version 2.6.0 and Rails versions 5.2.4.2 and 6.0.2.2.

It also works fine with recent Ruby version and Rails version.

@alboyadjian
Copy link

I'm getting the same error after upgrading to 5.2.4.2. Created a reproduction gist based on @kevinhq 's
https://gist.github.com/alboyadjian/189e062cc814b57d8ab1a48e94486d36

@dvandersluis
Copy link
Contributor Author

dvandersluis commented Apr 1, 2020

I tried to track this down and I think it relates to this line: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/query_methods.rb#L479

If unscope is just given a symbol (:where in this case) the entire key is deleted from @values, which seems to mean the join conditions are killed too. Previously (as of 5.2.3), it replaced them with the default values instead, which I think included join conditions.

A workaround I've found is to use the hash form unscope(where: :foo), which is a bit more brittle depending on how often your default scope is being changed but avoids the error.

@alboyadjian
Copy link

Thanks @dvandersluis. Changing to the hash form fixed the tests that were failing because of this. There don't seem to be any problems with unscope when used inside a scope, just when it's used in an association definition.

@imrubyist
Copy link

imrubyist commented Apr 2, 2020

I'm getting the same error after upgrading to 5.2.4.2. Created a reproduction gist based on @kevinhq 's
https://gist.github.com/alboyadjian/189e062cc814b57d8ab1a48e94486d36

Confirmed this.

Error happened on 5.2.4.2 with ruby 2.6.2p47 (2019-03-13 revision 67232) [x86_64-darwin19]

Error:
BugTest#test_association_stuff:
NoMethodError: undefined method `children' for nil:NilClass

kamipo added a commit to kamipo/rails that referenced this issue Apr 15, 2020
rails#29589 changed merging scope order to allow to unscope default scopes on
association scope (rails#29611), but that caused a regression rails#38811 that
accidentally allow join constraint which is required.

```
% bin/test test/cases/associations/has_many_associations_test.rb -n test_unscopes_the_default_scope_of_associated_model_when_used_with_include
Run options: -n test_unscopes_the_default_scope_of_associated_model_when_used_with_include --seed 32978

# Running:

E

Error:
HasManyAssociationsTest#test_unscopes_the_default_scope_of_associated_model_when_used_with_include:
NoMethodError: undefined method `children' for nil:NilClass
    ~/rails/activerecord/lib/active_record/associations/join_dependency/join_association.rb:39:in `block in join_constraints'
    ~/rails/activerecord/lib/active_record/associations/join_dependency/join_association.rb:30:in `reverse_each'
    ~/rails/activerecord/lib/active_record/associations/join_dependency/join_association.rb:30:in `with_index'
    ~/rails/activerecord/lib/active_record/associations/join_dependency/join_association.rb:30:in `join_constraints'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:171:in `make_constraints'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:196:in `block in walk'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:196:in `each'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:196:in `flat_map'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:196:in `walk'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:90:in `block in join_constraints'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:87:in `each'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:87:in `flat_map'
    ~/rails/activerecord/lib/active_record/associations/join_dependency.rb:87:in `join_constraints'
    ~/rails/activerecord/lib/active_record/relation/query_methods.rb:1226:in `build_join_query'
    ~/rails/activerecord/lib/active_record/relation/query_methods.rb:1211:in `build_joins'
    ~/rails/activerecord/lib/active_record/relation/query_methods.rb:1091:in `build_arel'
    ~/rails/activerecord/lib/active_record/relation/query_methods.rb:1063:in `arel'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:419:in `block in limited_ids_for'
    ~/rails/activerecord/lib/active_record/relation.rb:867:in `skip_query_cache_if_necessary'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:419:in `limited_ids_for'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:398:in `apply_join_dependency'
    ~/rails/activerecord/lib/active_record/relation.rb:839:in `block in exec_queries'
    ~/rails/activerecord/lib/active_record/relation.rb:867:in `skip_query_cache_if_necessary'
    ~/rails/activerecord/lib/active_record/relation.rb:834:in `exec_queries'
    ~/rails/activerecord/lib/active_record/relation.rb:639:in `load'
    ~/rails/activerecord/lib/active_record/relation.rb:250:in `records'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:508:in `find_take'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:98:in `take'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:458:in `find_one'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:442:in `find_with_ids'
    ~/rails/activerecord/lib/active_record/relation/finder_methods.rb:69:in `find'
    ~/rails/activerecord/test/cases/associations/has_many_associations_test.rb:2689:in `block in <class:HasManyAssociationsTest>'

bin/test test/cases/associations/has_many_associations_test.rb:2683
```

Required join constraint should not be allowed to unscoping.

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

Successfully merging a pull request may close this issue.

5 participants