#5488 default_scope doesn't work at a late point - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#5488 ✓resolved

default_scope doesn't work at a late point

Reported by Jan Xie | August 29th, 2010 @ 04:19 AM

I think there may be a problem with this line:

http://github.com/rails/rails/blob/master/activerecord/lib/active_r...

If scoped_methods invoked at an early point, and you use default_scope to (create a new)/(update existing) default scope later, all current_scoped_methods depending finders (like #all) will still use the old default_scope, because the thread cache is not cleared.

I got this problem when I try to update default scope in my plugin, I need to set Thread.current[key] to nil myself whenever I set a new default scope.

Comments and changes to this ticket

Rohit Arondekar August 29th, 2010 @ 12:36 PM
- State changed from “new” to “open”
- Assigned user set to “José Valim”
- Importance changed from “” to “High”
This issue: 4427 must be happening because of this. Although I couldn't reproduce that one in RC 1.

Assigning to Jose as this looks important.
You flagged this item as spam.
Jan Xie August 29th, 2010 @ 01:03 PM
#4427 looks like a different problem.

I guess add a line in default_scope to clear Thread.current[key] will resolve this problem?
José Valim October 11th, 2010 @ 11:19 PM
- Assigned user changed from “José Valim” to “Rohit Arondekar”
Can you guys provide a patch with tests? Rohit?
You flagged this item as spam.
Jan Xie October 12th, 2010 @ 05:45 AM
- Tag changed from “activerecord, default_scope, thread” to “activerecord, default_scope, patch, thread”
Here's a patch with tests.
- fix_default_scope_merge_where_clause.diff 2.7 KB
Rohit Arondekar October 15th, 2010 @ 10:35 AM
- Importance changed from “High” to “Low”
Jan, am I correct in saying that this will have a performance penalty?

Also I've lowered the priority as I had thought it was related to the other ticket. Apologies.
You flagged this item as spam.
Jan Xie October 15th, 2010 @ 11:32 AM
Do you mean reset_scoped_methods in default_scope will cause the following run more times?
```
Thread.current[key] = Thread.current[key].presence || self.default_scoping.dup
```
default_scope only runs when model classes loading, so I think this is not a big problem?

Also correctness should be the first goal? ;)

Rohit Arondekar October 15th, 2010 @ 11:46 AM

One more doubt, this one is more serious though — the test passes without the fix. I commented out the reset_scoped_methods and ran the suite:

(ruby-1.9.2@3-dev)(~/projects/rails/activerecord)९ rake test_sqlite3 TEST=test/cases/base_test.rb 2>/dev/null 
(in /home/rohit/projects/rails/activerecord)
Using native SQLite3
Loaded suite /home/rohit/.rvm/gems/ruby-1.9.2-p0@3-dev/gems/rake-0.8.7/lib/rake/rake_test_loader
Started
............................................................................................................................
Finished in 1.197417 seconds.

124 tests, 305 assertions, 0 failures, 0 errors, 0 skips

Test run options: --seed 15670

Shouldn't the test fail without the fix?

You flagged this item as spam.
Jan Xie October 15th, 2010 @ 11:46 AM
The real problem is default_scope doesn't merge where clauses correctly. Sorry for my inaccurate description in original ticket.
You flagged this item as spam.
Jan Xie October 15th, 2010 @ 11:48 AM
The new test I added is in relation_scoping_test.rb, try with TEST=test/cases/relation_scoping_test.rb?
Rohit Arondekar October 15th, 2010 @ 01:33 PM
- Assigned user changed from “Rohit Arondekar” to “Aaron Patterson”
Sorry! Tried base_test.rb :(

+1 test fails without the fix and the patch applies cleanly.
Repository October 20th, 2010 @ 05:02 PM
- State changed from “open” to “resolved”
(from [f2945401541616c1449799f41a510c8c4f019b8b]) default scope merge where clauses [#5488 state:resolved] http://github.com/rails/rails/commit/f2945401541616c1449799f41a510c...
Repository October 20th, 2010 @ 05:02 PM
(from [21beedf1ff925613fb1ca9b3cf44d10526b64a2e]) default scope merge where clauses [#5488 state:resolved] http://github.com/rails/rails/commit/21beedf1ff925613fb1ca9b3cf44d1...
Jeff Kreeftmeijer November 8th, 2010 @ 08:48 AM
- Tag cleared.
Automatic cleanup of spam.