Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing data bug #4

Merged
merged 9 commits into from
May 8, 2019
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ ActiveRecord 3.2.x and 4.x (mysql and mysql2 adapters).

## Limitations

Due to the Chunker implementation, Lhm requires that the table to migrate has a
Due to the Chunker implementation, Lhm requires that the table to migrate has
a single integer numeric key column called `id`.

Another note about the Chunker, it performs static sized row copies against the `id`
Expand Down Expand Up @@ -78,6 +78,7 @@ ActiveRecord::Base.establish_connection(
# and migrate
Lhm.change_table :users do |m|
m.add_column :arbitrary, "INT(12)"
m.add_column :locale, "VARCHAR(2) NOT NULL DEFAULT 'en'"
m.add_index [:arbitrary_id, :created_at]
m.ddl("alter table %s add column flag tinyint(1)" % m.name)
end
Expand Down
2 changes: 1 addition & 1 deletion lib/lhm/chunker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def initialize(migration, connection = nil, options = {})
def execute
return unless @start && @limit
@next_to_insert = @start
while @next_to_insert < @limit || (@start == @limit)
while @next_to_insert <= @limit
stride = @throttler.stride
affected_rows = @connection.update(copy(bottom, top(stride)))

Expand Down
47 changes: 47 additions & 0 deletions spec/unit/chunker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,56 @@ def @throttler.stride
@connection.verify
end

it 'correctly copies single record tables when start is not 1' do
@chunker = Lhm::Chunker.new(@migration, @connection, :throttler => @throttler,
:start => 2,
:limit => 2)

@connection.expect(:update, 1) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /between 2 and 2/
end

@chunker.run
@connection.verify
end

it 'correctly copies every record tables when difference is a multiple of stride plus one' do
def @throttler.stride
2
end
@chunker = Lhm::Chunker.new(@migration, @connection, :throttler => @throttler,
:start => 1,
:limit => 5)

@connection.expect(:update, 2) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /between 1 and 2/
end
@connection.expect(:update, 2) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /between 3 and 4/
end
@connection.expect(:update, 1) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /between 5 and 5/
end

@chunker.run
@connection.verify
end

it 'separates filter conditions from chunking conditions' do
def @throttler.stride
2
end
@chunker = Lhm::Chunker.new(@migration, @connection, :throttler => @throttler,
:start => 1,
:limit => 2)
@connection.expect(:update, 1) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /where \(foo.created_at > '2013-07-10' or foo.baz = 'quux'\) and `foo`/
stmt =~ /between 1 and 2/
end

def @migration.conditions
Expand All @@ -125,12 +168,16 @@ def @migration.conditions
end

it "doesn't mess with inner join filters" do
def @throttler.stride
2
end
@chunker = Lhm::Chunker.new(@migration, @connection, :throttler => @throttler,
:start => 1,
:limit => 2)
@connection.expect(:update, 1) do |stmt|
stmt = stmt.first if stmt.is_a?(Array)
stmt =~ /inner join bar on foo.id = bar.foo_id and/
stmt =~ /between 1 and 2/
end

def @migration.conditions
Expand Down