diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 91b01823..e3457327 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -7804,7 +7804,7 @@ sub main { PTDEBUG && _d("Renamed columns (old => new): ", Dumper(\%renamed_cols)); if ( %renamed_cols && $o->get('check-alter') ) { # sort is just for making output consistent for testing - my $msg = "--alter appears to be rename these columns: " + my $msg = "--alter appears to rename these columns: " . join(", ", map { "$_ to $renamed_cols{$_}" } sort keys %renamed_cols); if ( $o->get('dry-run') ) { @@ -7813,9 +7813,9 @@ sub main { else { die $msg . ". The tool should handle this correctly, but you should " - . " test it first because if it fails the renamed columns' " - . " data will be lost! Specify --no-check-alter to disable " - . " this check and perform the --alter.\n"; + . "test it first because if it fails the renamed columns' " + . "data will be lost! Specify --no-check-alter to disable " + . "this check and perform the --alter.\n"; } } } @@ -7840,13 +7840,27 @@ sub main { my $col_posn = $orig_tbl->{tbl_struct}->{col_posn}; my $orig_cols = $orig_tbl->{tbl_struct}->{is_col}; my $new_cols = $new_tbl->{tbl_struct}->{is_col}; - @renamed_cols{keys %$new_cols} = keys %$new_cols; - my @sorted_cols = sort { $col_posn->{$a} <=> $col_posn->{$b} } + my @common_cols = map { +{ old => $_, new => $renamed_cols{$_} || $_ } } + sort { $col_posn->{$a} <=> $col_posn->{$b} } + grep { $new_cols->{$_} || $renamed_cols{$_} } keys %$orig_cols; - my @old_cols = grep { $renamed_cols{$_} } @sorted_cols; - my @new_cols = map { $renamed_cols{$_} ? $renamed_cols{$_} : () } - @sorted_cols; - PTDEBUG && _d('New columns', @new_cols); + PTDEBUG && _d('Common columns', Dumper(\@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 @@ -7877,10 +7891,7 @@ sub main { create_triggers( orig_tbl => $orig_tbl, new_tbl => $new_tbl, - columns => { - old_columns => \@old_cols, - new_columns => \@new_cols, - }, + columns => \@common_cols, Cxn => $cxn, Quoter => $q, OptionParser => $o, @@ -8152,10 +8163,10 @@ sub main { # NibbleIterator combines these two statements and adds # "FROM $orig_table->{name} WHERE ". - my $dml = "INSERT LOW_PRIORITY IGNORE INTO $new_tbl->{name} " - . "(" . join(', ', map { $q->quote($_) } @new_cols) . ") " - . "SELECT"; - my $select = join(', ', map { $q->quote($_) } @old_cols); + my $dml = "INSERT LOW_PRIORITY IGNORE INTO $new_tbl->{name} " + . "(" . join(', ', map { $q->quote($_->{new}) } @common_cols) . ") " + . "SELECT"; + my $select = join(', ', map { $q->quote($_->{old}) } @common_cols); # The chunk size is auto-adjusted, so use --chunk-size as # the initial value, but then save and update the adjusted @@ -8690,22 +8701,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 } @@ -8951,31 +8946,29 @@ sub create_triggers { $prefix = $truncated_prefix; } - # new_columns and old_columns are probably the same, unless the - # user is doing a CHANGE COLUMN col col_different_name - my %renamed_cols; - my @new_cols = @{$cols->{new_columns}}; - my @old_cols = @{$cols->{old_columns}}; - @renamed_cols{@new_cols} = @old_cols; - # 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 $del_index_cols = join(" AND ", - map { - my $col = $q->quote($_); - "$new_tbl->{name}.$col <=> OLD.$renamed_cols{$_}" - } @{$tbl_struct->{keys}->{$del_index}->{cols}} ); + # unique indexes can be nullable. Cols are from the new table and + # they may have been renamed + my %old_col_for = map { $_->{new} => $_->{old} } @$cols; + my $tbl_struct = $new_tbl->{tbl_struct}; + my $del_index = $new_tbl->{del_index}; + my $del_index_cols = join(" AND ", map { + my $new_col = $_; + my $old_col = $old_col_for{$new_col} || $new_col; + my $new_qcol = $q->quote($new_col); + my $old_qcol = $q->quote($old_col); + "$new_tbl->{name}.$new_qcol <=> OLD.$old_qcol" + } @{$tbl_struct->{keys}->{$del_index}->{cols}} ); + my $delete_trigger = "CREATE TRIGGER `${prefix}_del` AFTER DELETE ON $orig_tbl->{name} " . "FOR EACH ROW " . "DELETE IGNORE FROM $new_tbl->{name} " . "WHERE $del_index_cols"; - my $qcols = join(', ', map { $q->quote($_) } @new_cols); - my $new_vals = join(', ', map { "NEW.".$q->quote($_) } @old_cols); + my $qcols = join(', ', map { $q->quote($_->{new}) } @$cols); + my $new_vals = join(', ', map { "NEW.".$q->quote($_->{old}) } @$cols); my $insert_trigger = "CREATE TRIGGER `${prefix}_ins` AFTER INSERT ON $orig_tbl->{name} " . "FOR EACH ROW " diff --git a/t/pt-online-schema-change/bugs.t b/t/pt-online-schema-change/bugs.t index e759a8f2..c62ebe67 100644 --- a/t/pt-online-schema-change/bugs.t +++ b/t/pt-online-schema-change/bugs.t @@ -213,7 +213,7 @@ ok( like( $output, - qr/--no-check-alter to disable this error/, + qr/Specify --no-check-alter to disable this check/, "--check-alter works" );