Skip to content

Commit

Permalink
Merge pull request #164 from Shopify/bb/primary-key-with-line-breaks
Browse files Browse the repository at this point in the history
Handle composite primary keys with line breaks
  • Loading branch information
coding-chimp authored Jul 23, 2024
2 parents 7389975 + d753bbf commit 1344ebe
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 7 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Unreleased

# 4.2.3 (Jul, 2024)
* Fix check for warnings against PKs with line breaks

# 4.2.2 (Jun, 2024)
* Avoid using the INSTANT algorithm.

Expand Down
2 changes: 1 addition & 1 deletion Gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: .
specs:
lhm-shopify (4.2.2)
lhm-shopify (4.2.3)
retriable (>= 3.0.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/activerecord_6.1.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
lhm-shopify (4.2.2)
lhm-shopify (4.2.3)
retriable (>= 3.0.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/activerecord_7.0.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
lhm-shopify (4.2.2)
lhm-shopify (4.2.3)
retriable (>= 3.0.0)

GEM
Expand Down
2 changes: 1 addition & 1 deletion gemfiles/activerecord_7.1.gemfile.lock
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
PATH
remote: ..
specs:
lhm-shopify (4.2.2)
lhm-shopify (4.2.3)
retriable (>= 3.0.0)

GEM
Expand Down
3 changes: 1 addition & 2 deletions lib/lhm/chunker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ def initialize(migration, connection = nil, options = {})
@chunk_finder = ChunkFinder.new(migration, connection, options)
@options = options
@raise_on_warnings = options.fetch(:raise_on_warnings, false)
@pk_duplicate_warning_regexp ||= /Duplicate entry .+ for key '(#{@migration.destination_name}\.)?PRIMARY'/
@verifier = options[:verifier]
if @throttler = options[:throttler]
@throttler.connection = @connection if @throttler.respond_to?(:connection=)
Expand Down Expand Up @@ -81,7 +80,7 @@ def execute

def raise_on_non_pk_duplicate_warning
@connection.select_all("SHOW WARNINGS", should_retry: true, log_prefix: LOG_PREFIX).each do |row|
next if row["Message"].match?(@pk_duplicate_warning_regexp)
next if row["Message"].start_with?("Duplicate entry") && row["Message"].match?(/for key '(#{@migration.destination_name}\.)?PRIMARY'\z/)

m = "Unexpected warning found for inserted row: #{row["Message"]}"
Lhm.logger.warn(m)
Expand Down
2 changes: 1 addition & 1 deletion lib/lhm/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
# Schmidt

module Lhm
VERSION = '4.2.2'
VERSION = '4.2.3'
end
10 changes: 10 additions & 0 deletions spec/fixtures/composite_primary_key_with_varchar_columns.ddl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CREATE TABLE `composite_primary_key_with_varchar_columns` (
`id` bigint NOT NULL AUTO_INCREMENT,
`shop_id` bigint NOT NULL,
`owner_type` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
`owner_id` bigint NOT NULL,
`namespace` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
`key` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
PRIMARY KEY (`shop_id`,`owner_type`,`owner_id`,`namespace`,`key`),
UNIQUE KEY `id` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC
10 changes: 10 additions & 0 deletions spec/fixtures/composite_primary_key_with_varchar_columns_dest.ddl
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
CREATE TABLE `composite_primary_key_with_varchar_columns_dest` (
`id` bigint NOT NULL AUTO_INCREMENT,
`shop_id` bigint NOT NULL,
`owner_type` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
`owner_id` bigint NOT NULL,
`namespace` varchar(255) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
`key` varchar(64) CHARACTER SET utf8mb4 COLLATE utf8mb4_general_ci NOT NULL DEFAULT '',
PRIMARY KEY (`shop_id`,`owner_type`,`owner_id`,`namespace`,`key`),
UNIQUE KEY `id` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci ROW_FORMAT=DYNAMIC
40 changes: 40 additions & 0 deletions spec/integration/chunker_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,46 @@ def log_messages
end
end

it 'should copy and ignore duplicate composite primary key with line breaks' do
origin = table_create(:composite_primary_key_with_varchar_columns)
destination = table_create(:composite_primary_key_with_varchar_columns_dest)
migration = Lhm::Migration.new(origin, destination)

execute("insert into composite_primary_key_with_varchar_columns set id = 1001, shop_id = 1, owner_type = 'Product', owner_id = 1, namespace = '
23
23
', `key` = '
14
1
'")
execute("insert into composite_primary_key_with_varchar_columns set id = 1002, shop_id = 1, owner_type = 'Product', owner_id = 1, namespace = '
23
22
', `key` = '
14
1
'")
execute("insert into composite_primary_key_with_varchar_columns_dest set id = 1002, shop_id = 1, owner_type = 'Product', owner_id = 1, namespace = '
23
22
', `key` = '
14
1
'")

Lhm::Chunker.new(migration, connection, {raise_on_warning: true, throttler: throttler, printer: printer} ).run

replica do
value(count_all(destination.name)).must_equal(2)
end
end

it 'should copy and raise on unexpected warnings' do
origin = table_create(:custom_primary_key)
destination = table_create(:custom_primary_key_dest)
Expand Down

0 comments on commit 1344ebe

Please sign in to comment.