Merge pull request #137 from marqeta/pt-osc-bug-1632522

pt-osc: Fails with duplicate key in table for self-referencing FK
This commit is contained in:
Carlos Salguero
2017-01-16 14:55:49 +02:00
committed by GitHub
5 changed files with 189 additions and 16 deletions
+1
View File
@@ -8,3 +8,4 @@ release
snapshot
.DS_Store
build
Makefile.old
+22 -8
View File
@@ -10123,11 +10123,23 @@ sub create_new_table {
$sql =~ s/\ACREATE TABLE .*?\($/CREATE TABLE $quoted (/m;
# If it has a leading underscore, we remove one, otherwise we add one
# When the new temp table is created, we need to avoid collisions on constraint names
# This is in contrast to previous behavior were we added underscores
# indefinitely, sometimes exceeding the allowed name limit
# https://bugs.launchpad.net/percona-toolkit/+bug/1215587
$sql =~ s/^ CONSTRAINT `(_?)/' CONSTRAINT `'.($1 eq '' ? '_' : '')/gme;
# So we do replacements when constraint names:
# Has 2 _, we remove them
# Has 1 _, we add one to make 2
# Has no _, we add one to make 1
# This gives on more salt where the FK names have been previously been altered
# https://bugs.launchpad.net/percona-toolkit/+bug/1632522
my %search_dict = (
'CONSTRAINT `__' => 'CONSTRAINT `',
'CONSTRAINT `_' => 'CONSTRAINT `__',
'CONSTRAINT `' => 'CONSTRAINT `_'
);
my $constraint_pattern = qr((CONSTRAINT `__|CONSTRAINT `_|CONSTRAINT `));
$sql =~ s/$constraint_pattern/$search_dict{$1}/gm;
if ( $o->get('default-engine') ) {
$sql =~ s/\s+ENGINE=\S+//;
@@ -10513,13 +10525,15 @@ sub rebuild_constraints {
# This is in contrast to previous behavior were we added underscores
# indefinitely, sometimes exceeding the allowed name limit
# https://bugs.launchpad.net/percona-toolkit/+bug/1215587
my $new_fk;
if ($fk =~ /^_/) {
($new_fk = $fk) =~ s/^_//;
}else {
$new_fk = '_'.$fk;
# Add one more salt to renaming FK constraint names
# This will add 2 _ to a self referencing FK thus avoiding a duplicate key constraint
# https://bugs.launchpad.net/percona-toolkit/+bug/1632522
my $new_fk;
if ($fk =~ /^__/) {
($new_fk = $fk) =~ s/^__//;
} else {
$new_fk = '_'.$fk;
}
PTDEBUG && _d("Old FK name: $fk New FK name: $new_fk");
@@ -42,7 +42,7 @@ my $sample = "t/pt-online-schema-change/samples/";
$sb->load_file('master', "$sample/bug-1215587.sql");
# run once: we expect constraint names to be prefixed with one underscore
# if they havre't one, and to remove one if they already do.
# if they havre't one, and to remove 2 if they have 2
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1215587,t=Table1",
@@ -50,25 +50,46 @@ $sb->load_file('master', "$sample/bug-1215587.sql");
qw(--execute)) },
);
my $constraints = $master_dbh->selectall_arrayref("SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1215587' and (TABLE_NAME='Table1' OR TABLE_NAME='Table2') and CONSTRAINT_NAME LIKE '%fkey%' ORDER BY TABLE_NAME, CONSTRAINT_NAME");
is_deeply(
$constraints,
[
['Table1', 'fkey1a'],
['Table1', '_fkey1b'],
['Table2', 'fkey2b'],
['Table1', '__fkey1a'],
['Table2', '_fkey2a'],
['Table2', '__fkey2b'],
],
"First run adds or removes underscore from constraint names, accordingly"
);
# run second time
# we expect constraints to be the same as we started (toggled back)
# run second time: we expect constraint names to be prefixed with one underscore
# if they havre't one, and to remove 2 if they have 2
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1215587,t=Table1",
"--alter", "ENGINE=InnoDB",
qw(--execute)) },
);
$constraints = $master_dbh->selectall_arrayref("SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1215587' and (TABLE_NAME='Table1' OR TABLE_NAME='Table2') and CONSTRAINT_NAME LIKE '%fkey%' ORDER BY TABLE_NAME, CONSTRAINT_NAME");
is_deeply(
$constraints,
[
['Table1', 'fkey1a'],
['Table1', '__fkey1b'],
['Table2', 'fkey2b'],
['Table2', '__fkey2a'],
],
"Second run adds or removes underscore from constraint names, accordingly"
);
# run third time: we expect constraints to be the same as we started (toggled back)
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1215587,t=Table1",
@@ -87,10 +108,9 @@ is_deeply(
['Table2', 'fkey2a'],
['Table2', '_fkey2b'],
],
"Second run toggles constraint names back to how they were"
"Third run toggles constraint names back to how they were"
);
# #############################################################################
# Done.
# #############################################################################
@@ -0,0 +1,113 @@
#!/usr/bin/env perl
BEGIN {
die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n"
unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH};
unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib";
};
use strict;
use warnings FATAL => 'all';
use English qw(-no_match_vars);
use Test::More;
use Data::Dumper;
use PerconaTest;
use Sandbox;
require "$trunk/bin/pt-online-schema-change";
my $dp = new DSNParser(opts=>$dsn_opts);
my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
my $master_dbh = $sb->get_dbh_for('master');
if ( !$master_dbh ) {
plan skip_all => 'Cannot connect to sandbox master';
}
# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
# so we need to specify --set-vars innodb_lock_wait_timeout-3 else the
# tool will die.
my $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox';
my @args = (qw(--set-vars innodb_lock_wait_timeout=3 --alter-foreign-keys-method rebuild_constraints));
my $output;
my $exit_status;
my $sample = "t/pt-online-schema-change/samples/";
# ############################################################################
# https://bugs.launchpad.net/percona-toolkit/+bug/1632522
# pt-online-schema-change fails with duplicate key in table for self-referencing FK
# ############################################################################
$sb->load_file('master', "$sample/bug-1632522.sql");
# run once: we expect the constraint name to be appended with one underscore
# but the self-referencing constraint will have 2 underscore
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1632522,t=test_table",
"--alter", "ENGINE=InnoDB",
qw(--execute)) },
);
my $constraints = $master_dbh->selectall_arrayref("SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1632522' and (TABLE_NAME='test_table' OR TABLE_NAME='person') and CONSTRAINT_NAME LIKE '%fk_%' ORDER BY TABLE_NAME, CONSTRAINT_NAME");
is_deeply(
$constraints,
[
['person', '_fk_testId'],
['test_table', '_fk_person'],
['test_table', '__fk_refId'],
],
"First run adds or removes underscore from constraint names, accordingly"
);
# run second time: we expect constraint names to be prefixed with one underscore
# if they havre't one, and to remove 2 if they have 2
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1632522,t=test_table",
"--alter", "ENGINE=InnoDB",
qw(--execute)) },
);
$constraints = $master_dbh->selectall_arrayref("SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1632522' and (TABLE_NAME='test_table' OR TABLE_NAME='person') and CONSTRAINT_NAME LIKE '%fk_%' ORDER BY TABLE_NAME, CONSTRAINT_NAME");
is_deeply(
$constraints,
[
['person', '__fk_testId'],
['test_table', '_fk_refId'],
['test_table', '__fk_person'],
],
"Second run self-referencing will be one due to rebuild_constraints"
);
# run third time: we expect constraints to be the same as we started (toggled back)
($output, $exit_status) = full_output(
sub { pt_online_schema_change::main(@args,
"$master_dsn,D=bug1632522,t=test_table",
"--alter", "ENGINE=InnoDB",
qw(--execute)) },
);
$constraints = $master_dbh->selectall_arrayref("SELECT TABLE_NAME, CONSTRAINT_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1632522' and (TABLE_NAME='test_table' OR TABLE_NAME='person') and CONSTRAINT_NAME LIKE '%fk_%' ORDER BY TABLE_NAME, CONSTRAINT_NAME");
is_deeply(
$constraints,
[
['person', 'fk_testId'],
['test_table', 'fk_person'],
['test_table', 'fk_refId'],
],
"Third run toggles constraint names back to how they were"
);
# #############################################################################
# Done.
# #############################################################################
$sb->wipe_clean($master_dbh);
ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
done_testing;
@@ -0,0 +1,25 @@
-- Setup database and test tables with self referencing FK
drop database if exists bug1632522;
create database bug1632522;
use bug1632522;
CREATE TABLE `person` (
`id` bigint(20) NOT NULL AUTO_INCREMENT,
`name` varchar(20) NOT NULL,
`testId` bigint(20) DEFAULT NULL,
PRIMARY KEY (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
CREATE TABLE `test_table` (
`id` bigint(20) NOT NULL AUTO_INCREMENT,
`refId` bigint(20) DEFAULT NULL,
`person` bigint(20) DEFAULT NULL,
PRIMARY KEY (`id`),
KEY `fk_person` (`person`),
KEY `fk_refId` (`refId`),
CONSTRAINT `fk_person` FOREIGN KEY (`person`) REFERENCES `person` (`id`),
CONSTRAINT `fk_refId` FOREIGN KEY (`refId`) REFERENCES `test_table` (`id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;
ALTER TABLE `person` ADD CONSTRAINT `fk_testId` FOREIGN KEY (`testId`) REFERENCES `test_table` (`id`);