From 21a9c35f5cf632f3e58ab876886228eef9de2ae7 Mon Sep 17 00:00:00 2001 From: frank-cizmich Date: Fri, 22 May 2015 17:03:32 -0300 Subject: [PATCH] pt-osc now limits size of contraint names --- bin/pt-online-schema-change | 29 ++++- .../rename_fk_constraints.t | 101 ++++++++++++++++++ .../samples/bug-1215587.sql | 23 ++++ 3 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 t/pt-online-schema-change/rename_fk_constraints.t create mode 100644 t/pt-online-schema-change/samples/bug-1215587.sql diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index ead42cfd..19e4b079 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -9861,7 +9861,17 @@ sub create_new_table { my $quoted = $q->quote($orig_tbl->{db}, $new_table_name); my $sql = $ddl; $sql =~ s/\ACREATE TABLE .*?\($/CREATE TABLE $quoted (/m; - $sql =~ s/^ CONSTRAINT `/ CONSTRAINT `_/gm; + + + # If it has a leading underscore, we remove one, otherwise we add one + # 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 + if ( $sql =~ /CONSTRAINT `_/ ) { + $sql =~ s/^ CONSTRAINT `_/ CONSTRAINT `/gm; + } else { + $sql =~ s/^ CONSTRAINT `/ CONSTRAINT `_/gm; + } if ( $o->get('default-engine') ) { $sql =~ s/\s+ENGINE=\S+//; } @@ -10200,8 +10210,23 @@ sub rebuild_constraints { # `actor` (`actor_id`) ON UPDATE CASCADE # Reference the correct table name... $constraint =~ s/REFERENCES[^\(]+/REFERENCES $orig_tbl->{name} /; + # And rename the constraint to avoid conflict - $constraint =~ s/CONSTRAINT `$fk`/CONSTRAINT `_$fk`/; + # If it has a leading underscore, we remove one, otherwise we add one + # 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; + } + + + PTDEBUG && _d("Old FK name: $fk New FK name: $new_fk"); + + $constraint =~ s/CONSTRAINT `$fk`/CONSTRAINT `$new_fk`/; my $sql = "DROP FOREIGN KEY `$fk`, " . "ADD $constraint"; diff --git a/t/pt-online-schema-change/rename_fk_constraints.t b/t/pt-online-schema-change/rename_fk_constraints.t new file mode 100644 index 00000000..cb1aabfe --- /dev/null +++ b/t/pt-online-schema-change/rename_fk_constraints.t @@ -0,0 +1,101 @@ +#!/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/1215587 +# Adding _ to constraints can create issues with constraint name length +# ############################################################################ + +$sb->load_file('master', "$sample/bug-1215587.sql"); + +# run once: we expect constraint names to be prefixed with one underscore +# note: We're running just a neutral no-op alter. We are only interested in constraint name +# changes. +($output, $exit_status) = full_output( + sub { pt_online_schema_change::main(@args, + "$master_dsn,D=bug1215587,t=Table1", + "--alter", "ENGINE=InnoDB", + qw(--execute)) }, +); + + +my $constraints = $master_dbh->selectall_hashref("SELECT CONSTRAINT_NAME, TABLE_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1215587' and (TABLE_NAME='Table1' OR TABLE_NAME='Table2') and CONSTRAINT_NAME LIKE '%fkey%'", 'table_name'); + + +is( + $constraints->{Table1}->{constraint_name}, + '_fkey1', + "Altered table: constraint name prefixed one underscore after 1st run" +); + +is( + $constraints->{Table2}->{constraint_name}, + '_fkey2', + "Child table : constraint name prefixed one underscore after 1st run" +); + + +# run second time +# we expect underscores to be removed +($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_hashref("SELECT CONSTRAINT_NAME, TABLE_NAME FROM information_schema.KEY_COLUMN_USAGE WHERE table_schema='bug1215587' and (TABLE_NAME='Table1' OR TABLE_NAME='Table2') and CONSTRAINT_NAME LIKE '%fkey%'", 'table_name'); + + +is( + $constraints->{'Table1'}->{constraint_name}, + 'fkey1', + "Altered table: constraint name removed underscore after 2nd run" +); + +is( + $constraints->{'Table2'}->{constraint_name}, + 'fkey2', + "Child table : constraint name removed underscore after 2nd run" +); + + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; diff --git a/t/pt-online-schema-change/samples/bug-1215587.sql b/t/pt-online-schema-change/samples/bug-1215587.sql new file mode 100644 index 00000000..1e6a5152 --- /dev/null +++ b/t/pt-online-schema-change/samples/bug-1215587.sql @@ -0,0 +1,23 @@ +/* ----- Create two test tables with FKs for scenario 1 and 2: ----- */ +drop database if exists bug1215587; +CREATE DATABASE bug1215587; +USE bug1215587; + +CREATE TABLE IF NOT EXISTS `Table1` ( + `ID` int unsigned NOT NULL AUTO_INCREMENT, + `T2ID` smallint unsigned DEFAULT NULL, + PRIMARY KEY (`ID`), + KEY `tagIndex` (`T2ID`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +CREATE TABLE `Table2` ( + `ID` smallint unsigned NOT NULL AUTO_INCREMENT, + PRIMARY KEY (`ID`) +) ENGINE=InnoDB DEFAULT CHARSET=latin1; + +ALTER TABLE `Table1` + ADD CONSTRAINT `fkey1` FOREIGN KEY (`T2ID`) REFERENCES `Table2` (`ID`) ON DELETE NO ACTION; + +ALTER TABLE `Table2` + ADD CONSTRAINT `fkey2` FOREIGN KEY (`ID`) REFERENCES `Table1` (`T2ID`) ON DELETE NO ACTION; +