Stop find_in_batches using the records after yielding. #1435

Closed
wants to merge 1 commit into
from

Projects

None yet

1 participant

@caius
caius commented May 31, 2011

Currently if the code that calls find_in_batches modifies the yielded array in place then find_in_batches can enter an infinite loop searching with ruby object ids in the database instead of the primary key of records in the database. This happens because it naively assumes the yielded array hasn't been modified before calling id on the last object in the array. And ruby (1.8 at least) alias' id to object_id so an integer is still returned no matter what the last object is.

By moving finding the id of the last object before yielding the array of records it means the calling code can do whatever it wants to the array in terms of modifying it in place, and find_in_batches doesn't care. Also moved the count of the array records before yielding in case elements are removed or added when yielded.

@caius caius Stop find_in_batches using the records after yielding.
Currently if the code that calls .find_in_batches modifies the yielded array in place then .find_in_batches can enter an infinite loop searching with ruby object ids in the database instead of the primary key of records in the database. This happens because it naively assumes the yielded array hasn't been modified before calling #id on the last object in the array. And ruby (1.8 at least) alias' #id to #object_id so an integer is still returned no matter what the last object is.

By moving finding the #id of the last object before yielding the array it means the calling code can do whatever it wants to the array in terms of modifying it in place, and .find_in_batches doesn't care.
96be08d
@caius
caius commented Jun 21, 2011

Refactored the test, going to open a new pull request with that code.

@caius caius closed this Jun 21, 2011
@jake3030 jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
@lifo lifo Make ActionController#render(string) work as a shortcut for render :f…
…ile => string. [#1435]

Examples:
  # Instead of render(:file => '/Users/lifo/home.html.erb')
  render('/Users/lifo/home.html.erb')

Note : Filename must begin with a forward slash ('/')
0619523
@jake3030 jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
@lifo lifo Make ActionController#render(string) work as a shortcut for render :t…
…emplate => string. [#1435]

Examples:
  # Instead of render(:template => 'controller/action')
  render('controller/action')

Note : Argument must not begin with a '/', but have at least one '/'
d67e038
@jake3030 jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
@lifo lifo Make ActionController#render(string) work as a shortcut for render :a…
…ction => string. [#1435]

Examples:
  # Instead of render(:action => 'other_action')
  render('other_action')

Note : Argument must not have any '/'
cd1d6e8
@jake3030 jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
@lifo lifo Make ActionController#render(symbol) behave same as ActionController#…
…render(string) [#1435]
80307c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment