Skip to content

Commit 80fac8c

Browse files
committed
Preserve order within individual batches for batch iteration
1 parent dd33918 commit 80fac8c

File tree

2 files changed

+33
-10
lines changed

2 files changed

+33
-10
lines changed

activerecord/lib/active_record/relation/batches.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -435,20 +435,20 @@ def batch_on_unloaded_relation(relation:, start:, finish:, load:, cursor:, order
435435
if load
436436
records = batch_relation.records
437437
values = records.pluck(*cursor)
438-
yielded_relation = where(cursor => values)
438+
yielded_relation = where(cursor => values).order(batch_orders.to_h)
439439
yielded_relation.load_records(records)
440440
elsif (empty_scope && use_ranges != false) || use_ranges
441441
values = batch_relation.pluck(*cursor)
442442

443443
finish = values.last
444444
if finish
445445
yielded_relation = apply_finish_limit(batch_relation, cursor, finish, batch_orders)
446-
yielded_relation = yielded_relation.except(:limit, :order)
446+
yielded_relation = yielded_relation.except(:limit).reorder(batch_orders.to_h)
447447
yielded_relation.skip_query_cache!(false)
448448
end
449449
else
450450
values = batch_relation.pluck(*cursor)
451-
yielded_relation = where(cursor => values)
451+
yielded_relation = where(cursor => values).order(batch_orders.to_h)
452452
end
453453

454454
break if values.empty?

activerecord/test/cases/batches_test.rb

Lines changed: 30 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,36 @@ def test_in_batches_should_be_loaded
455455
end
456456
end
457457

458+
def test_in_loaded_batches_preserves_order_within_batches
459+
expected_posts = Post.order(id: :desc).to_a
460+
posts = []
461+
462+
Post.in_batches(of: 2, load: true, order: :desc) do |relation|
463+
posts.concat(relation.where("1 = 1"))
464+
end
465+
assert_equal expected_posts, posts
466+
end
467+
468+
def test_in_range_batches_preserves_order_within_batches
469+
expected_posts = Post.order(id: :desc).to_a
470+
posts = []
471+
472+
Post.in_batches(of: 2, order: :desc, use_ranges: true) do |relation|
473+
posts.concat(relation)
474+
end
475+
assert_equal expected_posts, posts
476+
end
477+
478+
def test_in_scoped_batches_preserves_order_within_batches
479+
expected_posts = Post.order(id: :desc).to_a
480+
posts = []
481+
482+
Post.where("id > 0").in_batches(of: 2, order: :desc) do |relation|
483+
posts.concat(relation)
484+
end
485+
assert_equal expected_posts, posts
486+
end
487+
458488
def test_in_batches_if_not_loaded_executes_more_queries
459489
assert_queries_count(@total + 1) do
460490
Post.in_batches(of: 1, load: false) do |relation|
@@ -638,13 +668,6 @@ def test_in_batches_executes_range_queries_when_constrained_and_opted_in_into_ra
638668
end
639669
end
640670

641-
def test_in_batches_no_subqueries_for_whole_tables_batching
642-
quoted_posts_id = Regexp.escape(quote_table_name("posts.id"))
643-
assert_queries_match(/DELETE FROM #{Regexp.escape(quote_table_name("posts"))} WHERE #{quoted_posts_id} > .+ AND #{quoted_posts_id} <=/i) do
644-
Post.in_batches(of: 2).delete_all
645-
end
646-
end
647-
648671
def test_in_batches_shouldnt_execute_query_unless_needed
649672
assert_queries_count(2) do
650673
Post.in_batches(of: @total) { |relation| assert_kind_of ActiveRecord::Relation, relation }

0 commit comments

Comments
 (0)