This project is archived and is in readonly mode.
[VERIFIED] batches: :conditions for each are applied to each Model.find within the each loop
Reported by claudiob (at gmail) | March 13th, 2009 @ 09:56 AM | in 2.x
Consider the following code:
# app/models/song.rb
class Song < ActiveRecord::Base
def self.print_pairs(name)
Song.each(:conditions => ['name = ?', name]) do |song|
puts song[:name]
another_song = Song.first(:conditions => ['name <> ?', name])
puts another_song[:name]
end
end
end
I would call it by passing a song name, and it would print out the name of the same song, and the name of another song (with a different name). After applying patch #2201, the .each function has become .find_each
The problem that I have found is that the :conditions applied to Song.each (line 4) is also automatically applied to the Song.first (line 6), and as a consequence Song.first returns an error, since it cannot find a song that matches both conditions ['name = ?', name] and ['name <> ?', name].
I understand that the solution to this problem is to call the inside find as following (line 6):
another_song = Song.with_exclusive_scope { first(:conditions => ['name <> ?', name]) }
However it is not clear whether this is a desired behavior or rather a bug. In this case I have not defined any default_scope for the model Song that has to be overridden by calling with_exclusive_scope, I am just calling Song.find within a Song.each(:conditions) loop.
To replicate this behavior:
rails music
cd music
script/generate scaffold song name:string
rake db:create
rake db:migrate
script/console
Song.create(:name => "Keep The Faith")
Song.create(:name => "Bed Of Roses")
class Song < ActiveRecord::Base
def self.print_pairs(name)
Song.each(:conditions => ['name = ?', name]) do |song|
puts song[:name]
another_song = Song.with_exclusive_scope { first(:conditions => ['name <> ?', name]) }
puts another_song[:name]
end
end
end
Song.print_pairs('Bed Of Roses')
Once again, the same occurs also after patch #2201, calling .find_each rather than .each, and also using a named_scope to declare the conditions.
Comments and changes to this ticket
-
Pratik March 13th, 2009 @ 10:12 AM
- Assigned user set to DHH
-
Matthew Beale June 5th, 2009 @ 07:23 PM
This is really nasty. I've attached a patch (with tests) that doesn't use with_scope in find_in_batches.
Check this blog post:
http://davedupre.com/2009/05/20/gotcha-with-find_each-and-find_in_b...
A really simple example of why scopes here are very dangerous:
Balloons.count #=> 10 @clown = Clown.find(2) @clown.balloons.count #=> 3 @clown.balloons.collect {|b| b.color } #=> [ 'red', blue', 'green' ] Balloons.find_each(:conditions => { :color => 'red' }) do @clown = Clown.find(2) @clown.balloons.count #=> 1 @clown.balloons.collect {|b| b.color } #=> [ 'red' ] # Where did all my friggin balloons go? Balloons.all.collect { |b| b.color }.uniq #=> [ 'red' ] end
This means inside a find_each it's impossible to ask Balloons for a non-red object. Also hard to debug :-)
-
Matthew Beale June 5th, 2009 @ 07:55 PM
If this is closed, these can probably be marked dupes:
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets...
This guy covers a bug with using "id" explicitly:
https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets...
-
Eugene Pimenov July 2nd, 2009 @ 03:10 AM
- Tag changed from 2.3-rc1, 2.3-rc2, :conditions, active_record, find, named_scope to 2.3-rc1, 2.3-rc2, :conditions, active_record, find, named_scope, patch
More elegant fix
-
Jacob Kjeldahl September 3rd, 2009 @ 02:18 PM
- Tag changed from 2.3-rc1, 2.3-rc2, :conditions, active_record, find, named_scope, patch to 2-3-stable, 2.3-rc1, 2.3-rc2, :conditions, active_record, find, named_scope, patch
I have verified that the 0001-find_in_batches-shouldn-t-clog-conditions-for-find-c.patch applies cleany to 2.3-stable and the tests are running.
-
Thong Kuah September 8th, 2009 @ 01:07 PM
+1 I have tried this patch(0001-find_in_batches-shouldn-t-clog-conditions-for-find-c.patch) against 2-3-stable, and the tests work well. Nice test - shows how innocuous find(:id) fails in a find_each block.
-
Valentin Mihov September 11th, 2009 @ 09:33 AM
https://rails.lighthouseapp.com/projects/8994/tickets/2791-activere...
is the same is this one. I tested the patch on 2-3 stable and it applies and works cleanly. I like the fix. It is better than the two I offered.
I am attaching an additional unit test from #2791. It should be applied over the patch of this issue.
-
Thong Kuah September 11th, 2009 @ 10:21 AM
- Title changed from batches: :conditions for each are applied to each Model.find within the each loop to [VERIFIED] batches: :conditions for each are applied to each Model.find within the each loop
state:verified
-
DHH December 28th, 2009 @ 07:37 PM
- Assigned user changed from DHH to Pratik
-
Scott Windsor March 25th, 2010 @ 10:52 PM
So... this is a bug that's been "verified". Does that mean this fix will get integrated anytime soon?
This is kind of a horrible bug. I just ran into a case where a model was getting incremented with callbacks and that update was getting scoped. (because update_counters uses update_all). Any chance this will get integrated into 2.3 stable?
-
Ryan Bigg April 11th, 2010 @ 02:12 PM
- State changed from new to verified
-
Repository April 15th, 2010 @ 01:13 AM
- State changed from verified to resolved
(from [18ba648e0de032c6cb68cb48c2aaf3a347294e3d]) Implement find_in_batches without with_scope [#2227 state:resolved]
Signed-off-by: Pratik Naik pratiknaik@gmail.com
http://github.com/rails/rails/commit/18ba648e0de032c6cb68cb48c2aaf3... -
Christopher Sun April 20th, 2010 @ 03:51 PM
Is this patch supposed to fix find_in_batches with named_scopes? I'm seeing issues where conditions provided by named_scopes on a model are being passed through to subsequent finders on any instances obtained through find_in_batches. For example:
Subscription.active.commercial.expired_within(1.year.ago,1.year.from_now).find_in_batches(:batch_size => 500) { |subs| Subscription.first }
My expected behavior was to not get the first subscription within active and commercial subscription scopes.
I tested the patched in 2.3.3 and 2.3.5 and had the same issue.
-
Pratik April 20th, 2010 @ 03:55 PM
Please open a new ticket w/ a patch and/or failing test and assign to me.
Thanks.
-
Greg Hazel March 29th, 2011 @ 12:53 AM
- Importance changed from to
Was there ever a new ticket created for this? The bug still exists in ActiveRecord 2.3.11
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
People watching this ticket
Attachments
Referenced by
- 2227 [VERIFIED] batches: :conditions for each are applied to each Model.find within the each loop (from [18ba648e0de032c6cb68cb48c2aaf3a347294e3d]) Impleme...
- 2260 find_in_batches updates (+2 bugs) Fixed in #2227
- 2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed There is a similar one at #2227
- 2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed Yep... #2227 seems to be the same bug
- 2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed Yep... #2227 seems to be the same bug
- 2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed would you be willing to merge this ticket into #2227 and ...
- 2791 ActiveRecord::Base.find_in_batches puts a with_scope into the block that is executed Dupe of #2227
- 2128 ActiveRecord::find_in_batches bug or feature? Closing this as a duplicate of #2227.
- 3235 default scoping and associations This is a duplicate of #2227 (also #2128, #2791) - may wa...
- 2260 find_in_batches updates (+2 bugs) https://rails.lighthouseapp.com/projects/8994/tickets/22...
- 2128 ActiveRecord::find_in_batches bug or feature? https://rails.lighthouseapp.com/projects/8994/tickets/22...