Skip to content

Commit

Permalink
Fix background data migrations to work with includes/eager_load o…
Browse files Browse the repository at this point in the history
…n relation

Fixes #65.
  • Loading branch information
fatkodima committed Oct 31, 2024
1 parent ad4a641 commit 4f76103
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
## master (unreleased)

- Fix background data migrations to work with `includes`/`eager_load` on relation
- Fix problem with running migrations for `activerecord` 8.0.0.rc2

## 0.20.0 (2024-10-21)
Expand Down
3 changes: 2 additions & 1 deletion lib/online_migrations/batch_iterator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ def each_batch(of: 1000, column: relation.primary_key, start: nil, finish: nil,
end

relation = apply_limits(self.relation, column, start, finish, order)
base_relation = relation.reselect(column).reorder(column => order)
unscopes = Utils.ar_version < 7.1 ? [:includes] : [:includes, :preload, :eager_load]
base_relation = relation.unscope(*unscopes).reselect(column).reorder(column => order)

start_id = start || begin
start_row = base_relation.uncached { base_relation.first }
Expand Down
10 changes: 10 additions & 0 deletions test/background_migrations/background_migrations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,16 @@ def count
end
end

class RelationWithIncludes < OnlineMigrations::BackgroundMigration
def relation
Project.includes(user: :commits)
end

def process_batch(_users)
# no-op
end
end

class MigrationWithCount < OnlineMigrations::BackgroundMigration
def relation
User.all
Expand Down
19 changes: 16 additions & 3 deletions test/background_migrations/migration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

module BackgroundMigrations
class MigrationTest < Minitest::Test
class User < ActiveRecord::Base
end

def setup
@connection = ActiveRecord::Base.connection
@connection.create_table(:users, force: true) do |t|
Expand All @@ -18,10 +15,17 @@ def setup
t.bigint :user_id
end

@connection.create_table(:commits, force: true) do |t|
t.bigint :user_id
end

User.reset_column_information
Project.reset_column_information
Commit.reset_column_information
end

def teardown
@connection.drop_table(:commits, if_exists: true)
@connection.drop_table(:projects, if_exists: true)
@connection.drop_table(:users, if_exists: true)
OnlineMigrations::BackgroundMigrations::Migration.delete_all
Expand Down Expand Up @@ -155,6 +159,15 @@ def test_empty_relation
assert m.enqueued?
end

def test_relation_with_includes
user = User.create!
_project = user.projects.create!
m = create_migration(migration_name: "RelationWithIncludes")

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
Expand Down
7 changes: 7 additions & 0 deletions test/support/models.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,16 @@

class User < ActiveRecord::Base
has_many :projects
has_many :commits
end

class Project < ActiveRecord::Base
belongs_to :user
has_many :commits
end

class Commit < ActiveRecord::Base
belongs_to :user
end

class ShardRecord < ActiveRecord::Base
Expand Down

0 comments on commit 4f76103

Please sign in to comment.