From 77485efdbcea8dfb96fadab17e41ce6c13c8fe06 Mon Sep 17 00:00:00 2001 From: Daniel Nichter Date: Thu, 11 Oct 2012 15:20:53 -0600 Subject: [PATCH] Use new tbl indexes to create delete trigger. --- bin/pt-online-schema-change | 36 +++++++-------- t/pt-online-schema-change/bugs.t | 44 ++++++++++++++++++- .../samples/del-trg-bug-1062324.sql | 24 ++++++++++ t/pt-online-schema-change/sanity_checks.t | 4 +- 4 files changed, 87 insertions(+), 21 deletions(-) create mode 100644 t/pt-online-schema-change/samples/del-trg-bug-1062324.sql diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 882fd1bd..60642b39 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -7857,6 +7857,22 @@ sub main { keys %$orig_cols; PTDEBUG && _d('Common columns', @common_cols); + # Find a pk or unique index to use for the delete trigger. can_nibble() + # above returns an index, but NibbleIterator will use non-unique indexes, + # so we have to do this again here. + my $indexes = $new_tbl->{tbl_struct}->{keys}; # brevity + foreach my $index ( $tp->sort_indexes($new_tbl->{tbl_struct}) ) { + if ( $index eq 'PRIMARY' || $indexes->{$index}->{is_unique} ) { + PTDEBUG && _d('Delete trigger index:', Dumper($index)); + $new_tbl->{del_index} = $index; + last; + } + } + if ( !$new_tbl->{del_index} ) { + die "The new table $new_tbl->{name} does not have a PRIMARY KEY " + . "or a unique index which is required for the DELETE trigger.\n"; + } + # ######################################################################## # Step 3: Create the triggers to capture changes on the original table and # apply them to the new table. @@ -8651,22 +8667,6 @@ sub check_orig_table { die "Cannot chunk the original table $orig_tbl->{name}: $EVAL_ERROR\n"; } - # Find a pk or unique index to use for the delete trigger. can_nibble() - # above returns an index, but NibbleIterator will use non-unique indexes, - # so we have to do this again here. - my $indexes = $orig_tbl->{tbl_struct}->{keys}; # brevity - foreach my $index ( $tp->sort_indexes($orig_tbl->{tbl_struct}) ) { - if ( $index eq 'PRIMARY' || $indexes->{$index}->{is_unique} ) { - PTDEBUG && _d('Delete trigger index:', Dumper($index)); - $orig_tbl->{del_index} = $index; - last; - } - } - if ( !$orig_tbl->{del_index} ) { - die "The original table $orig_tbl->{name} does not have a PRIMARY KEY " - . "or a unique index which is required for the DELETE trigger.\n"; - } - return; # success } @@ -8915,8 +8915,8 @@ sub create_triggers { # To be safe, the delete trigger must specify all the columns of the # primary key/unique index. We use null-safe equals, because unique # unique indexes can be nullable. - my $tbl_struct = $orig_tbl->{tbl_struct}; - my $del_index = $orig_tbl->{del_index}; + my $tbl_struct = $new_tbl->{tbl_struct}; + my $del_index = $new_tbl->{del_index}; my $del_index_cols = join(" AND ", map { my $col = $q->quote($_); diff --git a/t/pt-online-schema-change/bugs.t b/t/pt-online-schema-change/bugs.t index 296c0844..e99213ff 100644 --- a/t/pt-online-schema-change/bugs.t +++ b/t/pt-online-schema-change/bugs.t @@ -77,7 +77,7 @@ $sb->load_file('master', "$sample/bug-1002448.sql"); unlike $output, - qr/\QThe original table `test1002448`.`table_name` does not have a PRIMARY KEY or a unique index which is required for the DELETE trigger/, + qr/\QThe new table `test1002448`.`_table_name_new` does not have a PRIMARY KEY or a unique index which is required for the DELETE trigger/, "Bug 1002448: mistakenly uses indexes instead of keys"; # ############################################################################ @@ -193,6 +193,48 @@ is_deeply( $master_dbh->do(q{DROP DATABASE IF EXISTS `bug_1041372`}); +# ############################################################################ +# https://bugs.launchpad.net/percona-toolkit/+bug/1062324 +# pt-online-schema-change sets bad DELETE trigger when changing Primary Key +# ############################################################################ +$sb->load_file('master', "$sample/del-trg-bug-1062324.sql"); + +{ + # pt-osc has no --no-drop-triggers option, so we hijack its sub. + no warnings; + local *pt_online_schema_change::drop_triggers = sub { return }; + + # Run the tool but leave the original and new tables as-is, + # and leave the triggers. + ($output, $exit_status) = full_output( + sub { pt_online_schema_change::main(@args, + "$master_dsn,D=test,t=t1", + "--alter", "drop key 2bpk, drop key c3, drop primary key, drop c1, add primary key (c2, c3(4)), add key (c3(4))", + qw(--execute --no-drop-new-table --no-swap-tables)) }, + ); + + # Since _t1_new no longer has the c1 column, the bug caused this + # query to throw "ERROR 1054 (42S22): Unknown column 'test._t1_new.c1' + # in 'where clause'". + eval { + $master_dbh->do("DELETE FROM test.t1 WHERE c1=1"); + }; + is( + $EVAL_ERROR, + "", + "No delete trigger error after altering PK (bug 1062324)" + ) or diag($output); + + # The original row was (c1,c2,c3) = (1,1,1). We deleted where c1=1, + # so the row where c2=1 AND c3=1 should no longer exist. + my $row = $master_dbh->selectrow_arrayref("SELECT * FROM test._t1_new WHERE c2=1 AND c3=1"); + is( + $row, + undef, + "Delete trigger works after altering PK (bug 1062324)" + ); +} + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-online-schema-change/samples/del-trg-bug-1062324.sql b/t/pt-online-schema-change/samples/del-trg-bug-1062324.sql new file mode 100644 index 00000000..f96bcb78 --- /dev/null +++ b/t/pt-online-schema-change/samples/del-trg-bug-1062324.sql @@ -0,0 +1,24 @@ +drop database if exists test; +create database test; +use test; + +CREATE TABLE `t1` ( + `c1` bigint(20) unsigned NOT NULL AUTO_INCREMENT, + `c2` bigint(20) unsigned DEFAULT NULL, + `c3` binary(20) DEFAULT NULL, + PRIMARY KEY (`c1`), + UNIQUE KEY `2bpk` (`c2`,`c3`), + KEY `c3` (`c3`) +) ENGINE=InnoDB; + +INSERT INTO t1 VALUES + (null, 1, 1), + (null, 1, 2), + (null, 1, 3), + (null, 1, 4), + (null, 1, 5), + (null, 2, 1), + (null, 2, 2), + (null, 2, 3), + (null, 2, 4), + (null, 2, 5); diff --git a/t/pt-online-schema-change/sanity_checks.t b/t/pt-online-schema-change/sanity_checks.t index a2cdc9fb..618c9350 100644 --- a/t/pt-online-schema-change/sanity_checks.t +++ b/t/pt-online-schema-change/sanity_checks.t @@ -92,8 +92,8 @@ $master_dbh->do("ALTER TABLE pt_osc.t DROP INDEX c"); ); like( $output, - qr/`pt_osc`.`t` does not have a PRIMARY KEY or a unique index/, - "Original table must have a PK or unique index" + qr/`pt_osc`.`_t_new` does not have a PRIMARY KEY or a unique index/, + "New table must have a PK or unique index" ); # #############################################################################