Merge lp:~percona-toolkit-dev/percona-toolkit/fix-pt-osc-prefix-bug-1215608 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.5

[Daniel Nichter]

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-online-schema-change'
--- bin/pt-online-schema-change	2013-10-09 20:23:10 +0000
+++ bin/pt-online-schema-change	2013-10-10 02:14:57 +0000
@@ -8430,19 +8430,27 @@
    # Step 1: Create the new table.
    # #####################################################################
 
+   my $new_table_name   = $o->get('new-table-name');
+   my $new_table_prefix = $o->got('new-table-name') ? undef : '_';
+
    # --plugin hook
-   if ( $plugin && $plugin->can('before_create_new_table') ) {
-      $plugin->before_create_new_table();
+  if ( $plugin && $plugin->can('before_create_new_table') ) {
+      $plugin->before_create_new_table(
+            new_table_name   => $new_table_name,
+            new_table_prefix => $new_table_prefix,
+      );
    }
 
    my $new_tbl;
    eval {
       $new_tbl = create_new_table(
-         orig_tbl     => $orig_tbl,
-         Cxn          => $cxn,
-         Quoter       => $q,
-         OptionParser => $o,
-         TableParser  => $tp,
+         new_table_name   => $new_table_name,
+         new_table_prefix => $new_table_prefix,
+         orig_tbl         => $orig_tbl,
+         Cxn              => $cxn,
+         Quoter           => $q,
+         OptionParser     => $o,
+         TableParser      => $tp,
       );
    };
    if ( $EVAL_ERROR ) {
@@ -9528,11 +9536,12 @@
 
 sub create_new_table {
    my (%args) = @_;
-   my @required_args = qw(orig_tbl Cxn Quoter OptionParser TableParser);
+   my @required_args = qw(new_table_name orig_tbl Cxn Quoter OptionParser TableParser);
    foreach my $arg ( @required_args ) {
       die "I need a $arg argument" unless $args{$arg};
    }
-   my ($orig_tbl, $cxn, $q, $o, $tp) = @args{@required_args};
+   my ($new_table_name, $orig_tbl, $cxn, $q, $o, $tp) = @args{@required_args};
+   my $new_table_prefix = $args{new_table_prefix};
 
    # Get the original table struct.
    my $ddl = $tp->get_create_table(
@@ -9541,25 +9550,23 @@
       $orig_tbl->{tbl},
    );
 
-   my $tries      = $args{tries}  || 10;  # don't try forever
-   my $prefix     = $args{prefix} || '_';
-   my $suffix     = '_new';
-   my $table_name = $orig_tbl->{tbl} . $suffix;
+   $new_table_name =~ s/%T/$orig_tbl->{tbl}/;
 
    print "Creating new table...\n";
+   my $tries = $new_table_prefix ? 10 : 1;
    my $tryno = 1;
    my @old_tables;
-   while ( $tryno++ < $tries ) {
-      $table_name = $prefix . $table_name;
+   while ( $tryno++ <= $tries ) {
+      if ( $new_table_prefix ) {
+         $new_table_name = $new_table_prefix . $new_table_name;
+      }
 
-      if ( length($table_name) > 64 ) {
-         my $truncated_table_name = substr($table_name, 0, 60) . $suffix;
-         PTDEBUG && _d($table_name, 'is over 64 characters long, truncating to',
-                       $truncated_table_name);
-         $table_name = $truncated_table_name;
+      if ( length($new_table_name) > 64 ) {
+         my $truncated_table_name = substr($new_table_name, 0, 64);
+         PTDEBUG && _d($new_table_name, 'is over 64 characters long, '
+            . 'truncating to', $truncated_table_name);
+         $new_table_name = $truncated_table_name;
       }
-      
-      my $quoted  = $q->quote($orig_tbl->{db}, $table_name);
 
       # Generate SQL to create the new table.  We do not use CREATE TABLE LIKE
       # because it doesn't preserve foreign key constraints. Here we need to
@@ -9572,6 +9579,7 @@
       # or another table, we can still have a collision. But if there are
       # multiple FKs on this table, it's hard to know which one is causing the
       # trouble. Should we generate random/UUID FK names or something instead?
+      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;
@@ -9590,7 +9598,7 @@
          # to have a similarly named table created by the user for other
          # purposes.
          if ( $EVAL_ERROR =~ m/table.+?already exists/i ) {
-            push @old_tables, $q->quote($orig_tbl->{db}, $table_name);
+            push @old_tables, $q->quote($orig_tbl->{db}, $new_table_name);
             next;
          }
 
@@ -9598,11 +9606,11 @@
          die $EVAL_ERROR;
       }
       print $sql, "\n" if $o->get('print');  # the sql that work
-      print "Created new table $orig_tbl->{db}.$table_name OK.\n";
+      print "Created new table $orig_tbl->{db}.$new_table_name OK.\n";
       return { # success
          db   => $orig_tbl->{db},
-         tbl  => $table_name,
-         name => $q->quote($orig_tbl->{db}, $table_name),
+         tbl  => $new_table_name,
+         name => $q->quote($orig_tbl->{db}, $new_table_name),
       };
    }
 
@@ -10943,8 +10951,8 @@
 Drop the new table if copying the original table fails.
 
 Specifying C<--no-drop-new-table> and C<--no-swap-tables> leaves the new,
-altered copy of the table without modifying the original table.  The new
-table name is like C<_TBL_new> where C<TBL> is the table name.
+altered copy of the table without modifying the original table.  See
+L<"--new-table-name">.
 
 L<--no-drop-new-table> does not work with
 C<alter-foreign-keys-method drop_swap>.
@@ -11038,6 +11046,16 @@
 however; it will only give the server a chance to recover from the queueing.  If
 you notice queueing, it is best to decrease the chunk time.
 
+=item --new-table-name
+
+type: string; default: %T_new
+
+New table name before it is swapped.  C<%T> is replaced with the original
+table name.  When the default is used, the tool prefixes the name with up
+to 10 C<_> (underscore) to find a unique table name.  If a table name is
+specified, the tool does not prefix it with C<_>, so the table must not
+exist.
+
 =item --password
 
 short form: -p; type: string

=== modified file 't/pt-online-schema-change/basics.t'
--- t/pt-online-schema-change/basics.t	2013-10-09 20:23:10 +0000
+++ t/pt-online-schema-change/basics.t	2013-10-10 02:14:57 +0000
@@ -134,7 +134,7 @@
    # should still exist, but not yet, so add it to the list so
    # is_deeply() against $new_tbls passes.  This only works for
    # single-table tests.
-   my $new_tbl = "_${tbl}_new";
+   my $new_tbl = $args{new_table} || "_${tbl}_new";
    if ( grep { $_ eq '--no-drop-new-table' } @$cmds ) {
       unshift @$orig_tbls, [$new_tbl];
    }
@@ -739,6 +739,34 @@
 }
 
 # #############################################################################
+# --new-table-name
+# #############################################################################
+
+test_alter_table(
+   name       => "--new-table-name %T_foo",
+   table      => "pt_osc.t",
+   file       => "basic_no_fks_innodb.sql",
+   max_id     => 20,
+   test_type  => "add_col",
+   new_col    => "foo",
+   cmds       => [
+      qw(--execute --new-table-name %T_foo), '--alter', 'ADD COLUMN foo INT'
+   ],
+);
+
+test_alter_table(
+   name       => "--new-table-name static_new",
+   table      => "pt_osc.t",
+   max_id     => 20,
+   test_type  => "drop_col",
+   drop_col   => "foo",
+   new_table  => 'static_new',
+   cmds       => [
+      qw(--execute --new-table-name static_new), '--alter', 'DROP COLUMN foo'
+   ],
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($master_dbh);