Merge lp:~percona-toolkit-dev/percona-toolkit/fix-1022622-ptcd-case-sensitivity into lp:percona-toolkit/2.1

[Brian Fraser]

[Daniel Nichter (daniel-nichter)]

23 - if ( my ($charset) = $cxn_string =~ m/charset=(\w+)/ ) {
24 - $sql = "/*!40101 SET NAMES $charset*/";
25 + if ( my ($charset) = $cxn_string =~ m/charset=([\w]+)/ ) {
26 + $sql = qq{/*!40101 SET NAMES "$charset"*/};

Is that part of the bug fix? Or what's it related to?

[Brian Fraser (fraserbn)]

Looks like that's from running update-modules on ptcd.

[Daniel Nichter (daniel-nichter)]

review: Approve

=== modified file 'bin/pt-config-diff'
--- bin/pt-config-diff	2012-07-13 01:42:54 +0000
+++ bin/pt-config-diff	2012-07-31 16:04:38 +0000
@@ -1291,7 +1291,7 @@
       PTDEBUG && _d($dbh, $sql);
       my ($sql_mode) = eval { $dbh->selectrow_array($sql) };
       if ( $EVAL_ERROR ) {
-         die $EVAL_ERROR;
+         die "Error getting the current SQL_MODE: $EVAL_ERROR";
       }
 
       $sql = 'SET @@SQL_QUOTE_SHOW_CREATE = 1'
@@ -1301,15 +1301,17 @@
       PTDEBUG && _d($dbh, $sql);
       eval { $dbh->do($sql) };
       if ( $EVAL_ERROR ) {
-         die $EVAL_ERROR;
+         die "Error setting SQL_QUOTE_SHOW_CREATE, SQL_MODE"
+           . ($sql_mode ? " and $sql_mode" : '')
+           . ": $EVAL_ERROR";
       }
 
-      if ( my ($charset) = $cxn_string =~ m/charset=(\w+)/ ) {
-         $sql = "/*!40101 SET NAMES $charset*/";
+      if ( my ($charset) = $cxn_string =~ m/charset=([\w]+)/ ) {
+         $sql = qq{/*!40101 SET NAMES "$charset"*/};
          PTDEBUG && _d($dbh, ':', $sql);
          eval { $dbh->do($sql) };
          if ( $EVAL_ERROR ) {
-            die $EVAL_ERROR;
+            die "Error setting NAMES to $charset: $EVAL_ERROR";
          }
          PTDEBUG && _d('Enabling charset for STDOUT');
          if ( $charset eq 'utf8' ) {
@@ -1321,12 +1323,12 @@
          }
       }
 
-      if ( $self->prop('set-vars') ) {
-         $sql = "SET " . $self->prop('set-vars');
+      if ( my $var = $self->prop('set-vars') ) {
+         $sql = "SET $var";
          PTDEBUG && _d($dbh, ':', $sql);
          eval { $dbh->do($sql) };
          if ( $EVAL_ERROR ) {
-            die $EVAL_ERROR;
+            die "Error setting $var: $EVAL_ERROR";
          }
       }
    }
@@ -1451,6 +1453,7 @@
       dsn_name     => $dp->as_string($dsn, [qw(h P S)]),
       hostname     => '',
       set          => $args{set},
+      NAME_lc      => defined($args{NAME_lc}) ? $args{NAME_lc} : 1,
       dbh_set      => 0,
       OptionParser => $o,
       DSNParser    => $dp,
@@ -1488,7 +1491,10 @@
 
    PTDEBUG && _d($dbh, 'Setting dbh');
 
-   $dbh->{FetchHashKeyName} = 'NAME_lc';
+   if ( !exists $self->{NAME_lc}
+        || (defined $self->{NAME_lc} && $self->{NAME_lc}) ) {
+      $dbh->{FetchHashKeyName} = 'NAME_lc';
+   }
    
    my $sql = 'SELECT @@hostname, @@server_id';
    PTDEBUG && _d($dbh, $sql);
@@ -2382,6 +2388,9 @@
       value_is_optional => \%value_is_optional,
       any_value_is_true => \%any_value_is_true,
       base_path         => \%base_path,
+      ignore_case       => exists $args{ignore_case}
+                                ? $args{ignore_case}
+                                : 1,
    };
 
    return bless $self, $class;
@@ -2408,6 +2417,7 @@
    my $config0     = $configs->[0];
    my $last_config = @$configs - 1;
    my $vars        = $self->_get_shared_vars(%args);
+   my $ignore_case = $self->{ignore_case};
 
    my $diffs;
    VARIABLE:
@@ -2432,7 +2442,9 @@
                next CONFIG if $val0 == $valN;
             }
             else {
-               next CONFIG if $val0 eq $valN;
+               next CONFIG if $ignore_case
+                              ? lc($val0) eq lc($valN)
+                              : $val0 eq $valN;
 
                if ( $config0->format() ne $configN->format() ) {
                   if ( $any_value_is_true->{$var} ) {
@@ -2933,6 +2945,7 @@
    my $trp         = new TextResultSetParser();
    my $config_cmp  = new MySQLConfigComparer(
       ignore_variables => $o->get('ignore-variables'),
+      ignore_case      => $o->get('ignore-case'),
    );
    my %common_modules = (
       DSNParser           => $dp,
@@ -3162,6 +3175,12 @@
 
 Prompt for a password when connecting to MySQL.
 
+=item --[no]ignore-case
+
+default: yes
+
+Compare the variables case-insensitively.
+
 =item --charset
 
 short form: -A; type: string

=== modified file 'lib/MySQLConfigComparer.pm'
--- lib/MySQLConfigComparer.pm	2012-01-19 19:46:56 +0000
+++ lib/MySQLConfigComparer.pm	2012-07-31 16:04:38 +0000
@@ -126,6 +126,9 @@
       value_is_optional => \%value_is_optional,
       any_value_is_true => \%any_value_is_true,
       base_path         => \%base_path,
+      ignore_case       => exists $args{ignore_case}
+                                ? $args{ignore_case}
+                                : 1,
    };
 
    return bless $self, $class;
@@ -172,6 +175,7 @@
    my $config0     = $configs->[0];
    my $last_config = @$configs - 1;
    my $vars        = $self->_get_shared_vars(%args);
+   my $ignore_case = $self->{ignore_case};
 
    # Compare variables from first config (config0) to other configs (configN).
    my $diffs;
@@ -197,7 +201,9 @@
                next CONFIG if $val0 == $valN;
             }
             else {
-               next CONFIG if $val0 eq $valN;
+               next CONFIG if $ignore_case
+                              ? lc($val0) eq lc($valN)
+                              : $val0 eq $valN;
 
                # Special rules apply when comparing different inputs/formats,
                # e.g. when comparing an option file to SHOW VARIABLES.  This

=== modified file 't/lib/MySQLConfigComparer.t'
--- t/lib/MySQLConfigComparer.t	2012-03-06 13:56:08 +0000
+++ t/lib/MySQLConfigComparer.t	2012-07-31 16:04:38 +0000
@@ -9,7 +9,7 @@
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 16;
+use Test::More;
 
 use TextResultSetParser();
 use MySQLConfigComparer;
@@ -370,6 +370,39 @@
 }
 
 # #############################################################################
+# Case insensitivity
+# #############################################################################
+
+$c1 = new MySQLConfig(
+   result_set => [['binlog_format', 'MIXED']],
+   format     => 'option_file',
+);
+
+$c2 = new MySQLConfig(
+   result_set => [['binlog_format', 'mixed']],
+   format     => 'option_file',
+);
+
+is_deeply(
+   diff($c1, $c2),
+   undef,
+   "Case insensitivity is on by default"
+);
+
+my $case_cc = MySQLConfigComparer->new( ignore_case => undef, );
+
+is_deeply(
+   $case_cc->diff(configs => [$c1, $c2]),
+   {
+      binlog_format => [
+         'MIXED',
+         'mixed'
+      ]
+   },
+   "..but can be turned off"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 {
@@ -382,4 +415,5 @@
    qr/Complete test coverage/,
    '_d() works'
 );
-exit;
+
+done_testing;

=== modified file 't/pt-config-diff/basics.t'
--- t/pt-config-diff/basics.t	2012-06-03 19:14:30 +0000
+++ t/pt-config-diff/basics.t	2012-07-31 16:04:38 +0000
@@ -26,11 +26,9 @@
 elsif ( !$slave_dbh ) {
    plan skip_all => 'Cannot connect to sandbox slave';
 }
-else {
-   plan tests => 13;
-}
 
 my $cnf = '/tmp/12345/my.sandbox.cnf';
+my $samples = "$trunk/t/pt-config-diff/samples/";
 my $output;
 my $retval;
 
@@ -163,7 +161,43 @@
 );
 
 # #############################################################################
+# Case insensitivity
+# #############################################################################
+
+use File::Spec;
+
+$output = output(
+   sub { $retval = pt_config_diff::main(
+      File::Spec->catfile($samples, "case1.cnf"),
+      File::Spec->catfile($samples, "case2.cnf"),
+   ) },
+   stderr => 1,
+);
+
+is(
+   $output,
+   "",
+   "Case-insensitive by default, finds no diffs"
+);
+
+$output = output(
+   sub { $retval = pt_config_diff::main(
+      File::Spec->catfile($samples, "case1.cnf"),
+      File::Spec->catfile($samples, "case2.cnf"),
+      '--no-ignore-case',
+   ) },
+   stderr => 1,
+);
+
+like(
+   $output,
+   qr/binlog_format\s+genlog\s+GENLOG/,
+   "With case-insensitivity turned off, finds one diff"
+);
+
+# #############################################################################
 # Done.
 # #############################################################################
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
-exit;
+
+done_testing;