Cosmetic changes and tests

bin/pt-online-schema-change

@@ -572,7 +572,7 @@ sub get_opts {
 
    if ( @ARGV && $ARGV[0] =~/^--config=/ ) {
       $ARGV[0] = substr($ARGV[0],9);
-      $ARGV[0] =~ s/^'(.*)'$/$1/
+      $ARGV[0] =~ s/^'(.*)'$/$1/;
       $ARGV[0] =~ s/^"(.*)"$/$1/;
       $self->_set_option('config', shift @ARGV);
    }
@@ -10108,9 +10108,13 @@ sub create_new_table {
       if ( $o->get('default-engine') ) {
          $sql =~ s/\s+ENGINE=\S+//;
       }
-      if ( $o->get('new-partition') ) {
+      if ( $o->get('data-dir') ) {
-         my $ady = " DATA DIRECTORY = '". $o->get('new-partition')."'";
+          if ( (-d $o->get('data-dir')) && (-w $o->get('data-dir')) ){
-         $sql =  $sql.${ady};
+             $sql =~ s/DATA DIRECTORY='.*?'//; 
+             $sql .= sprintf(" DATA DIRECTORY='%s' ",$o->get('data-dir'));
+          } else {
+              die $o->get('data-dir') . " is not a directory or it is not writable";
+          }
       }
       PTDEBUG && _d($sql);
       eval {
@@ -11563,7 +11567,7 @@ cause unintended changes on replicas which use a different engine for
 the same table.  Specifying this option causes the new table to be
 created with the system's default engine.
 
- =item --new-partition
+=item --data-dir
 
 type: string
 

t/pt-online-schema-change/issue-1638293.t

@@ -0,0 +1,76 @@
+#!/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 Data::Dumper;
+use PerconaTest;
+use Sandbox;
+use SqlModes;
+use File::Temp qw/ tempdir /;
+
+require "$trunk/bin/pt-online-schema-change";
+
+my $dp = new DSNParser(opts=>$dsn_opts);
+my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
+
+my ($master_dbh, $master_dsn) = $sb->start_sandbox(
+   server => 'cmaster',
+   type   => 'master',
+   env    => q/FORK="pxc" BINLOG_FORMAT="ROW"/,
+);
+
+if ( !$master_dbh ) {
+   plan skip_all => 'Cannot connect to sandbox master';
+}
+
+# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
+# so we need to specify --set-vars innodb_lock_wait_timeout=3 else the
+# tool will die.
+my @args       = (qw(--set-vars innodb_lock_wait_timeout=3));
+my $output;
+my $exit_status;
+my $sample  = "t/pt-online-schema-change/samples/";
+
+# This is the same test we have for bug-1613915 but using DATA-DIR
+$sb->load_file('cmaster', "$sample/bug-1613915.sql");
+my $dir = tempdir( CLEANUP => 1 );
+$output = output(
+   sub { pt_online_schema_change::main(@args, "$master_dsn,D=test,t=o1",
+         '--execute', 
+         '--alter', "ADD COLUMN c INT",
+         '--chunk-size', '10',
+         '--data-dir', $dir,
+         ),
+      },
+);
+
+like(
+      $output,
+      qr/Successfully altered/s,
+      "bug-1613915 enum field in primary key",
+);
+
+my $rows = $master_dbh->selectrow_arrayref(
+   "SELECT COUNT(*) FROM test.o1");
+is(
+   $rows->[0],
+   100,
+   "bug-1613915 correct rows count"
+) or diag(Dumper($rows));
+
+$master_dbh->do("DROP DATABASE IF EXISTS test");
+
+# #############################################################################
+# Done.
+# #############################################################################
+$sb->wipe_clean($master_dbh);
+done_testing;