Use 'h=localhost' if no DSN or DSN options given. Remove unused DSN parts (D and t) and don't copy some parts. Move issue_947.t tests into standard_options.t and remove issue_947.t.

2025-09-19 10:34:01 +00:00 · 2011-10-14 07:59:43 -06:00
parent 60d8b1a710
commit 48fb4baa7c
5 changed files with 143 additions and 83 deletions
--- a/bin/pt-table-checksum
+++ b/bin/pt-table-checksum
@@ -1417,18 +1417,19 @@ sub new {
   foreach my $arg ( @required_args ) {
      die "I need a $arg argument" unless $args{$arg};
   };
-   die "I need a dsn or dsn_string argument"
-      unless $args{dsn} || $args{dsn_string};
   my ($dp, $o) = @args{@required_args};

+   my $dsn_defaults = $dp->parse_options($o);
+
   my $dsn = $args{dsn};
   if ( !$dsn ) {
+      $args{dsn_string} ||= 'h=' . ($dsn_defaults->{h} || 'localhost');
+
      $dsn = $dp->parse(
-         $args{dsn_string}, $args{prev_dsn}, $dp->parse_options($o));
+         $args{dsn_string}, $args{prev_dsn}, $dsn_defaults);
   }

   my $self = {
-      dsn_string   => $args{dsn_string},
      dsn          => $dsn,
      dbh          => $args{dbh},
      set          => $args{set},
@@ -7073,6 +7074,12 @@ group: Help

 Show help and exit.

+=item --host
+
+short form: -h; type: string; default: localhost; group: Connection
+
+Host to connect to.
+
 =item --ignore-columns

 type: Hash; group: Filter
@@ -7466,21 +7473,15 @@ dsn: charset; copy: yes

 Default character set.

-=item * D
-
-dsn: database; copy: yes
-
-Default database.
-
 =item * F

-dsn: mysql_read_default_file; copy: yes
+dsn: mysql_read_default_file; copy: no

 Only read default options from the given file

 =item * h

-dsn: host; copy: yes
+dsn: host; copy: no

 Connect to host.

@@ -7498,16 +7499,10 @@ Port number to use for connection.

 =item * S

-dsn: mysql_socket; copy: yes
+dsn: mysql_socket; copy: no

 Socket file to use for connection.

-=item * t
-
-dsn: table; copy: no
-
-Table for DSN L<"--recursion-method">.
-
 =item * u

 dsn: user; copy: yes
--- a/lib/Cxn.pm
+++ b/lib/Cxn.pm
@@ -57,18 +57,29 @@ sub new {
   foreach my $arg ( @required_args ) {
      die "I need a $arg argument" unless $args{$arg};
   };
-   die "I need a dsn or dsn_string argument"
-      unless $args{dsn} || $args{dsn_string};
   my ($dp, $o) = @args{@required_args};

+   # Any tool that connects to MySQL should have a standard set of
+   # connection options like --host, --port, --user, etc.  These
+   # are default values; they're used in the DSN if the DSN doesn't
+   # explicate the corresponding part (h=--host, P=--port, etc.).
+   my $dsn_defaults = $dp->parse_options($o);
+
   my $dsn = $args{dsn};
   if ( !$dsn ) {
+      # If there's no DSN and no DSN string, then the user probably ran
+      # the tool without specifying a DSN or any default connection options.
+      # They're probably relying on DBI/DBD::mysql to do the right thing
+      # by connecting to localhost.  On many systems, connecting just to
+      # localhost causes DBI to use a built-in socket, i.e. it doesn't
+      # always equate to 'h=127.0.0.1,P=3306'.
+      $args{dsn_string} ||= 'h=' . ($dsn_defaults->{h} || 'localhost');
+
      $dsn = $dp->parse(
-         $args{dsn_string}, $args{prev_dsn}, $dp->parse_options($o));
+         $args{dsn_string}, $args{prev_dsn}, $dsn_defaults);
   }

   my $self = {
-      dsn_string   => $args{dsn_string},
      dsn          => $dsn,
      dbh          => $args{dbh},
      set          => $args{set},
--- a/t/lib/Cxn.t
+++ b/t/lib/Cxn.t
@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 14;
+use Test::More tests => 16;

 use Sandbox;
 use OptionParser;
@@ -36,6 +36,7 @@ $dp->prop('set-vars', $o->get('set-vars'));

 sub make_cxn {
   my (%args) = @_;
+   $o->get_opts();
   return new Cxn(
      OptionParser => $o,
      DSNParser    => $dp,
@@ -197,6 +198,50 @@ is_deeply(
   "cxn->dsn()"
 );

+# ############################################################################
+# Default cxn, should be equivalent to 'h=localhost'.
+# ############################################################################
+my $default_cxn = make_cxn();
+is_deeply(
+   $default_cxn->dsn(),
+   {
+      h => 'localhost',
+      P => undef,
+      u => undef,
+      p => undef,
+      A => undef,
+      F => undef,
+      S => undef,
+      D => undef,
+      t => undef,
+      n => 'h=localhost',
+   },
+   "Defaults to h=localhost"
+);
+
+# But now test if it will inherit just a few standard connection options.
+@ARGV = qw(--port 12345);
+$default_cxn = make_cxn();
+is_deeply(
+   $default_cxn->dsn(),
+   {
+      h => 'localhost',
+      P => '12345',
+      u => undef,
+      p => undef,
+      A => undef,
+      F => undef,
+      S => undef,
+      D => undef,
+      t => undef,
+      n => 'h=localhost,P=12345',
+   },
+   "Default cxn inherits default connection options"
+);
+
+@ARGV = ();
+$o->get_opts();
+
 # #############################################################################
 # Done.
 # #############################################################################
--- a/t/pt-table-checksum/issue_947.t
+++ b/t/pt-table-checksum/issue_947.t
@@ -1,49 +0,0 @@
-#!/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 PerconaTest;
-use Sandbox;
-require "$trunk/bin/pt-table-checksum";
-
-my $dp  = new DSNParser(opts=>$dsn_opts);
-my $sb  = new Sandbox(basedir => '/tmp', DSNParser => $dp);
-my $dbh = $sb->get_dbh_for('master');
-
-if ( !$dbh ) {
-   plan skip_all => 'Cannot connect to sandbox master';
-}
-else {
-   plan tests => 1;
-}
-
-my $output;
-my $cnf = '/tmp/12345/my.sandbox.cnf';
-
-# #############################################################################
-# Issue 947: mk-table-checksum crashes if h DSN part is not given
-# #############################################################################
-
-$output = output(
-   sub { pt_table_checksum::main("F=$cnf", qw(-d mysql -t user)) },
-   stderr => 1,
-);
-like(
-   $output,
-   qr/mysql\s+user\s+0/,
-   "Doesn't crash if no h DSN part (issue 947)"
-);
-
-# #############################################################################
-# Done.
-# #############################################################################
-exit;
--- a/t/pt-table-checksum/standard_options.t
+++ b/t/pt-table-checksum/standard_options.t
@@ -9,32 +9,90 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 2;
+use Test::More;

 use PerconaTest;
+use Sandbox;
 require "$trunk/bin/pt-table-checksum";

+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 slave1';
+}
+else {
+   plan tests => 4;
+}
+
+# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
+# so we need to specify --lock-wait-timeout=3 else the tool will die.
+my @args     = (qw(--lock-wait-timeout 3 --explain --tables sakila.country)); 
+my $cnf      = "/tmp/12345/my.sandbox.cnf";
+my $pid_file = "/tmp/mk-table-checksum-test.pid";
 my $output;

-# Test DSN value inheritance
-$output = `$trunk/bin/pt-table-checksum h=127.1 h=127.2,P=12346 --port 12345 --explain-hosts`;
+# ############################################################################
+# DSN should inherit connection options (--port, etc.)
+# ############################################################################
+
+$output = output(
+   sub { pt_table_checksum::main(@args, 'h=127.1',
+      qw(--port 12345 --user msandbox --password msandbox)) },
+);
 like(
   $output,
-   qr/^Server 127.1:\s+P=12345,h=127.1\s+Server 127.2:\s+P=12346,h=127.2/,
-   'DSNs inherit values from --port, etc. (issue 248)'
+   qr/-- sakila\.country/,
+   'DSN inherits values from --port, etc. (issue 248)'
+);
+
+# Same test but this time intentionally use the wrong port so the connection
+# fails so we know that the previous test didn't work because your system
+# has a .my.cnf file or something.
+eval {
+   pt_table_checksum::main(@args, 'h=127.1',
+      qw(--port 4 --user msandbox --password msandbox));
+};
+like(
+   $EVAL_ERROR,
+   qr/port=4.+failed/,
+   'DSN truly inherits values from --port, etc. (issue 248)'
+);
+
+# #############################################################################
+# Issue 947: mk-table-checksum crashes if h DSN part is not given
+# #############################################################################
+
+$output = output(
+   sub { pt_table_checksum::main(@args, "F=$cnf") },
+);
+like(
+   $output,
+   qr/-- sakila\.country/,
+   "Doesn't crash if no h DSN part (issue 947)"
 );

 # #########################################################################
 # Issue 391: Add --pid option to all scripts
 # #########################################################################
-`touch /tmp/mk-script.pid`;
-$output = `$trunk/bin/pt-table-checksum h=127.1,P=12345,u=msandbox,p=msandbox -d test -t issue_122,issue_94 --pid /tmp/mk-script.pid 2>&1`;
+diag(`rm -rf $pid_file >/dev/null 2>&1`);
+diag(`touch $pid_file`);
+
+eval {
+   pt_table_checksum::main(@args, $cnf, '--pid', $pid_file);
+};
 like(
-   $output,
-   qr{PID file /tmp/mk-script.pid already exists},
+   $EVAL_ERROR,
+   qr/PID file $pid_file already exists/,
   'Dies if PID file already exists (issue 391)'
 );
-`rm -rf /tmp/mk-script.pid`;
+
+diag(`rm -rf $pid_file >/dev/null 2>&1`);

 # #############################################################################
 # Done.