PT-91 Added --preserve-triggers feature for MySQL 5.7+

bin/pt-online-schema-change

@@ -14,6 +14,7 @@ use warnings FATAL => 'all';
 BEGIN {
    $INC{$_} = __FILE__ for map { (my $pkg = "$_.pm") =~ s!::!/!g; $pkg } (qw(
       Percona::Toolkit
+      VersionCompare
       OptionParser
       Lmo::Utils
       Lmo::Meta
@@ -106,6 +107,54 @@ sub _d {
 # ###########################################################################
 # End Percona::Toolkit package
 # ###########################################################################
+#
+
+# ###########################################################################
+# VersionCompare package
+# This package is a copy without comments from the original.  The original
+# with comments and its test file can be found in the Bazaar repository at,
+#   lib/VersionCompare.pm
+#   t/lib/VersionCompare.t
+# See https://launchpad.net/percona-toolkit for more information.
+# ###########################################################################
+{
+package VersionCompare;
+
+use strict;
+use English qw(-no_match_vars);
+use constant PTDEBUG => $ENV{PTDEBUG} || 0;
+
+sub cmp {
+   my ($v1, $v2) = @_;
+
+   $v1 =~ s/[^\d\.]//;
+   $v2 =~ s/[^\d\.]//;
+
+   my @a = ( $v1 =~ /(\d+)\.?/g ); 
+   my @b = ( $v2 =~ /(\d+)\.?/g ); 
+   foreach my $n1 (@a) {
+      $n1 += 0; #convert to number
+      if (!@b) {
+         return 1;
+      }  
+      my $n2 = shift @b;
+      $n2 += 0; # convert to number
+      if ($n1 == $n2) {
+          next;
+      }
+      else {
+         return $n1 <=> $n2;
+      }  
+   }  
+   return @b ? -1 : 0;
+}
+
+
+1;
+}
+# ###########################################################################
+# End VersionCompare package
+# ###########################################################################
 
 # ###########################################################################
 # OptionParser package
@@ -2267,6 +2316,7 @@ sub get_dbh {
       PrintError         => 0,
       ShowErrorStatement => 1,
       mysql_enable_utf8 => ($cxn_string =~ m/charset=utf8/i ? 1 : 0),
+      #      mysql_multi_statements => 1,
    };
    @{$defaults}{ keys %$opts } = values %$opts;
    if (delete $defaults->{L}) { # L for LOAD DATA LOCAL INFILE, our own extension
@@ -8112,6 +8162,7 @@ use constant PTDEBUG => $ENV{PTDEBUG} || 0;
 use List::Util qw(max);
 use Time::HiRes qw(time sleep);
 use Data::Dumper;
+use VersionCompare;
 $Data::Dumper::Indent    = 1;
 $Data::Dumper::Sortkeys  = 1;
 $Data::Dumper::Quotekeys = 0;
@@ -8133,6 +8184,9 @@ my $dont_interrupt_now = 0;
 my @drop_trigger_sqls;
 my @triggers_not_dropped;
 my $pxc_version = '0';
+
+my $triggers_info = [];
+
 # Completely ignore these error codes.
 my %ignore_code = (
    # Error: 1592 SQLSTATE: HY000  (ER_BINLOG_UNSAFE_STATEMENT)
@@ -8238,6 +8292,35 @@ sub main {
       $o->set('drop-old-table', 0);
    }
 
+   if ( !$o->get('drop-triggers') && $o->get('preserve-triggers') ) {
+      my $msg = "Cannot use --no-drop-triggers along with --preserve-triggers "
+              . "since --preserve-triggers implies that the old triggers should be deleted"
+              . " and recreated in the new table.\nPlease read the documentation for "
+              . "--preserve-triggers";
+      die $msg;
+   }
+
+   if ( !$o->get('drop-old-table') && $o->get('preserve-triggers') ) {
+      my $msg = "Cannot use --no-drop-old-table and --preserve-triggers together.\n"
+              . "--preserve-triggers implies that the old table and triggers "
+              . " should be deleted and recreated into the new table.\n"
+              . "Please read the documentation for --preserve-triggers";
+      die $msg;
+   }
+
+   if ( !$o->get('swap-tables') && $o->get('preserve-triggers') ) {
+      my $msg = "Canot use --no-swap-tables with --preserve-triggers since trigger names "
+              . "cannot be duplicated so, the old table and triggers should be deleted "
+              . "and recreated into the new table.\n"
+              . "Please read the documentation for --preserve-triggers";
+      die $msg;
+   }
+
+   if ( $o->get('preserve-triggers') ) {
+      $o->set('drop-triggers', 1);
+   }
+
+
    if ( !$o->get('help') ) {
       if ( @ARGV ) {
          $o->save_error('Specify only one DSN on the command line');
@@ -9261,7 +9344,7 @@ sub main {
              . "To drop the triggers, execute:\n"
              . join("\n", @drop_trigger_sqls) . "\n";
       }
-      elsif ( !$drop_triggers ) {
+      elsif ( !$drop_triggers  ) {
          print "Not dropping triggers because --no-drop-triggers was "
          . "specified.  To drop the triggers, execute:\n"
          . join("\n", @drop_trigger_sqls) . "\n";
@@ -9695,6 +9778,29 @@ sub main {
       $plugin->before_swap_tables();
    }
 
+   if ( $o->get('preserve-triggers') ) {
+      warn "Adding original triggers to new table.\n";
+      foreach my $trigger_info (@$triggers_info) {
+          next if ! ($trigger_info->{orig_triggers});
+          foreach my $orig_trigger (@{$trigger_info->{orig_triggers}}) {
+              my $new_trigger_sqls = create_trigger_sql($orig_trigger, 
+                                                        $new_tbl->{db}, 
+                                                        $new_tbl->{tbl}, 
+                                                        $orig_tbl->{tbl}
+                                                       );
+              next if !$o->get('execute');
+              for my $sql (@$new_trigger_sqls) {
+                  eval {
+                      $cxn->dbh()->do($sql);
+                  };
+                  if ($EVAL_ERROR) {
+                       die "Exiting due to errors while restoring triggers: $EVAL_ERROR";
+                  }
+              }
+          }
+      }
+   }
+
    my $old_tbl;
    if ( $o->get('swap-tables') ) {
 
@@ -10444,15 +10550,22 @@ sub check_orig_table {
       die "The original table $orig_tbl->{name} does not exist.\n";
    }
 
+   my ( $version ) = $dbh->selectrow_array("SELECT VERSION()");
    # There cannot be any triggers on the original table.
    my $sql = 'SHOW TRIGGERS FROM ' . $q->quote($orig_tbl->{db})
            . ' LIKE ' . $q->literal_like($orig_tbl->{tbl});
    PTDEBUG && _d($sql);
    my $triggers = $dbh->selectall_arrayref($sql);
    if ( $triggers && @$triggers ) {
-      die "The table $orig_tbl->{name} has triggers.  This tool "
+       if ( VersionCompare::cmp($version, '5.7.0') < 0  && VersionCompare::cmp($version, '10.0.0') <= 0) {
-         . "needs to create its own triggers, so the table cannot "
+           die "The table $orig_tbl->{name} has triggers.  This tool "
-         . "already have triggers.\n";
+             . "needs to create its own triggers, so the table cannot "
+             . "already have triggers.\n";
+       } elsif ( ( VersionCompare::cmp($version, '5.7.0') >= 0 || VersionCompare::cmp($version, '10.0.0') >0 )
+                 && !$o->get('preserve-triggers') ) {
+           die "The table $orig_tbl->{name} has triggers but you --preserve-triggers was not specified.\n"
+             . "Original triggers will be lost. Please read the documentation for --preserve-triggers.\n";
+       }
    }
 
    # Get the table struct.  NibbleIterator needs this, and so do we.
@@ -10801,6 +10914,8 @@ sub create_triggers {
    # they may have been renamed
    my %old_col_for    = map { $_->{new} => $_->{old} } @$cols;
    my $tbl_struct     = $del_tbl->{tbl_struct};
+   
+   # ---------------------------------------------------------------------------------------
    my $del_index      = $del_tbl->{del_index};
    my $del_index_cols = join(" AND ", map {
       my $new_col  = $_;
@@ -10816,21 +10931,25 @@ sub create_triggers {
       . "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 $insert_trigger
       = "CREATE TRIGGER `${prefix}_ins` AFTER INSERT ON $orig_tbl->{name} "
       . "FOR EACH ROW "
       . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals)";
 
+   # ---------------------------------------------------------------------------------------
    my $upd_index_cols = join(" AND ", map {
-      my $new_col = $_;
+          my $new_col = $_;
-      my $old_col = $old_col_for{$new_col} || $new_col;
+          my $old_col = $old_col_for{$new_col} || $new_col;
-      my $new_qcol = $q->quote($new_col);
+          my $new_qcol = $q->quote($new_col);
-      my $old_qcol = $q->quote($old_col);
+          my $old_qcol = $q->quote($old_col);
-      "OLD.$old_qcol <=> NEW.$new_qcol"
+          "OLD.$old_qcol <=> NEW.$new_qcol"
    } @{$tbl_struct->{keys}->{$del_index}->{cols}} );
-      
+   # ---------------------------------------------------------------------------------------
+
    my $update_trigger
       = "CREATE TRIGGER `${prefix}_upd` AFTER UPDATE ON $orig_tbl->{name} "
       . "FOR EACH ROW "
@@ -10839,36 +10958,110 @@ sub create_triggers {
       . "REPLACE INTO $new_tbl->{name} ($qcols) VALUES ($new_vals);"
       . "END ";      
 
-   my @triggers = (
+   $triggers_info = [                                                     
-      ['del', $delete_trigger],
+      { 
-      ['upd', $update_trigger],
+          suffix => 'del', event => 'DELETE', time => 'AFTER', orig_triggers => [], 
-      ['ins', $insert_trigger],
+          new_trigger_sql => $delete_trigger, new_trigger_name => "${prefix}_del", 
-   );
+      },
+      { 
+          suffix => 'upd', event => 'UPDATE', time => 'AFTER', orig_triggers => [], 
+          new_trigger_sql => $update_trigger, new_trigger_name => "${prefix}_upd",
+      },
+      { 
+          suffix => 'ins', event => 'INSERT', time => 'AFTER', orig_triggers => [], 
+          new_trigger_sql => $insert_trigger, new_trigger_name => "${prefix}_ins", 
+      },
+
+      { 
+          suffix => 'delb', event => 'DELETE', time => 'BEFORE', orig_triggers => [], 
+          new_trigger_sql => '', new_trigger_name => '' 
+      },
+      { 
+          suffix => 'updb', event => 'UPDATE', time => 'BEFORE', orig_triggers => [], 
+          new_trigger_sql => '', new_trigger_name => '' 
+      },
+      { 
+          suffix => 'insb', event => 'INSERT', time => 'BEFORE', orig_triggers => [], 
+          new_trigger_sql => '', new_trigger_name => '' 
+      },
+   ];
+
+   $cxn->connect();
+   my $dbh = $cxn->dbh();
+
+   my $trigger_sql = "SELECT TRIGGER_SCHEMA, TRIGGER_NAME, DEFINER, ACTION_STATEMENT, SQL_MODE, "
+                   . "       CHARACTER_SET_CLIENT, COLLATION_CONNECTION, EVENT_MANIPULATION, ACTION_TIMING "
+                   . "  FROM INFORMATION_SCHEMA.TRIGGERS "
+                   . " WHERE EVENT_MANIPULATION = ? "
+                   . "   AND ACTION_TIMING = ? "
+                   . "   AND TRIGGER_SCHEMA = ? "
+                   . "   AND EVENT_OBJECT_TABLE = ?";
+   foreach my $trigger_info (@$triggers_info) {
+       $trigger_info->{orig_triggers} = $dbh->selectall_arrayref( $trigger_sql, 
+                                                                  { Slice => {} }, 
+                                                                  $trigger_info->{event},
+                                                                  $trigger_info->{time},
+                                                                  $orig_tbl->{db}, 
+                                                                  $orig_tbl->{tbl}
+                                                                ) || [];
+   }
+
+   # If --preserve-triggers was specified, try to create the original triggers into the new table.
+   # We are doing this to ensure the original triggers will work in the new modified table
+   # and we want to know this BEFORE copying all rows from the old table to the new one.
+   if ($o->get('preserve-triggers')) {
+       foreach my $trigger_info (@$triggers_info) {
+           foreach my $orig_trigger (@{$trigger_info->{orig_triggers}}) {
+               my $definer = $orig_trigger->{definer} || '';
+               $definer =~ s/@/`@`/;
+               $definer = "`$definer`" ;
+               
+               my @chars = ("a".."z");
+               my $tmp_trigger_name;
+               $tmp_trigger_name .= $chars[rand @chars] for 1..15;
+
+               my $sql = "CREATE DEFINER=$definer " 
+                       . "TRIGGER `$new_tbl->{db}`.`$tmp_trigger_name` "
+                       . "$orig_trigger->{action_timing} $orig_trigger->{event_manipulation} ON $new_tbl->{tbl}\n"
+                       . "FOR EACH ROW\n"
+                       . $orig_trigger->{action_statement};
+               eval {
+                   $dbh->do($sql);
+               };
+               if ($EVAL_ERROR) {
+                   my $msg = "$EVAL_ERROR.\n"
+                           . "Check if all fields referenced by the trigger still exists "
+                           . "after the operation you are trying to apply";
+                   die ($msg);
+               }
+               $dbh->do("DROP TRIGGER IF EXISTS `$tmp_trigger_name`");
+           }
+       }
+   }
 
    my @trigger_names;
    @drop_trigger_sqls = ();
-   foreach my $trg ( @triggers ) {
+
-      my ($name, $sql) = @$trg;
+   foreach my $trigger_info ( @$triggers_info ) {
-      print $sql, "\n" if $o->get('print');
+       next if !$trigger_info->{new_trigger_sql};
-      if ( $o->get('execute') ) {
+       if ( $o->get('execute') ) {
-         osc_retry(
+          osc_retry(
-            Cxn     => $cxn,
+             Cxn     => $cxn,
-            Retry   => $retry,
+             Retry   => $retry,
-            tries   => $tries->{create_triggers},
+             tries   => $tries->{create_triggers},
-            stats   => $stats,
+             stats   => $stats,
-            code    => sub {
+             code    => sub {
-               PTDEBUG && _d($sql);
+                PTDEBUG && _d($trigger_info->{new_trigger_sql});
-               $cxn->dbh()->do($sql);
+                $cxn->dbh()->do($trigger_info->{new_trigger_sql});
-            },
+             },
-         );
+          );
-      }
+       }
-      # Only save the trigger once it has been created
+       # Only save the trigger once it has been created
-      # (or faked to be created) so if the 2nd trigger
+       # (or faked to be created) so if the 2nd trigger
-      # fails to create, we know to only drop the 1st.
+       # fails to create, we know to only drop the 1st.
-      push @trigger_names, "${prefix}_$name";
+       push @trigger_names, $trigger_info->{new_trigger_name};
-      push @drop_trigger_sqls,
+       push @drop_trigger_sqls,
-         "DROP TRIGGER IF EXISTS "
+          "DROP TRIGGER IF EXISTS " . $q->quote($orig_tbl->{db}, $trigger_info->{new_trigger_name});
-         . $q->quote($orig_tbl->{db}, "${prefix}_$name") . ";";
    }
 
    if ( $o->get('execute') ) {
@@ -10878,6 +11071,42 @@ sub create_triggers {
    return @trigger_names;
 }
 
+sub create_trigger_sql {
+    my ($trigger, $db, $new_table, $orig_table) = @_;
+    my $definer = $trigger->{definer} || '';
+    $definer =~ s/@/`@`/;
+    $definer = "`$definer`" ;
+
+    my $sqls = [];
+    push @$sqls, '/*!50003 SET @saved_sql_mode = @@sql_mode */';
+    push @$sqls, '/*!50003 SET @saved_cs_client      = @@character_set_client */ ;';
+    push @$sqls, '/*!50003 SET @saved_cs_results     = @@character_set_results */ ;';
+    push @$sqls, '/*!50003 SET @saved_col_connection = @@collation_connection */ ;';
+
+    push @$sqls, "/*!50003 SET character_set_client  = $trigger->{character_set_client} */ ;";
+    push @$sqls, "/*!50003 SET collation_connection  = $trigger->{collation_connection} */ ;";
+    push @$sqls, "SET SESSION sql_mode = '$trigger->{sql_mode}'";
+
+    push @$sqls, "LOCK TABLES `$db`.`$new_table` WRITE";
+    push @$sqls, "LOCK TABLES `$db`.`$orig_table` WRITE" if $orig_table;
+    push @$sqls, "DROP TRIGGER IF EXISTS `$db`.`$trigger->{trigger_name}` ";
+
+    push @$sqls, "CREATE DEFINER=$definer " 
+               . "TRIGGER `$db`.`$trigger->{trigger_name}` "
+               . "$trigger->{action_timing} $trigger->{event_manipulation} ON $new_table\n"
+               . "FOR EACH ROW\n"
+               . $trigger->{action_statement};
+
+   push @$sqls, '/*!50003 SET sql_mode              = @saved_sql_mode */ ;';
+   push @$sqls, '/*!50003 SET character_set_client  = @saved_cs_client */ ;';
+   push @$sqls, '/*!50003 SET character_set_results = @saved_cs_results */';
+   push @$sqls, '/*!50003 SET collation_connection  = @saved_col_connection */ ;';
+   push @$sqls, 'UNLOCK TABLES';
+
+   return $sqls;
+    
+}
+
 sub drop_triggers {
    my ( %args ) = @_;
    my @required_args = qw(tbl Cxn Quoter OptionParser Retry tries stats);
@@ -11157,6 +11386,40 @@ sub ts {
    return $msg ? "$ts $msg" : $ts;
 }
 
+# find point in trigger we can insert pt-osc code for --preserve-triggers
+sub trigger_ins_point {
+   my ( %args ) = @_;
+   my @required_args = qw(trigger);
+   foreach my $arg ( @required_args ) {
+      die "I need a $arg argument" unless defined $args{$arg};
+   }
+   my ($trigger) = @args{@required_args};
+
+   my $ins_point;
+   if ($trigger =~ /begin(.*?)end(?!.*end)/igms) {
+         $ins_point = $+[0] - 3;
+      }
+   else { $ins_point = 0;}
+
+   return $ins_point;
+}
+
+# sub to add ; if line doesn't end in ;
+sub terminate_sql {
+   my ( $text ) = @_;
+   die "I need a text argument" unless defined $text;
+   $text = trim($text);
+   if(substr($text, -1) ne ';') { $text .= ';'; }
+   return $text;
+}
+
+sub trim {
+   my ( $text ) = @_;
+   die "I need a text argument" unless defined $text;
+   $text =~ s/^\s+|\s+$//g;
+   return $text;
+}
+
 # Catches signals so we can exit gracefully.
 sub sig_int {
    my ( $signal ) = @_;
@@ -11926,6 +12189,55 @@ until queries are running normally again.  This will not prevent queueing,
 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 --preserve-triggers
+
+Preserves old triggers when specified. 
+As of MySQL 5.7.2, it is possible to define multiple triggers for a given 
+table that have the same trigger event and action time. This allows us to
+add the triggers needed for C<pt-online-schema-change> even if the table
+already has its own triggers.
+If this option is enabled, C<pt-online-schema-change> will try to copy all the
+existing triggers to the new table BEFORE start copying rows from the original
+table to ensure the old triggers can be applied after altering the table.
+
+Example.
+
+  CREATE TABLE test.t1 (
+       id INT NOT NULL AUTO_INCREMENT,
+       f1 INT,
+       f2 VARCHAR(32),
+       PRIMARY KEY (id)
+  );
+              
+  CREATE TABLE test.log (
+     ts  TIMESTAMP,
+     msg VARCHAR(255)
+  );
+  
+  CREATE TRIGGER test.after_update
+   AFTER
+     UPDATE ON test.t1
+     FOR EACH ROW 
+       INSERT INTO test.log VALUES (NOW(), CONCAT("updated row row with id ", OLD.id, " old f1:", OLD.f1, " new f1: ", NEW.f1 ));
+
+For this table and triggers combination, it is not possible to use L<--preserve-triggers> 
+with an L<--alter> like this: C<"DROP COLUMN f1"> since the trigger references the column 
+being dropped and at would make the trigger to fail.
+
+After testing the triggers will work on the new table, the triggers are 
+dropped from the new table until all rows have been copied and then they are
+re-applied. 
+
+B<WARNING> The process of re-applying the triggers is not atomic at it will hold a lock 
+on both tables (original and new) for a short period of time, while the old table is 
+dropped and the new table is being renamed.
+
+L<--preserve-triggers> cannot be used with these other parameters, L<--no-drop-triggers>, 
+L<--no-drop-old-table> and L<--no-swap-tables> since L<--preserve-triggers> implies that 
+the old triggers should be deleted and recreated in the new table. 
+Since it is not possible to have more than one trigger with the same name, old triggers 
+must be deleted in order to be able to recreate them into the new table.
+
 =item --new-table-name
 
 type: string; default: %T_new

t/pt-online-schema-change/basics.t

@@ -0,0 +1,945 @@
+#!/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 Time::HiRes qw(sleep);
+
+$ENV{PTTEST_FAKE_TS} = 1;
+$ENV{PERCONA_TOOLKIT_TEST_USE_DSN_NAMES} = 1;
+
+use PerconaTest;
+use Sandbox;
+require "$trunk/bin/pt-online-schema-change";
+require VersionParser;
+
+use Data::Dumper;
+$Data::Dumper::Indent    = 1;
+$Data::Dumper::Sortkeys  = 1;
+$Data::Dumper::Quotekeys = 0;
+
+my $dp         = new DSNParser(opts=>$dsn_opts);
+my $sb         = new Sandbox(basedir => '/tmp', DSNParser => $dp);
+my $master_dbh = $sb->get_dbh_for('master');
+my $slave_dbh  = $sb->get_dbh_for('slave1');
+
+if ( !$master_dbh ) {
+   plan skip_all => 'Cannot connect to sandbox master';
+}
+elsif ( !$slave_dbh ) {
+   plan skip_all => 'Cannot connect to sandbox slave';
+}
+
+my $q      = new Quoter();
+my $tp     = new TableParser(Quoter => $q);
+my @args   = qw(--set-vars innodb_lock_wait_timeout=3);
+my $output = "";
+my $dsn    = "h=127.1,P=12345,u=msandbox,p=msandbox";
+my $exit   = 0;
+my $sample = "t/pt-online-schema-change/samples";
+my $rows;
+
+# #############################################################################
+# Tool shouldn't run without --execute (bug 933232).
+# #############################################################################
+
+
+$sb->load_file('master', "$sample/basic_no_fks_innodb.sql");
+
+($output, $exit) = full_output(
+   sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=t",
+      '--alter', 'drop column id') }
+);
+
+like(
+   $output,
+   qr/neither --dry-run nor --execute was specified/,
+   "Doesn't run without --execute (bug 933232)"
+) or diag($output);
+
+my $ddl = $master_dbh->selectrow_arrayref("show create table pt_osc.t");
+like(
+   $ddl->[1],
+   qr/^\s+["`]id["`]/m,
+   "Did not alter the table"
+);
+
+is(
+   $exit,
+   1,
+   "Exit 1"
+);
+
+# #############################################################################
+# A helper sub to do the heavy lifting for us.
+# #############################################################################
+
+sub test_alter_table {
+   my (%args) = @_;
+   return if $args{skip};
+
+   my @required_args = qw(name table test_type cmds);
+   foreach my $arg ( @required_args ) {
+      die "I need a $arg argument" unless $args{$arg};
+   }
+   my ($name, $table, $test_type, $cmds) = @args{@required_args};
+
+   my ($db, $tbl) = $q->split_unquote($table);
+   my $table_name = $tbl;
+   my $pk_col     = $args{pk_col} || 'id';
+
+   if ( my $file = $args{file} ) {
+      $sb->load_file('master', "$sample/$file");
+      $master_dbh->do("USE `$db`");
+      $slave_dbh->do("USE `$db`");
+   }
+
+   my $ddl        = $tp->get_create_table($master_dbh, $db, $tbl);
+   my $tbl_struct = $tp->parse($ddl);
+
+   my $cols = '*';
+   if ( $test_type =~ m/(?:add|drop)_col/  && !grep { $_ eq '--dry-run' } @$cmds ) {
+      # Don't select the column being dropped.
+      my $col = $args{drop_col} || $args{new_col};
+      die "I need a drop_col argument" unless $col;
+      $cols = join(', ', grep { $_ ne $col } @{$tbl_struct->{cols}});
+   }
+   my $orig_rows = $master_dbh->selectall_arrayref(
+      "SELECT $cols FROM $table ORDER BY `$pk_col`");
+
+   my $orig_tbls = $master_dbh->selectall_arrayref(
+      "SHOW TABLES FROM `$db`");
+
+   my $orig_max_id = $master_dbh->selectall_arrayref(
+      "SELECT MAX(`$pk_col`) FROM `$db`.`$tbl`");
+
+   my $triggers_sql = "SELECT TRIGGER_SCHEMA, TRIGGER_NAME, DEFINER, ACTION_STATEMENT ".
+                      "  FROM INFORMATION_SCHEMA.TRIGGERS ".
+                       "WHERE TRIGGER_SCHEMA = '$db' " .
+                       "  AND EVENT_OBJECT_TABLE = '$tbl'";
+   my $orig_triggers = $master_dbh->selectall_arrayref($triggers_sql);
+
+   my ($orig_auto_inc) = $ddl =~ m/\s+AUTO_INCREMENT=(\d+)\s+/;
+
+   my $fk_method = $args{check_fks};
+   my @orig_fks;
+   if ( $fk_method ) {
+      foreach my $tbl ( @$orig_tbls ) {
+         my $fks = $tp->get_fks(
+            $tp->get_create_table($master_dbh, $db, $tbl->[0]));
+         push @orig_fks, $fks;
+      }
+   }
+
+   # If --no-drop-new-table is given, then the new, altered table
+   # 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 = $args{new_table} || "_${tbl}_new";
+   if ( grep { $_ eq '--no-drop-new-table' } @$cmds ) {
+      unshift @$orig_tbls, [$new_tbl];
+   }
+
+   ($output, $exit) = full_output(
+      sub { pt_online_schema_change::main(
+         @args,
+         '--print',
+         "$dsn,D=$db,t=$tbl",
+         @$cmds,
+      )},
+      stderr => 1,
+   );
+
+   my $new_ddl = $tp->get_create_table($master_dbh, $db, $tbl);
+   my $new_tbl_struct = $tp->parse($new_ddl);
+   my $fail    = 0;
+
+   is(
+      $exit,
+      0,
+      "$name exit 0"
+   ) or $fail = 1;
+
+   # There should be no new or missing tables.
+   my $new_tbls = $master_dbh->selectall_arrayref("SHOW TABLES FROM `$db`");
+   is_deeply(
+      $new_tbls,
+      $orig_tbls,
+      "$name tables"
+   ) or $fail = 1;
+
+   # Rows in the original and new table should be identical.
+   my $new_rows = $master_dbh->selectall_arrayref("SELECT $cols FROM $table ORDER BY `$pk_col`");
+   is_deeply(
+      $new_rows,
+      $orig_rows,
+      "$name rows"
+   ) or $fail = 1;
+
+   if ( grep { $_ eq '--preserve-triggers' } @$cmds ) {
+      my $new_triggers = $master_dbh->selectall_arrayref($triggers_sql);
+      is_deeply(
+         $new_triggers,
+         $orig_triggers,
+         "$name triggers"
+      ) or $fail = 1;
+   }
+
+   if ( grep { $_ eq '--no-drop-new-table' } @$cmds ) {
+      $new_rows = $master_dbh->selectall_arrayref(
+         "SELECT $cols FROM `$db`.`$new_tbl` ORDER BY `$pk_col`");
+      is_deeply(
+         $new_rows,
+         $orig_rows,
+         "$name new table rows"
+      ) or $fail = 1;
+   }
+
+   my $new_max_id = $master_dbh->selectall_arrayref(
+      "SELECT MAX(`$pk_col`) FROM `$db`.`$tbl`");
+   is(
+      $orig_max_id->[0]->[0],
+      $new_max_id->[0]->[0],
+      "$name MAX(pk_col)"
+   ) or $fail = 1;
+
+   my ($new_auto_inc) = $new_ddl =~ m/\s+AUTO_INCREMENT=(\d+)\s+/;
+   is(
+      $orig_auto_inc,
+      $new_auto_inc,
+      "$name AUTO_INCREMENT=" . ($orig_auto_inc || '<unknown>')
+   ) or $fail = 1;
+
+   # Check if the ALTER was actually done.
+   if ( $test_type eq 'drop_col' ) {
+      my $col = $q->quote($args{drop_col});
+
+      if ( grep { $_ eq '--dry-run' } @$cmds ) {
+         like(
+            $new_ddl,
+            qr/^\s+$col\s+/m,
+            "$name ALTER DROP COLUMN=$args{drop_col} (dry run)"
+         ) or $fail = 1;
+      }
+      else {
+         unlike(
+            $new_ddl,
+            qr/^\s+$col\s+/m,
+            "$name ALTER DROP COLUMN=$args{drop_col}"
+         ) or $fail = 1;
+      }
+   }
+   elsif ( $test_type eq 'add_col' ) {
+      if ( $args{no_change} ) {
+         ok(
+            !$new_tbl_struct->{is_col}->{$args{new_col}},
+            "$name $args{new_col} not added"
+         );
+      }
+      else {
+         ok(
+            $new_tbl_struct->{is_col}->{$args{new_col}},
+            "$name $args{new_col} added"
+         );
+      }
+   }
+   elsif ( $test_type eq 'new_engine' ) {
+      my $new_engine = lc($args{new_engine});
+      die "I need a new_engine argument" unless $new_engine;
+      my $rows = $master_dbh->selectall_hashref(
+         "SHOW TABLE STATUS FROM `$db`", "name");
+      is(
+         lc($rows->{$tbl}->{engine}),
+         $new_engine,
+         "$name ALTER ENGINE=$args{new_engine}"
+      ) or $fail = 1;
+
+   }
+ 
+   if ( $fk_method ) {
+      my @new_fks;
+      my $rebuild_method = 0;
+
+      foreach my $tbl ( @$orig_tbls ) {
+         my $fks = $tp->get_fks(
+            $tp->get_create_table($master_dbh, $db, $tbl->[0]));
+
+         # The tool does not use the same/original fk name,
+         # it appends a single _.  So we need to strip this
+         # to compare the original fks to the new fks.
+         # if ( $fk_method eq 'rebuild_constraints' ) {
+         if ( $fk_method eq 'rebuild_constraints'
+               || $table_name eq $tbl->[0] ) {
+            my %new_fks = map {
+               my $real_fk_name = $_;
+               my $fk_name      = $_;
+               if ( $fk_name =~ s/^_// && $table_name ne $tbl->[0] ) {
+                  $rebuild_method = 1;
+               }
+               $fks->{$real_fk_name}->{name} =~ s/^_//;
+               $fks->{$real_fk_name}->{ddl}  =~ s/`$real_fk_name`/`$fk_name`/;
+               $fk_name => $fks->{$real_fk_name};
+            } keys %$fks;
+            push @new_fks, \%new_fks;
+         }
+         else {
+            # drop_swap
+            push @new_fks, $fks;
+         }
+      }
+
+      if ( grep { $_ eq '--execute' } @$cmds ) {
+         ok(
+              $fk_method eq 'rebuild_constraints' &&  $rebuild_method ? 1
+            : $fk_method eq 'drop_swap'           && !$rebuild_method ? 1
+            :                                                           0,
+            "$name FK $fk_method method"
+         );
+      }
+
+      is_deeply(
+         \@new_fks,
+         \@orig_fks,
+         "$name FK constraints"
+      ) or $fail = 1;
+
+      # Go that extra mile and verify that the fks are actually
+      # still functiona: i.e. that they'll prevent us from delete
+      # a parent row that's being referenced by a child.
+      my $sql = "DELETE FROM $table WHERE $pk_col=1 LIMIT 1";
+      eval {
+         $master_dbh->do($sql);
+      };
+      like(
+         $EVAL_ERROR,
+         qr/foreign key constraint fails/,
+         "$name FK constraints still hold"
+      ) or $fail = 1;
+   }
+
+   if ( $fail ) {
+      diag("Output from failed test:\n$output");
+   }
+   elsif ( $args{output} ) {
+      warn $output;
+   }
+
+   return;
+}
+
+# #############################################################################
+# The most basic: alter a small table with no fks that's not active.
+# #############################################################################
+
+my $db_flavor = VersionParser->new($master_dbh)->flavor();
+if ( $db_flavor =~ m/XtraDB Cluster/ ) {
+   test_alter_table(
+      name       => "Basic no fks --dry-run",
+      table      => "pt_osc.t",
+      file       => "basic_no_fks_innodb.sql",
+      max_id     => 20,
+      test_type  => "drop_col",
+      drop_col   => "d",
+      cmds       => [qw(--dry-run --alter), 'DROP COLUMN d'],
+   );
+
+   test_alter_table(
+      name       => "Basic no fks --execute",
+      table      => "pt_osc.t",
+      # The previous test should not have modified the table.
+      # file       => "basic_no_fks_innodb.sql",
+      # max_id     => 20,
+      test_type  => "drop_col",
+      drop_col   => "d",
+      cmds       => [qw(--execute --alter), 'DROP COLUMN d'],
+   );
+
+   test_alter_table(
+      name       => "--execute but no --alter",
+      table      => "pt_osc.t",
+      file       => "basic_no_fks_innodb.sql",
+       max_id     => 20,
+      test_type  => "new_engine",    # When there's no change, we just check
+      new_engine => "InnoDB",        # the engine as a NOP.  Any other
+      cmds       => [qw(--execute)], # unintended changes are still detected.
+   );
+}
+else {
+   test_alter_table(
+      name       => "Basic no fks --dry-run",
+      table      => "pt_osc.t",
+      file       => "basic_no_fks.sql",
+      max_id     => 20,
+      test_type  => "new_engine",
+      new_engine => "MyISAM",
+      cmds       => [qw(--dry-run --alter ENGINE=InnoDB)],
+   );
+
+   test_alter_table(
+      name       => "Basic no fks --execute",
+      table      => "pt_osc.t",
+      # The previous test should not have modified the table.
+      # file       => "basic_no_fks.sql",
+      # max_id     => 20,
+      test_type  => "new_engine",
+      new_engine => "InnoDB",
+      cmds       => [qw(--execute --alter ENGINE=InnoDB)],
+   );
+
+   test_alter_table(
+      name       => "--execute but no --alter",
+      table      => "pt_osc.t",
+      file       => "basic_no_fks.sql",
+       max_id     => 20,
+      test_type  => "new_engine",
+      new_engine => "MyISAM",
+      cmds       => [qw(--execute)],
+   );
+}
+
+# ############################################################################
+# Alter a table with foreign keys.
+# ############################################################################
+
+# The tables we're loading have fk constraints like:
+# country <-- city <-- address
+
+# rebuild_constraints method -- This parses the fk constraint ddls from
+# the create table ddl, rewrites them, then does an alter table on the
+# child tables so they point back to the original table name.
+
+test_alter_table(
+   name       => "Basic FK rebuild --dry-run",
+   table      => "pt_osc.country",
+   pk_col     => "country_id",
+   file       => "basic_with_fks.sql",
+   test_type  => "drop_col",
+   drop_col   => "last_update",
+   check_fks  => "rebuild_constraints",
+   cmds       => [
+   qw(
+      --dry-run
+      --alter-foreign-keys-method rebuild_constraints
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+test_alter_table(
+   name       => "Basic FK rebuild --execute",
+   table      => "pt_osc.country",
+   pk_col     => "country_id",
+   test_type  => "drop_col",
+   drop_col   => "last_update",
+   check_fks  => "rebuild_constraints",
+   cmds       => [
+   qw(
+      --execute
+      --alter-foreign-keys-method rebuild_constraints
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+# drop_swap method -- This method tricks MySQL by disabling fk checks,
+# then dropping the original table and renaming the new table in its place.
+# Since fk checks were disabled, MySQL doesn't update the child table fk refs.
+# Somewhat dangerous, but quick.  Downside: table doesn't exist for a moment.
+
+test_alter_table(
+   name       => "Basic FK drop_swap --dry-run",
+   table      => "pt_osc.country",
+   pk_col     => "country_id",
+   file       => "basic_with_fks.sql",
+   test_type  => "drop_col",
+   drop_col   => "last_update",
+   check_fks  => "drop_swap",
+   cmds       => [
+   qw(
+      --dry-run
+      --alter-foreign-keys-method drop_swap
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+test_alter_table(
+   name       => "Basic FK drop_swap --execute",
+   table      => "pt_osc.country",
+   pk_col     => "country_id",
+   test_type  => "drop_col",
+   drop_col   => "last_update",
+   check_fks  => "drop_swap",
+   cmds       => [
+   qw(
+      --execute
+      --alter-foreign-keys-method drop_swap
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+# Let the tool auto-determine the fk update method.  This should choose
+# the rebuild_constraints method because the tables are quite small.
+# This is tested by indicating the rebuild_constraints method, which
+# causes the test sub to verify that the fks have leading _; they won't
+# if drop_swap was used.  To verify this, change auto to drop_swap
+# and this test will fail.
+test_alter_table(
+   name        => "Basic FK auto --execute",
+   table       => "pt_osc.country",
+   pk_col      => "country_id",
+   file        => "basic_with_fks.sql",
+   test_type   => "drop_col",
+   drop_col    => "last_update",
+   check_fks   => "rebuild_constraints",
+   cmds        => [
+   qw(
+      --execute
+      --alter-foreign-keys-method auto
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+# Specify --alter-foreign-keys-method for a table with no child tables.
+test_alter_table(
+   name        => "Child table",
+   table       => "pt_osc.address",
+   pk_col      => "address_id",
+   file        => "basic_with_fks.sql",
+   test_type   => "new_engine",
+   new_engine  => "innodb",
+   cmds        => [
+   qw(
+      --execute
+      --alter-foreign-keys-method auto
+   ),
+      '--alter', 'ENGINE=INNODB',
+   ],
+);
+
+# Use drop_swap to alter address, which no other table references,
+# so the tool should re-enable --swap-tables and --drop-old-table.
+test_alter_table(
+   name        => "Drop-swap child",
+   table       => "pt_osc.address",
+   pk_col      => "address_id",
+   file        => "basic_with_fks.sql",
+   test_type   => "drop_col",
+   drop_col    => "last_update",
+   cmds        => [
+   qw(
+      --execute
+      --alter-foreign-keys-method drop_swap
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+# Alter city and verify that its fk to country still exists.
+# (https://bugs.launchpad.net/percona-toolkit/+bug/969726)
+test_alter_table(
+   name        => "Preserve all fks",
+   table       => "pt_osc.city",
+   pk_col      => "city_id",
+   file        => "basic_with_fks.sql",
+   test_type   => "drop_col",
+   drop_col    => "last_update",
+   check_fks   => "rebuild_constraints",
+   cmds        => [
+   qw(
+      --execute
+      --alter-foreign-keys-method rebuild_constraints
+   ),
+      '--alter', 'DROP COLUMN last_update',
+   ],
+);
+
+SKIP: {
+   skip 'Sandbox master does not have the sakila database', 7
+   unless @{$master_dbh->selectcol_arrayref("SHOW DATABASES LIKE 'sakila'")};
+
+   # This test will use the drop_swap method because the child tables
+   # are large.  To prove this, change check_fks to rebuild_constraints
+   # and the test will fail.
+   test_alter_table(
+      name        => "sakila.staff",
+      table       => "sakila.staff",
+      pk_col      => "staff_id",
+      test_type   => "new_engine",
+      new_engine  => "InnoDB",
+      check_fks   => "drop_swap",
+      cmds        => [
+      qw(
+         --chunk-size 100
+         --execute
+         --alter-foreign-keys-method auto
+      ),
+         '--alter', 'ENGINE=InnoDB'
+      ],
+   );
+
+   # Restore the original fks.
+   diag('Restoring sakila...');
+   diag(`$trunk/sandbox/load-sakila-db 12345`);
+}
+
+# #############################################################################
+# --alter-foreign-keys-method=none.  This intentionally breaks fks because
+# they're not updated so they'll point to the old table that is dropped.
+# #############################################################################
+
+# Specify --alter-foreign-keys-method for a table with no child tables.
+test_alter_table(
+   name         => "Update fk method none",
+   file         => "basic_with_fks.sql",
+   table        => "pt_osc.country",
+   pk_col       => "country_id",
+   max_id       => 20,
+   test_type    => "new_engine",
+   new_engine   => "innodb",
+   cmds         => [
+      qw(--execute --alter-foreign-keys-method none --force --alter ENGINE=INNODB)
+   ],
+);
+
+my $fks = $tp->get_fks(
+   $tp->get_create_table($master_dbh, "pt_osc", "city"));
+is(
+   $fks->{fk_city_country}->{parent_tbl}->{tbl},
+   "_country_old",
+   "--alter-foreign-keys-method=none"
+);
+
+# #############################################################################
+# Alter tables with columns with resvered words and spaces.
+# #############################################################################
+sub test_table {
+   my (%args) = @_;
+   my ($file, $name) = @args{qw(file name)};
+
+   $sb->load_file('master', "t/lib/samples/osc/$file");
+   $master_dbh->do('use osc');
+   $master_dbh->do("DROP TABLE IF EXISTS osc.__new_t");
+
+   my $org_rows = $master_dbh->selectall_arrayref('select * from osc.t order by id');
+
+   ($output, $exit) = full_output(
+      sub { pt_online_schema_change::main(@args,
+         "$dsn,D=osc,t=t", qw(--execute --alter ENGINE=InnoDB)) },
+      stderr => 1,
+   );
+
+   my $new_rows = $master_dbh->selectall_arrayref('select * from osc.t order by id');
+
+   my $fail = 0;
+
+   is_deeply(
+      $new_rows,
+      $org_rows,
+      "$name rows"
+   ) or $fail = 1;
+
+   is(
+      $exit,
+      0,
+      "$name exit status 0"
+   ) or $fail = 1;
+
+   if ( $fail ) {
+      diag("Output from failed test:\n$output");
+   }
+}
+
+test_table(
+   file => "tbl002.sql",
+   name => "Reserved word column",
+);
+
+test_table(
+   file => "tbl003.sql",
+   name => "Space column",
+);
+
+# #############################################################################
+# --[no]swap-tables
+# #############################################################################
+
+test_alter_table(
+   name       => "--no-swap-tables",
+   table      => "pt_osc.t",
+   file       => "basic_no_fks_innodb.sql",
+   max_id     => 20,
+   test_type  => "add_col",
+   new_col    => "foo",
+   no_change  => 1,
+   cmds       => [
+      qw(--execute --no-swap-tables), '--alter', 'ADD COLUMN foo INT'
+   ],
+);
+
+test_alter_table(
+   name       => "--no-swap-tables --no-drop-new-table",
+   table      => "pt_osc.t",
+   file       => "basic_no_fks_innodb.sql",
+   max_id     => 20,
+   test_type  => "add_col",
+   new_col    => "foo",
+   no_change  => 1,
+   cmds       => [
+      qw(--execute --no-swap-tables), '--alter', 'ADD COLUMN foo INT',
+      qw(--no-drop-new-table),
+   ],
+);
+
+# #############################################################################
+# --statistics
+# #############################################################################
+
+$sb->load_file('master', "$sample/bug_1045317.sql");
+
+ok(
+   no_diff(
+      sub { pt_online_schema_change::main(@args, "$dsn,D=bug_1045317,t=bits",
+         '--dry-run', '--statistics',
+         '--alter', "modify column val ENUM('M','E','H') NOT NULL")
+      },
+      "$sample/stats-dry-run.txt",
+      update_sample => 1,
+   ),
+   "--statistics --dry-run"
+) or diag($test_diff);
+
+# 5.5, 5.6 and 5.7 have different outputs 
+my $res_file = "$sample/stats-execute.txt";
+if ($sandbox_version eq '5.5' && $db_flavor !~ m/XtraDB Cluster/) {
+   $res_file =  "$sample/stats-execute-5.5.txt";
+} elsif ($sandbox_version eq '5.6' && $db_flavor !~ m/XtraDB Cluster/) {
+   $res_file =  "$sample/stats-execute-5.6.txt";
+} elsif ($sandbox_version eq '5.7' && $db_flavor !~ m/XtraDB Cluster/) {
+   $res_file =  "$sample/stats-execute-5.7.txt";
+}
+ 
+
+ok(
+   no_diff(
+      sub { pt_online_schema_change::main(@args, "$dsn,D=bug_1045317,t=bits",
+         '--execute', '--statistics',
+         '--alter', "modify column val ENUM('M','E','H') NOT NULL",
+         '--recursion-method', 'none'),
+      },
+      $res_file,
+      keep_output=>1,
+   ),
+   "--statistics --execute"
+) or diag($test_diff);
+
+
+# #############################################################################
+#  --chunk-size-limit=0  must not skip tables that would be chunked 
+#  in one nibble
+#  https://bugs.launchpad.net/percona-toolkit/+bug/1441928
+# #############################################################################
+
+($output, $exit) = full_output(
+   sub { pt_online_schema_change::main(@args,
+      "$dsn,D=sakila,t=actor", qw(--chunk-size-limit 0 --alter-foreign-keys-method drop_swap --execute --alter ENGINE=InnoDB)) },
+   stderr => 1,
+);
+
+like(
+      $output,
+      qr/Successfully altered/i,
+      "--chunk-size-limit=0  doesn't skip tables - lp1441928"
+);
+
+
+# #############################################################################
+# --default-engine
+# #############################################################################
+
+SKIP: {
+   skip "--default-engine tests require < MySQL 5.5", 1
+      if $sandbox_version ge '5.5';
+
+   # The alter doesn't actually change the engine (test_type),
+   # but the --default-engine does because the table uses InnoDB
+   # but MyISAM is the default engine before MySQL 5.5.
+   test_alter_table(
+      name       => "--default-engine",
+      table      => "pt_osc.t",
+      file       => "default-engine.sql",
+      test_type  => "new_engine",
+      new_engine => "MyISAM",
+      cmds       => [
+         '--default-engine',
+         '--execute',
+         '--alter', 'ADD INDEX (d)',
+      ],
+   );
+}
+
+# #############################################################################
+# --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'
+   ],
+);
+
+# #############################################################################
+# --recursion-method=dns  (lp: 1523685)
+# #############################################################################
+
+$sb->load_file('master', "$sample/create_dsns.sql");
+
+($output, $exit) = full_output(
+   sub { pt_online_schema_change::main(@args,
+      "$dsn,D=sakila,t=actor", ('--recursion-method=dsn=D=test_recursion_method,t=dsns,h=127.0.0.1,P=12345,u=msandbox,p=msandbox',  
+          '--alter-foreign-keys-method', 'drop_swap', '--execute', '--alter', 'ENGINE=InnoDB')) 
+       },
+   stderr => 1,
+);
+
+like(
+      $output,
+      qr/Found 2 slaves.*Successfully altered/si,
+      "--recursion-method=dns works"
+);
+
+$master_dbh->do("DROP DATABASE test_recursion_method");
+
+
+# #############################################################################
+# Tests for --preserve-triggers option
+# #############################################################################
+
+SKIP: {
+   skip 'Sandbox MySQL version should be >= 5.7' unless $sandbox_version ge '5.7';
+
+    test_alter_table(
+       name       => "Basic --preserve-triggers #1",
+       table      => "pt_osc.account",
+       pk_col     => "id",
+       file       => "triggers.sql",
+       test_type  => "add_col",
+       new_col    => "foo",
+       cmds       => [
+          qw(--execute --preserve-triggers), '--alter', 'ADD COLUMN foo INT',
+       ],
+    );
+    
+    test_alter_table(
+       name       => "Basic --preserve-triggers after #3",
+       table      => "test.t1",
+       pk_col     => "id",
+       file       => "after_triggers.sql",
+       test_type  => "add_col",
+       new_col    => "foo3",
+       trigger_timing => 'AFTER',
+       cmds       => [
+          qw(--execute --preserve-triggers --alter-foreign-keys-method rebuild_constraints), '--alter', 'ADD COLUMN foo3 INT',
+       ],
+    );
+    
+    
+    #test_alter_table(
+    #   name       => "--preserve-triggers --no-swap-tables",
+    #   table      => "pt_osc.t",
+    #   file       => "basic_no_fks_innodb.sql",
+    #   max_id     => 20,
+    #   test_type  => "add_col",
+    #   new_col    => "foo",
+    #   no_change  => 1,
+    #   cmds       => [
+    #      qw(--execute --no-swap-tables --preserve-triggers), '--alter', 'ADD COLUMN foo INT'
+    #   ],
+    #);
+
+    ($output, $exit) = full_output(
+       sub { pt_online_schema_change::main(@args,
+          "$dsn,D=pt_osc,t=t", 
+          qw(--execute --no-swap-tables --preserve-triggers), '--alter', 'ADD COLUMN foo INT')
+       },
+       stderr => 1,
+    );
+    
+    isnt(
+          $exit,
+          0,
+          "--preserve-triggers --no-swap-tables",
+    );
+    
+    test_alter_table(
+       name        => "Basic FK auto --execute",
+       table       => "pt_osc.country",
+       pk_col      => "country_id",
+       file        => "basic_with_fks.sql",
+       test_type   => "drop_col",
+       drop_col    => "last_update",
+       check_fks   => "rebuild_constraints",
+       cmds        => [
+       qw(
+          --execute
+          --alter-foreign-keys-method rebuild_constraints
+          --preserve-triggers
+       ),
+          '--alter', 'DROP COLUMN last_update',
+       ],
+    );
+}
+
+diag("Reloading sakila");
+my $master_port = $sb->port_for('master');
+system "$trunk/sandbox/load-sakila-db $master_port &";
+
+$sb->do_as_root("master", q/GRANT REPLICATION SLAVE ON *.* TO 'slave_user'@'%' IDENTIFIED BY 'slave_password'/);
+$sb->do_as_root("master", q/set sql_log_bin=0/);
+$sb->do_as_root("master", q/DROP USER 'slave_user'/);
+$sb->do_as_root("master", q/set sql_log_bin=1/);
+
+test_alter_table(
+   name       => "--slave-user --slave-password",
+   file       => "basic_no_fks_innodb.sql",
+   table      => "pt_osc.t",
+   test_type  => "add_col",
+   new_col    => "bar",
+   cmds       => [
+         qw(--execute --slave-user slave_user --slave-password slave_password), '--alter', 'ADD COLUMN bar INT',
+   ],
+);
+# #############################################################################
+# Done.
+# #############################################################################
+$sb->wipe_clean($master_dbh);
+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+#
+done_testing;

t/pt-online-schema-change/samples/after_triggers.sql

@@ -0,0 +1,77 @@
+DROP SCHEMA IF EXISTS test;
+CREATE SCHEMA test;
+
+CREATE TABLE test.t1 (
+   id INT NOT NULL AUTO_INCREMENT,
+   f1 INT,
+   f2 VARCHAR(32),
+   PRIMARY KEY (id)
+);
+
+CREATE TABLE test.t2 LIKE test.t1;
+
+CREATE TABLE test.log (
+   ts  TIMESTAMP,
+   msg VARCHAR(255)
+);
+
+
+DROP TRIGGER IF EXISTS test.after_insert;
+DROP TRIGGER IF EXISTS test.after_update;
+DROP TRIGGER IF EXISTS test.after_delete;
+
+
+CREATE TRIGGER test.after_insert
+ AFTER
+ -- a comment here
+   INSERT ON test.t1
+ -- just to make things harder
+   FOR EACH ROW INSERT INTO test.log VALUES (NOW(), CONCAT("inserted new row with id: ", NEW.id))
+ -- for pt_osc
+;
+
+CREATE TRIGGER test.after_insert2
+ AFTER
+ -- a comment here
+   INSERT ON test.t1
+ -- just to make things harder
+   FOR EACH ROW INSERT INTO test.log VALUES (NOW(), CONCAT("inserted duplicate of new row with id: ", NEW.id))
+ -- for pt_osc
+;
+
+DELIMITER //
+
+CREATE TRIGGER test.after_update
+ AFTER
+ -- a comment here
+   UPDATE ON test.t1
+ -- just to make things harder
+   FOR EACH ROW 
+   BEGIN
+     INSERT INTO test.log VALUES (NOW(), CONCAT("updated row row with id ", OLD.id, " old f1:", OLD.f1, " new f1: ", NEW.f1 ));
+     INSERT INTO test.log VALUES (NOW(), CONCAT("updated row row with id ", OLD.id, " old f1:", OLD.f1, " new f1: ", NEW.f1 ));
+   END
+ -- for pt_osc
+//
+
+DELIMITER ;
+
+CREATE TRIGGER test.after_delete
+ AFTER
+ -- a comment here
+   DELETE ON test.t1
+ -- just to make things harder
+   FOR EACH ROW INSERT INTO test.log VALUES (NOW(), CONCAT("deleted row with id: ", OLD.id))
+ -- for pt_osc
+;
+
+
+INSERT INTO test.t1 VALUES
+(1, 1, 'a'), (2, 1, 'b'), (3, 1, 'c'), (4, 1, 'd'),
+(5, 2, 'e'), (6, 2, 'f'), (7, 3, 'h'), (8, 3, 'g');
+
+DELETE FROM test.t1 WHERE f2 = 'h';
+UPDATE test.t1 
+   SET f1 = f1 + 1
+ WHERE f2 = 'g';
+

t/pt-online-schema-change/samples/stats-execute-5.7.txt

@@ -10,9 +10,10 @@ Operation, tries, wait:
 Altering `bug_1045317`.`bits`...
 TS Dropping triggers...
 TS Dropped triggers OK.
-# Event  Count
+# Event              Count
-# ====== =====
+# ================== =====
-# INSERT     1
+# INSERT                 1
+# mysql_warning_1592     1
 Successfully altered `bug_1045317`.`bits`.
 Creating new table...
 Created new table bug_1045317._bits_new OK.