#2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed - 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.

#2791 ✓duplicate

ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed

Reported by Valentin Mihov | June 11th, 2009 @ 03:32 PM | in 2.x

When you use find_in_batches it turns out that there is a with_scope that gets executed, which could lead to unexpected behavior. See the attached unit test for an example. The idea of the test is to get all posts with body "hello" in batches of 2 and for each post get its author and count the number of posts he/she has.

The easiest fix is to put with_exclusive_scope around the yield, but that could affect people who used with_scope around find_in_batches. The harder fix will be not to use with_scope.

Comments and changes to this ticket

Valentin Mihov June 11th, 2009 @ 03:34 PM
- no changes were found...
- 0002-Added-with_exclusive_scope-around-the-find_in_batche.patch 1019 Bytes
You flagged this item as spam.
Valentin Mihov June 11th, 2009 @ 03:34 PM
Here is the fix with adding with_exclusive_scope around the yield statement
Valentin Mihov June 11th, 2009 @ 05:15 PM
- no changes were found...
- removed_with_scope_around_yield.patch 3.9 KB
You flagged this item as spam.
Valentin Mihov June 11th, 2009 @ 05:15 PM
Here is another fix of the problem. This time the with_scope is removed from the yield and no with_exclusive_scope is used. This one should be a better solution. There is some code duplication but I didn't figure out a way to remove it. Probably a private method could be made to avoid that?
You flagged this item as spam.
Elliot Winkler August 1st, 2009 @ 09:21 PM
I don't understand why the exclusive scope is needed. Shouldn't you be able to do Shirt.red.find_each and find_each then do the right thing?
You flagged this item as spam.
Valentin Mihov August 19th, 2009 @ 09:18 AM
The problem here is that when you make additional queries into a find_each block, the queries are affected in an unexpected behavior.

Have a look at the unit test in the diff. In that example you have posts. Each post have a body and an author_id. You make a find_each for all posts with a given body (ex. "Hello world!"). In the find_each block imagine that you start processing a post from the author X. You want to count how many posts X has and you do Post.all(:conditions => { :author_id => id_of_author_X }). Unfortunately this query will return the number of posts from author X, which has a body equal to "Hello world!", which is obviously not the idea in that situation.
You flagged this item as spam.
Elliot Winkler August 19th, 2009 @ 11:13 AM
Ah, my bad, you were already one step ahead. +1, then -- makes sense to me.
You flagged this item as spam.
Thong Kuah September 8th, 2009 @ 12:09 PM
There is a similar one at #2227
You flagged this item as spam.
Valentin Mihov September 8th, 2009 @ 12:35 PM
Yep... #2227 seems to be the same bug
You flagged this item as spam.
Thong Kuah September 10th, 2009 @ 01:33 PM
Yep... #2227 seems to be the same bug

would you be willing to merge this ticket into #2227 and review the patch there? We are only one short of getting patch verified and applied. Thanks
CancelProfileIsBroken September 11th, 2009 @ 11:10 AM
- State changed from “new” to “duplicate”
Dupe of #2227