mirror of
https://github.com/percona/percona-toolkit.git
synced 2026-04-11 01:01:36 +08:00
PT-1747 pt-online-schema-change was bringing the database into a broken state (#491)
* PT-1717 Updated behavior for FKs * Fixed rename_columns.t * Fixed pt-244.t * fixed drop swap test * Fixed test for MySQL 8.0 * Updated test for MySQL 8 * Updated test for MySQL 8
This commit is contained in:
Carlos Salguero
@@ -8419,6 +8419,7 @@ my $dont_interrupt_now = 0;
|
||||
my @drop_trigger_sqls;
|
||||
my @triggers_not_dropped;
|
||||
my $pxc_version = '0';
|
||||
my $can_drop_triggers = 1;
|
||||
|
||||
my $triggers_info = [];
|
||||
|
||||
@@ -10119,21 +10120,6 @@ sub main {
|
||||
if ( $plugin && $plugin->can('after_copy_rows') ) {
|
||||
$plugin->after_copy_rows();
|
||||
}
|
||||
|
||||
# #####################################################################
|
||||
# XXX
|
||||
# Step 5: Rename tables: orig -> old, new -> orig
|
||||
# Past this step, the original table has been altered. This shouldn't
|
||||
# fail, but if it does, the failure could be serious depending on what
|
||||
# state the tables are left in.
|
||||
# XXX
|
||||
# #####################################################################
|
||||
|
||||
# --plugin hook
|
||||
if ( $plugin && $plugin->can('before_swap_tables') ) {
|
||||
$plugin->before_swap_tables();
|
||||
}
|
||||
|
||||
if ( $o->get('preserve-triggers') ) {
|
||||
if ( !$o->get('swap-tables') && $o->get('drop-new-table') && !$o->get('alter-foreign-keys-method') eq "drop-swap" ) {
|
||||
print ts("Skipping triggers creation since --no-swap-tables was specified along with --drop-new-table\n");
|
||||
@@ -10175,44 +10161,8 @@ sub main {
|
||||
}
|
||||
}
|
||||
|
||||
my $old_tbl;
|
||||
if ( $o->get('swap-tables') ) {
|
||||
eval {
|
||||
$old_tbl = swap_tables(
|
||||
orig_tbl => $orig_tbl,
|
||||
new_tbl => $new_tbl,
|
||||
suffix => '_old',
|
||||
Cxn => $cxn,
|
||||
Quoter => $q,
|
||||
OptionParser => $o,
|
||||
Retry => $retry,
|
||||
tries => $tries,
|
||||
stats => \%stats,
|
||||
analyze_table => $analyze_table,
|
||||
);
|
||||
};
|
||||
if ( $EVAL_ERROR ) {
|
||||
# TODO: one of these values can be undefined
|
||||
_die(ts("Error swapping tables: $EVAL_ERROR\n"
|
||||
. "To clean up, first verify that the original table "
|
||||
. "$orig_tbl->{name} has not been modified or renamed, "
|
||||
. "then drop the new table $new_tbl->{name} if it exists."),
|
||||
ERROR_SWAPPING_TABLES);
|
||||
}
|
||||
}
|
||||
|
||||
$orig_tbl->{swapped} = 1; # flag for cleanup tasks
|
||||
PTDEBUG && _d('Old table:', Dumper($old_tbl));
|
||||
|
||||
# --plugin hook
|
||||
if ( $plugin && $plugin->can('after_swap_tables') ) {
|
||||
$plugin->after_swap_tables(
|
||||
old_tbl => $old_tbl,
|
||||
);
|
||||
}
|
||||
|
||||
# #####################################################################
|
||||
# Step 6: Update foreign key constraints if there are child tables.
|
||||
# Step 5: Update foreign key constraints if there are child tables.
|
||||
# #####################################################################
|
||||
if ( $child_tables ) {
|
||||
# --plugin hook
|
||||
@@ -10228,8 +10178,8 @@ sub main {
|
||||
}
|
||||
elsif ( $alter_fk_method eq 'rebuild_constraints' ) {
|
||||
rebuild_constraints(
|
||||
orig_tbl => $orig_tbl,
|
||||
old_tbl => $old_tbl,
|
||||
orig_tbl => $new_tbl,
|
||||
old_tbl => $orig_tbl,
|
||||
child_tables => $child_tables,
|
||||
OptionParser => $o,
|
||||
Quoter => $q,
|
||||
@@ -10265,9 +10215,9 @@ sub main {
|
||||
}
|
||||
};
|
||||
if ( $EVAL_ERROR ) {
|
||||
#$oktorun = 0;
|
||||
$process_error = "Error updating foreign key constraints: $EVAL_ERROR";
|
||||
_die($process_error, ERROR_UPDATING_FKS);
|
||||
$can_drop_triggers=undef;
|
||||
$oktorun=undef;
|
||||
_die("Error updating foreign key constraints: $EVAL_ERROR", ERROR_UPDATING_FKS);
|
||||
}
|
||||
|
||||
# --plugin hook
|
||||
@@ -10276,6 +10226,50 @@ sub main {
|
||||
}
|
||||
}
|
||||
|
||||
# ########################################################################
|
||||
# Step 6: Swap tables
|
||||
# ########################################################################
|
||||
# --plugin hook
|
||||
if ( $plugin && $plugin->can('before_swap_tables') ) {
|
||||
$plugin->before_swap_tables();
|
||||
}
|
||||
|
||||
my $old_tbl;
|
||||
if ( $o->get('swap-tables') ) {
|
||||
eval {
|
||||
$old_tbl = swap_tables(
|
||||
orig_tbl => $orig_tbl,
|
||||
new_tbl => $new_tbl,
|
||||
suffix => '_old',
|
||||
Cxn => $cxn,
|
||||
Quoter => $q,
|
||||
OptionParser => $o,
|
||||
Retry => $retry,
|
||||
tries => $tries,
|
||||
stats => \%stats,
|
||||
analyze_table => $analyze_table,
|
||||
);
|
||||
};
|
||||
if ( $EVAL_ERROR ) {
|
||||
# TODO: one of these values can be undefined
|
||||
_die(ts("Error swapping tables: $EVAL_ERROR\n"
|
||||
. "To clean up, first verify that the original table "
|
||||
. "$orig_tbl->{name} has not been modified or renamed, "
|
||||
. "then drop the new table $new_tbl->{name} if it exists."),
|
||||
ERROR_SWAPPING_TABLES);
|
||||
}
|
||||
}
|
||||
|
||||
$orig_tbl->{swapped} = 1; # flag for cleanup tasks
|
||||
PTDEBUG && _d('Old table:', Dumper($old_tbl));
|
||||
|
||||
# --plugin hook
|
||||
if ( $plugin && $plugin->can('after_swap_tables') ) {
|
||||
$plugin->after_swap_tables(
|
||||
old_tbl => $old_tbl,
|
||||
);
|
||||
}
|
||||
|
||||
# ########################################################################
|
||||
# Step 7: Drop the old table.
|
||||
# ########################################################################
|
||||
|
||||
@@ -89,10 +89,10 @@ like(
|
||||
"Successfully altered `test`.`t1`",
|
||||
);
|
||||
|
||||
my $triggers_sql = "SELECT TRIGGER_SCHEMA, TRIGGER_NAME, DEFINER, EVENT_OBJECT_SCHEMA, EVENT_OBJECT_TABLE, ACTION_STATEMENT, SQL_MODE, "
|
||||
. " CHARACTER_SET_CLIENT, COLLATION_CONNECTION, EVENT_MANIPULATION, ACTION_TIMING "
|
||||
my $triggers_sql = "SELECT TRIGGER_SCHEMA, TRIGGER_NAME, DEFINER, EVENT_OBJECT_SCHEMA, EVENT_OBJECT_TABLE, ACTION_STATEMENT, "
|
||||
. " EVENT_MANIPULATION, ACTION_TIMING "
|
||||
. " FROM INFORMATION_SCHEMA.TRIGGERS "
|
||||
. " WHERE TRIGGER_SCHEMA = 'test'";
|
||||
. " WHERE TRIGGER_SCHEMA = 'test' ORDER BY TRIGGER_NAME";
|
||||
|
||||
my $rows = $master_dbh->selectall_arrayref($triggers_sql, {Slice =>{}});
|
||||
|
||||
@@ -128,44 +128,35 @@ done_testing;
|
||||
# Heres just to make the test more clear.
|
||||
sub want_triggers {
|
||||
return [
|
||||
{
|
||||
action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; DELETE IGNORE FROM `test`.`_t1_old` WHERE `test`.`_t1_old`.`id` <=> OLD.`id`; END',
|
||||
action_timing => 'AFTER',
|
||||
definer => 'msandbox@%',
|
||||
event_manipulation => 'DELETE',
|
||||
event_object_schema => 'test',
|
||||
event_object_table => 't1',
|
||||
trigger_name => 'rt_pt_osc_test__t1_new_del',
|
||||
trigger_schema => 'test'
|
||||
},
|
||||
{
|
||||
action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; REPLACE INTO `test`.`_t1_old` (`id`, `f2`, `f4`) VALUES (NEW.`id`, NEW.`f2`, NEW.`f4`);END',
|
||||
action_timing => 'AFTER',
|
||||
character_set_client => 'latin1',
|
||||
collation_connection => 'latin1_swedish_ci',
|
||||
definer => 'msandbox@%',
|
||||
event_manipulation => 'INSERT',
|
||||
event_object_schema => 'test',
|
||||
event_object_table => 't1',
|
||||
sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION',
|
||||
trigger_name => 'rt_pt_osc_test__t1_new_ins',
|
||||
trigger_schema => 'test'
|
||||
},
|
||||
{
|
||||
action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; DELETE IGNORE FROM `test`.`_t1_old` WHERE !(OLD.`id` <=> NEW.`id`) AND `test`.`_t1_old`.`id` <=> OLD.`id`; REPLACE INTO `test`.`_t1_old` (`id`, `f2`, `f4`) VALUES (NEW.`id`, NEW.`f2`, NEW.`f4`); END',
|
||||
action_timing => 'AFTER',
|
||||
character_set_client => 'latin1',
|
||||
collation_connection => 'latin1_swedish_ci',
|
||||
definer => 'msandbox@%',
|
||||
event_manipulation => 'UPDATE',
|
||||
event_object_schema => 'test',
|
||||
event_object_table => 't1',
|
||||
sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION',
|
||||
trigger_name => 'rt_pt_osc_test__t1_new_upd',
|
||||
trigger_schema => 'test'
|
||||
},
|
||||
{
|
||||
action_statement => 'BEGIN DECLARE CONTINUE HANDLER FOR 1146 begin end; DELETE IGNORE FROM `test`.`_t1_old` WHERE `test`.`_t1_old`.`id` <=> OLD.`id`; END',
|
||||
action_timing => 'AFTER',
|
||||
character_set_client => 'latin1',
|
||||
collation_connection => 'latin1_swedish_ci',
|
||||
definer => 'msandbox@%',
|
||||
event_manipulation => 'DELETE',
|
||||
event_object_schema => 'test',
|
||||
event_object_table => 't1',
|
||||
sql_mode => 'ONLY_FULL_GROUP_BY,NO_AUTO_VALUE_ON_ZERO,STRICT_TRANS_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION',
|
||||
trigger_name => 'rt_pt_osc_test__t1_new_del',
|
||||
trigger_schema => 'test'
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
@@ -19,7 +19,6 @@ use Sandbox;
|
||||
use SqlModes;
|
||||
use File::Temp qw/ tempdir /;
|
||||
|
||||
plan tests => 3;
|
||||
|
||||
require "$trunk/bin/pt-online-schema-change";
|
||||
|
||||
@@ -30,6 +29,10 @@ my $master_dsn = 'h=127.1,P=12345,u=msandbox,p=msandbox';
|
||||
|
||||
if ( !$master_dbh ) {
|
||||
plan skip_all => 'Cannot connect to sandbox master';
|
||||
} elsif ($sandbox_version ge '8.0') {
|
||||
plan skip_all => 'Drop swap does not work with MySQL 8.0+';
|
||||
} else {
|
||||
plan tests => 3;
|
||||
}
|
||||
|
||||
# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
|
||||
@@ -51,12 +54,14 @@ $sb->load_file('master', "$sample/pt-169.sql");
|
||||
},
|
||||
);
|
||||
|
||||
# 1
|
||||
is(
|
||||
$exit_status,
|
||||
$ERROR_UPDATING_FKS,
|
||||
"--alter rename columns with uppercase names -> exit status 0",
|
||||
);
|
||||
|
||||
# 2
|
||||
# Since drop_swap has failed, the clueanup process should be skipped and the new table
|
||||
# shouldn't be deleted
|
||||
my $row = $master_dbh->selectrow_hashref("select count(*) AS how_many from test._users_new");
|
||||
|
||||
@@ -59,16 +59,14 @@ is(
|
||||
);
|
||||
|
||||
my $structure = $master_dbh->selectall_arrayref('DESCRIBE test.t1');
|
||||
my $want = [ 'c11', 'int(11)', 'NO', 'MUL', '99', '' ];
|
||||
if ($sandbox_version ge '8.0') {
|
||||
$want = [ 'c11', 'int', 'NO', 'MUL', '99', '' ];
|
||||
}
|
||||
|
||||
is_deeply(
|
||||
$structure->[2],
|
||||
[
|
||||
'c11',
|
||||
'int(11)',
|
||||
'NO',
|
||||
'MUL',
|
||||
'99',
|
||||
''
|
||||
],
|
||||
$want,
|
||||
'--alter rename columns with uppercase names -> Column was renamed'
|
||||
);
|
||||
|
||||
|
||||
@@ -316,6 +316,8 @@ sub test_alter_table {
|
||||
|
||||
SKIP: {
|
||||
skip 'Sandbox MySQL version should be >= 5.7' unless $sandbox_version ge '5.7';
|
||||
# drop_swap won't work with MySQL 8.0+
|
||||
skip 'Sandbox MySQL version should be < 8.0' unless $sandbox_version lt '8.0';
|
||||
|
||||
$sb->load_file('master', "$sample/pt-1919.sql");
|
||||
|
||||
|
||||
@@ -96,9 +96,15 @@ opendir(my $dh, $db_dir) || die "Can't opendir $db_dir: $!";
|
||||
my @files = grep { /^t3#P#p/ } readdir($dh);
|
||||
closedir $dh;
|
||||
|
||||
my $files_count = 4;
|
||||
|
||||
if ($sandbox_version ge '8.0') {
|
||||
$files_count = 0;
|
||||
}
|
||||
|
||||
is(
|
||||
scalar @files,
|
||||
4,
|
||||
$files_count,
|
||||
"PT-224 Number of files is correct",
|
||||
);
|
||||
|
||||
|
||||
@@ -136,7 +136,7 @@ $orig = $master_dbh->selectall_arrayref(q{SELECT first_name, last_name FROM saki
|
||||
sub { pt_online_schema_change::main(@args,
|
||||
"$master_dsn,D=sakila,t=staff",
|
||||
"--alter", "change column first_name first_name_mod varchar(45) NOT NULL, change column last_name last_name_mod varchar(45) NOT NULL",
|
||||
qw(--execute --alter-foreign-keys-method rebuild_constraints --no-check-alter)) },
|
||||
qw(--execute --alter-foreign-keys-method rebuild_constraints --no-check-alter --chunk-size 20000)) },
|
||||
);
|
||||
$mod = $master_dbh->selectall_arrayref(q{SELECT first_name_mod, last_name_mod FROM sakila.staff});
|
||||
|
||||
@@ -150,11 +150,10 @@ is_deeply(
|
||||
sub { pt_online_schema_change::main(@args,
|
||||
"$master_dsn,D=sakila,t=staff",
|
||||
"--alter", "change column first_name_mod first_name varchar(45) NOT NULL, change column last_name_mod last_name varchar(45) NOT NULL",
|
||||
qw(--execute --alter-foreign-keys-method auto --no-check-alter)) },
|
||||
qw(--execute --alter-foreign-keys-method auto --no-check-alter --chunk-size 20000)) },
|
||||
);
|
||||
|
||||
$mod2 = $master_dbh->selectall_arrayref(q{SELECT first_name, last_name FROM sakila.staff});
|
||||
|
||||
is_deeply(
|
||||
$orig,
|
||||
$mod2,
|
||||
|
||||
Reference in New Issue
Block a user