Fix for 1003315: dry-run + alter-fk-method = auto always fail.

bin/pt-online-schema-change

@@ -6044,6 +6044,13 @@ sub main {
                OptionParser => $o,
             );
          }
+         elsif ( !$alter_fk_method
+               && $o->has('alter-foreign-keys-method')
+               && ($o->get('alter-foreign-keys-method') || '') eq 'auto' ) {
+            # If --alter-foreign-keys-method is 'auto' and we are on a dry run,
+            # $alter_fk_method is left as an empty string.
+            print "Not updating foreign key constraints because this is a dry run.\n";
+         }
          else {
             # This should "never" happen because we check this var earlier.
             die "Invalid --alter-foreign-keys-method: $alter_fk_method\n";

lib/PerconaTest.pm

@@ -35,6 +35,7 @@ use constant PTDEVDEBUG => $ENV{PTDEVDEBUG} || 0;
 
 use Test::More;
 use Time::HiRes qw(sleep time);
+use File::Temp qw(tempfile);
 use POSIX qw(signal_h);
 use Data::Dumper;
 $Data::Dumper::Indent    = 1;
@@ -47,6 +48,7 @@ our %EXPORT_TAGS = ();
 our @EXPORT_OK   = qw();
 our @EXPORT      = qw(
    output
+   full_output
    load_data
    load_file
    parse_file
@@ -645,6 +647,36 @@ sub _d {
    print STDERR "# $package:$line $PID $t ", join(' ', @_), "\n";
 }
 
+# Like output(), but forks a process to execute the coderef.
+# This is because otherwise, errors thrown during cleanup
+# would be skipped.
+sub full_output {
+   my ( $code ) = @_;
+   die "I need a code argument" unless $code;
+
+   my (undef, $file) = tempfile();
+   open *output_fh, '>', $file
+         or die "Cannot open file $file: $OS_ERROR";
+   local *STDOUT = *output_fh;
+
+   *STDERR = *STDOUT;
+
+   my $status;
+   warn $file;
+   if (my $pid = fork) {
+      waitpid($pid, 0);
+      $status = $?;
+   }
+   else {
+      $code->();
+      exit 0;
+   }
+   close *output_fh;
+   my $output = do { local $/; open my $fh, "<", $file or die $!; <$fh> };
+
+   return ($output, $status);
+}
+
 1;
 }
 # ###########################################################################

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

@@ -29,7 +29,7 @@ elsif ( !$slave_dbh ) {
    plan skip_all => 'Cannot connect to sandbox slave1';
 }
 else {
-   plan tests => 3;
+   plan tests => 6;
 }
 
 # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
@@ -83,6 +83,34 @@ 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/,
     "Bug 1002448: mistakenly uses indexes instead of keys";
 
+# ############################################################################
+# https://bugs.launchpad.net/percona-toolkit/+bug/1003315
+# ############################################################################
+$sb->load_file('master', "$sample/bug-1003315.sql");
+
+# Have to use full_output here, because the error message may happen during
+# cleanup, and so won't be caught by output().
+($output, $exit_status) = full_output(
+    sub { pt_online_schema_change::main(@args,
+            "$master_dsn,D=test1003315,t=A",
+            "--alter", "ENGINE=InnoDB",
+            "--alter-foreign-keys-method", "auto",
+            "--dry-run",
+            qw(--chunk-size 2 --dry-run --print))
+        
+    },
+);
+
+is $exit_status, 0, "Bug 1003315: Correct exit value for a dry run";
+
+unlike $output,
+    qr/\QError updating foreign key constraints: Invalid --alter-foreign-keys-method:/,
+    "Bug 1003315: No error when combining --alter-foreign-keys-method auto and --dry-run";
+
+like $output,
+    qr/\QNot updating foreign key constraints because this is a dry run./,
+    "Bug 1003315: But now we do get an explanation from --dry-run";
+
 # #############################################################################
 # Done.
 # #############################################################################

t/pt-online-schema-change/samples/bug-1003315.sql

@@ -0,0 +1,23 @@
+drop database if exists test1003315;
+create database test1003315;
+use test1003315;
+
+DROP TABLE IF EXISTS `B`;
+DROP TABLE IF EXISTS `A`;
+CREATE TABLE `A` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `foo` varchar(30) NOT NULL,
+  PRIMARY KEY (`id`)
+) ENGINE=InnoDB;
+
+DROP TABLE IF EXISTS `B`;
+CREATE TABLE `B` (
+  `id` int(11) NOT NULL AUTO_INCREMENT,
+  `a` int(11) NOT NULL,
+  KEY `9dde1f34` (`a`),
+  PRIMARY KEY (`id`),
+  CONSTRAINT `6970ddb42bec57fc` FOREIGN KEY (`a`) REFERENCES `A` (`id`)
+) ENGINE=InnoDB;
+
+INSERT INTO `A` VALUES (1,'bar'), (2,'bar2'), (3,'bar3');
+INSERT INTO `B` VALUES (1, 1), (2, 2), (3, 1);