From 2bb3d184469f0f19c64e86934563c2448046eb74 Mon Sep 17 00:00:00 2001
From: fatkodima <fatkodima123@gmail.com>
Date: Sat, 9 Nov 2024 03:33:47 +0200
Subject: [PATCH] Fix running background migrations over relations with
 duplicate records

---
 CHANGELOG.md                                        |  2 ++
 lib/online_migrations/batch_iterator.rb             |  3 ++-
 test/background_migrations/background_migrations.rb | 10 ++++++++++
 test/background_migrations/migration_test.rb        |  9 +++++++++
 4 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a2489a7..169a0bb 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,5 +1,7 @@
 ## master (unreleased)
 
+- Fix running background migrations over relations with duplicate records
+
 ## 0.20.1 (2024-11-05)
 
 - Fix background data migrations to work with `includes`/`eager_load` on relation
diff --git a/lib/online_migrations/batch_iterator.rb b/lib/online_migrations/batch_iterator.rb
index 242782c..664c1fe 100644
--- a/lib/online_migrations/batch_iterator.rb
+++ b/lib/online_migrations/batch_iterator.rb
@@ -75,9 +75,10 @@ def each_batch(of: 1000, column: relation.primary_key, start: nil, finish: nil,
         # Retaining the results in the query cache would undermine the point of batching.
         batch_relation.uncached { yield batch_relation, start_id, last_id }
 
-        break if stop_row.nil?
+        break if last_id == finish
 
         start_id = stop_id
+        stop_id = nil
       end
     end
 
diff --git a/test/background_migrations/background_migrations.rb b/test/background_migrations/background_migrations.rb
index caa46fa..16f33bf 100644
--- a/test/background_migrations/background_migrations.rb
+++ b/test/background_migrations/background_migrations.rb
@@ -101,6 +101,16 @@ def process_batch(_users)
     end
   end
 
+  class RelationWithJoins < OnlineMigrations::BackgroundMigration
+    def relation
+      User.joins(:projects)
+    end
+
+    def process_batch(_users)
+      # no-op
+    end
+  end
+
   class MigrationWithCount < OnlineMigrations::BackgroundMigration
     def relation
       User.all
diff --git a/test/background_migrations/migration_test.rb b/test/background_migrations/migration_test.rb
index a00e90a..bdac566 100644
--- a/test/background_migrations/migration_test.rb
+++ b/test/background_migrations/migration_test.rb
@@ -168,6 +168,15 @@ def test_relation_with_includes
       assert m.succeeded?
     end
 
+    def test_relation_with_duplicate_records
+      user = User.create!
+      3.times { user.projects.create! }
+      m = create_migration(migration_name: "RelationWithJoins", batch_size: 2, sub_batch_size: 2)
+
+      run_all_migration_jobs(m)
+      assert m.succeeded?
+    end
+
     def test_progress_succeded_migration
       m = build_migration(status: :succeeded)
       assert_in_delta 100.0, m.progress