Skip to content

Commit 14cfad3

Browse files
committed
Fix ActiveRecord batching over composite primary keys
1 parent 18acbe8 commit 14cfad3

File tree

2 files changed

+33
-15
lines changed

2 files changed

+33
-15
lines changed

activerecord/lib/active_record/relation/batches.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,9 @@ def batch_condition(relation, columns, values, operators)
343343
where_clause = predicate_builder[first_clause_column, first_clause_value, operator]
344344

345345
cursor_positions.reverse_each do |column_name, value, operator|
346-
where_clause = predicate_builder[column_name, value, operator].and(where_clause)
346+
where_clause = predicate_builder[column_name, value, operator == :lteq ? :lt : :gt].or(
347+
predicate_builder[column_name, value, :eq].and(where_clause)
348+
)
347349
end
348350

349351
relation.where(where_clause)

activerecord/test/cases/batches_test.rb

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -773,9 +773,13 @@ def test_find_in_batches_should_return_a_sized_enumerator
773773

774774
test ".find_each iterates over composite primary key" do
775775
orders = Cpk::Order.order(*Cpk::Order.primary_key).to_a
776-
Cpk::Order.find_each(batch_size: 1).with_index do |order, index|
776+
777+
index = 0
778+
Cpk::Order.find_each(batch_size: 1) do |order|
777779
assert_equal orders[index], order
780+
index += 1
778781
end
782+
assert_equal orders.size, index
779783
end
780784

781785
test ".in_batches should start from the start option when using composite primary key" do
@@ -798,37 +802,49 @@ def test_find_in_batches_should_return_a_sized_enumerator
798802
end
799803

800804
test ".find_each with multiple column ordering and using composite primary key" do
801-
Cpk::Book.create(author_id: 1, number: 1)
802-
Cpk::Book.create(author_id: 1, number: 2)
803-
Cpk::Book.create(author_id: 1, number: 3)
805+
Cpk::Book.insert_all!([
806+
{ author_id: 1, number: 1 },
807+
{ author_id: 2, number: 1 },
808+
{ author_id: 2, number: 2 }
809+
])
804810
books = Cpk::Book.order(author_id: :asc, number: :desc).to_a
805-
Cpk::Book.find_each(batch_size: 1, order: [:asc, :desc]).with_index do |book, index|
811+
812+
index = 0
813+
Cpk::Book.find_each(batch_size: 1, order: [:asc, :desc]) do |book|
806814
assert_equal books[index], book
815+
index += 1
807816
end
817+
assert_equal books.size, index
808818
end
809819

810820
test ".in_batches should start from the start option when using composite primary key with multiple column ordering" do
811-
Cpk::Book.create(author_id: 1, number: 1)
812-
Cpk::Book.create(author_id: 1, number: 2)
813-
Cpk::Book.create(author_id: 1, number: 3)
821+
Cpk::Book.insert_all!([
822+
{ author_id: 1, number: 1 },
823+
{ author_id: 1, number: 2 },
824+
{ author_id: 1, number: 3 }
825+
])
814826
second_book = Cpk::Book.order(author_id: :asc, number: :desc).second
815827
relation = Cpk::Book.in_batches(of: 1, start: second_book.id, order: [:asc, :desc]).first
816828
assert_equal second_book, relation.first
817829
end
818830

819831
test ".in_batches should end at the finish option when using composite primary key with multiple column ordering" do
820-
Cpk::Book.create(author_id: 1, number: 1)
821-
Cpk::Book.create(author_id: 1, number: 2)
822-
Cpk::Book.create(author_id: 1, number: 3)
832+
Cpk::Book.insert_all!([
833+
{ author_id: 1, number: 1 },
834+
{ author_id: 1, number: 2 },
835+
{ author_id: 1, number: 3 }
836+
])
823837
second_book = Cpk::Book.order(author_id: :asc, number: :desc).second
824838
relation = Cpk::Book.in_batches(of: 1, finish: second_book.id, order: [:asc, :desc]).to_a.last
825839
assert_equal second_book, relation.first
826840
end
827841

828842
test ".in_batches with scope and multiple column ordering and using composite primary key" do
829-
Cpk::Book.create(author_id: 1, number: 1)
830-
Cpk::Book.create(author_id: 1, number: 2)
831-
Cpk::Book.create(author_id: 1, number: 3)
843+
Cpk::Book.insert_all!([
844+
{ author_id: 1, number: 1 },
845+
{ author_id: 1, number: 2 },
846+
{ author_id: 1, number: 3 }
847+
])
832848
book1, book2 = Cpk::Book.order(author_id: :asc, number: :desc).first(2)
833849
author_id, number = book1.id
834850
relation = Cpk::Book.where("author_id >= ? AND number < ?", author_id, number).in_batches(of: 1, order: [:asc, :desc]).first

0 commit comments

Comments
 (0)