soundcloud / lhm

Online MySQL schema migrations
BSD 3-Clause "New" or "Revised" License
1.83k stars 190 forks source link

Forgotten line #126

Closed tdeo closed 6 years ago

tdeo commented 9 years ago

Hi,

I was writing some migration on a table, and came across a weird issue while testing in development environment. When I have a single line in a table, this line is lost during the migration process. It works well with 2, 5 or more.

I believe this comes from https://github.com/soundcloud/lhm/blob/master/lib/lhm/chunker.rb#L30. If I understand that line well, the || (@next_to_insert == 1 && @start == 1) part was added in case @next_to_insert == 0 && @start == 1 && @limit == 1 (if @limit != 1, the second part of the or statement implies the first one).

Shouldn't that line be changed from while @next_to_insert < @limit || (@next_to_insert == 1 && @start == 1) to while @next_to_insert <= @limit?

Moreover, I had a feeling that if @limit - @start % stride == 1, the last line would be lost. I tested that it is actually the case.

tdeo commented 7 years ago

@grobie @sunny Does this issue makes sense and have a chance of being addressed?

I have two PR opened for this:

Should I rebase them?

grobie commented 7 years ago

@tdeo Please go ahead! While I'm not actively using or maintaining LHM anymore, I'd be more than happy to merge a well tested bug fix.