From 8e9e8eb7a6f7953920102a95009f763acee10ea5 Mon Sep 17 00:00:00 2001
From: Daniel Nichter <daniel@percona.com>
Date: Tue, 21 Feb 2012 13:22:56 -0700
Subject: [PATCH] Test and fix space-flattening bug.

---
 bin/pt-table-checksum                       | 52 +++++++--------
 lib/TableParser.pm                          | 70 +++++++++++++--------
 t/lib/TableParser.t                         | 15 ++++-
 t/pt-table-checksum/basics.t                | 26 +++++++-
 t/pt-table-checksum/samples/2-space-col.sql |  8 +++
 5 files changed, 118 insertions(+), 53 deletions(-)
 create mode 100644 t/pt-table-checksum/samples/2-space-col.sql

diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum
index 5e949542..5d709291 100755
--- a/bin/pt-table-checksum
+++ b/bin/pt-table-checksum
@@ -1771,41 +1771,43 @@ sub get_create_table {
    die "I need a tbl parameter" unless $tbl;
    my $q = $self->{Quoter};
 
-   my $sql = '/*!40101 SET @OLD_SQL_MODE := @@SQL_MODE, '
-           . q{@@SQL_MODE := REPLACE(REPLACE(@@SQL_MODE, 'ANSI_QUOTES', ''), ',,', ','), }
-           . '@OLD_QUOTE := @@SQL_QUOTE_SHOW_CREATE, '
-           . '@@SQL_QUOTE_SHOW_CREATE := 1 */';
-   PTDEBUG && _d($sql);
-   eval { $dbh->do($sql); };
+   my $new_sql_mode
+      = '/*!40101 SET @OLD_SQL_MODE := @@SQL_MODE, '
+      . q{@@SQL_MODE := REPLACE(REPLACE(@@SQL_MODE, 'ANSI_QUOTES', ''), ',,', ','), }
+      . '@OLD_QUOTE := @@SQL_QUOTE_SHOW_CREATE, '
+      . '@@SQL_QUOTE_SHOW_CREATE := 1 */';
+
+   my $old_sql_mode = '/*!40101 SET @@SQL_MODE := @OLD_SQL_MODE, '
+                     . '@@SQL_QUOTE_SHOW_CREATE := @OLD_QUOTE */';
+
+   PTDEBUG && _d($new_sql_mode);
+   eval { $dbh->do($new_sql_mode); };
    PTDEBUG && $EVAL_ERROR && _d($EVAL_ERROR);
 
-   $sql = 'USE ' . $q->quote($db);
-   PTDEBUG && _d($dbh, $sql);
-   $dbh->do($sql);
+   my $use_sql = 'USE ' . $q->quote($db);
+   PTDEBUG && _d($dbh, $use_sql);
+   $dbh->do($use_sql);
 
-   $sql = "SHOW CREATE TABLE " . $q->quote($db, $tbl);
-   PTDEBUG && _d($sql);
+   my $show_sql = "SHOW CREATE TABLE " . $q->quote($db, $tbl);
+   PTDEBUG && _d($show_sql);
    my $href;
-   eval { $href = $dbh->selectrow_hashref($sql); };
+   eval { $href = $dbh->selectrow_hashref($show_sql); };
    if ( $EVAL_ERROR ) {
       PTDEBUG && _d($EVAL_ERROR);
+
+      PTDEBUG && _d($old_sql_mode);
+      $dbh->do($old_sql_mode);
+
       return;
    }
 
-   $sql = '/*!40101 SET @@SQL_MODE := @OLD_SQL_MODE, '
-        . '@@SQL_QUOTE_SHOW_CREATE := @OLD_QUOTE */';
-   PTDEBUG && _d($sql);
-   $dbh->do($sql);
+   PTDEBUG && _d($old_sql_mode);
+   $dbh->do($old_sql_mode);
 
-   my ($key) = grep { m/create table/i } keys %$href;
-   if ( $key ) {
-      PTDEBUG && _d('This table is a base table');
-      $href->{$key}  =~ s/\b[ ]{2,}/ /g;
-      $href->{$key} .= "\n";
-   }
-   else {
-      PTDEBUG && _d('This table is a view');
-      ($key) = grep { m/create view/i } keys %$href;
+   my ($key) = grep { m/create (?:table|view)/i } keys %$href;
+   if ( !$key ) {
+      die "Error: no 'Create Table' or 'Create View' in result set from "
+         . "$show_sql: " . Dumper($href);
    }
 
    return $href->{$key};
diff --git a/lib/TableParser.pm b/lib/TableParser.pm
index 260d0224..994d1e6b 100644
--- a/lib/TableParser.pm
+++ b/lib/TableParser.pm
@@ -58,43 +58,63 @@ sub get_create_table {
    die "I need a tbl parameter" unless $tbl;
    my $q = $self->{Quoter};
 
-   my $sql = '/*!40101 SET @OLD_SQL_MODE := @@SQL_MODE, '
-           . q{@@SQL_MODE := REPLACE(REPLACE(@@SQL_MODE, 'ANSI_QUOTES', ''), ',,', ','), }
-           . '@OLD_QUOTE := @@SQL_QUOTE_SHOW_CREATE, '
-           . '@@SQL_QUOTE_SHOW_CREATE := 1 */';
-   PTDEBUG && _d($sql);
-   eval { $dbh->do($sql); };
+   # To ensure a consistent output, we save the current (old) SQL mode,
+   # then set it to the new SQL mode that what we need.  When done, even
+   # if an error occurs, we restore the old SQL mode.
+   my $new_sql_mode
+      = '/*!40101 SET @OLD_SQL_MODE := @@SQL_MODE, '
+      . q{@@SQL_MODE := REPLACE(REPLACE(@@SQL_MODE, 'ANSI_QUOTES', ''), ',,', ','), }
+      . '@OLD_QUOTE := @@SQL_QUOTE_SHOW_CREATE, '
+      . '@@SQL_QUOTE_SHOW_CREATE := 1 */';
+
+   my $old_sql_mode = '/*!40101 SET @@SQL_MODE := @OLD_SQL_MODE, '
+                     . '@@SQL_QUOTE_SHOW_CREATE := @OLD_QUOTE */';
+
+   # Set new SQL mode.
+   PTDEBUG && _d($new_sql_mode);
+   eval { $dbh->do($new_sql_mode); };
    PTDEBUG && $EVAL_ERROR && _d($EVAL_ERROR);
 
    # Must USE the tbl's db because some bug with SHOW CREATE TABLE on a
    # view when the current db isn't the view's db causes MySQL to crash.
-   $sql = 'USE ' . $q->quote($db);
-   PTDEBUG && _d($dbh, $sql);
-   $dbh->do($sql);
+   my $use_sql = 'USE ' . $q->quote($db);
+   PTDEBUG && _d($dbh, $use_sql);
+   $dbh->do($use_sql);
 
-   $sql = "SHOW CREATE TABLE " . $q->quote($db, $tbl);
-   PTDEBUG && _d($sql);
+   my $show_sql = "SHOW CREATE TABLE " . $q->quote($db, $tbl);
+   PTDEBUG && _d($show_sql);
    my $href;
-   eval { $href = $dbh->selectrow_hashref($sql); };
+   eval { $href = $dbh->selectrow_hashref($show_sql); };
    if ( $EVAL_ERROR ) {
+      # TODO: I think we fail silently for tools which may try to call
+      # this on temp tables, or don't care if the table goes away.  We
+      # should warn $EVAL_ERROR and require callers to eval us and do
+      # what they want with the warning.
       PTDEBUG && _d($EVAL_ERROR);
+
+      # Restore old SQL mode.
+      PTDEBUG && _d($old_sql_mode);
+      $dbh->do($old_sql_mode);
+
       return;
    }
 
-   $sql = '/*!40101 SET @@SQL_MODE := @OLD_SQL_MODE, '
-        . '@@SQL_QUOTE_SHOW_CREATE := @OLD_QUOTE */';
-   PTDEBUG && _d($sql);
-   $dbh->do($sql);
+   # Restore old SQL mode.
+   PTDEBUG && _d($old_sql_mode);
+   $dbh->do($old_sql_mode);
 
-   my ($key) = grep { m/create table/i } keys %$href;
-   if ( $key ) {
-      PTDEBUG && _d('This table is a base table');
-      $href->{$key}  =~ s/\b[ ]{2,}/ /g;
-      $href->{$key} .= "\n";
-   }
-   else {
-      PTDEBUG && _d('This table is a view');
-      ($key) = grep { m/create view/i } keys %$href;
+   # SHOW CREATE TABLE has at least 2 columns like:
+   # mysql> show create table city\G
+   # *************************** 1. row ***************************
+   #        Table: city
+   # Create Table: CREATE TABLE `city` (
+   #   `city_id` smallint(5) unsigned NOT NULL AUTO_INCREMENT,
+   #   ... 
+   # We want the second column.
+   my ($key) = grep { m/create (?:table|view)/i } keys %$href;
+   if ( !$key ) {
+      die "Error: no 'Create Table' or 'Create View' in result set from "
+         . "$show_sql: " . Dumper($href);
    }
 
    return $href->{$key};
diff --git a/t/lib/TableParser.t b/t/lib/TableParser.t
index e65e6867..45390605 100644
--- a/t/lib/TableParser.t
+++ b/t/lib/TableParser.t
@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 38;
+use Test::More tests => 39;
 
 use TableParser;
 use Quoter;
@@ -38,15 +38,26 @@ SKIP: {
       "get_create_table(nonexistent table)"
    );
 
+   my $ddl = $tp->get_create_table($dbh, 'sakila', 'actor');
    ok(
       no_diff(
-         $tp->get_create_table($dbh, 'sakila', 'actor'),
+         "$ddl\n",
          $sandbox_version ge '5.1' ? "$sample/sakila.actor"
                                    : "$sample/sakila.actor-5.0",
          cmd_output => 1,
       ),
       "get_create_table(sakila.actor)"
    );
+
+   # Bug 932442: column with 2 spaces
+   $sb->load_file('master', "t/pt-table-checksum/samples/2-space-col.sql");
+   PerconaTest::wait_for_table($dbh, "test.t");
+   $ddl = $tp->get_create_table($dbh, qw(test t));
+   like(
+      $ddl,
+      qr/`a  b`\s+/,
+      "Does not compress spaces (bug 932442)"
+   );
 };
 
 eval {
diff --git a/t/pt-table-checksum/basics.t b/t/pt-table-checksum/basics.t
index b8fc9249..bba96cad 100644
--- a/t/pt-table-checksum/basics.t
+++ b/t/pt-table-checksum/basics.t
@@ -41,7 +41,7 @@ elsif ( !@{$master_dbh->selectall_arrayref('show databases like "sakila"')} ) {
    plan skip_all => 'sakila database is not loaded';
 }
 else {
-   plan tests => 30;
+   plan tests => 32;
 }
 
 # The sandbox servers run with lock_wait_timeout=3 and it's not dynamic
@@ -396,6 +396,30 @@ like(
    "--where for upper oob chunk"
 );
 
+# #############################################################################
+# Bug 932442: column with 2 spaces
+# #############################################################################
+$sb->load_file('master', "t/pt-table-checksum/samples/2-space-col.sql");
+PerconaTest::wait_for_table($master_dbh, "test.t", "id=10");
+
+$output = output(
+   sub { $exit_status = pt_table_checksum::main(@args,
+      qw(-t test.t --chunk-size 3)) },
+   stderr => 1,
+);
+
+is(
+   $exit_status,
+   0,
+   "Bug 932442: 0 exit"
+);
+
+is(
+   PerconaTest::count_checksum_results($output, 'errors'),
+   0,
+   "Bug 932442: 0 errors"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################
diff --git a/t/pt-table-checksum/samples/2-space-col.sql b/t/pt-table-checksum/samples/2-space-col.sql
new file mode 100644
index 00000000..ff924789
--- /dev/null
+++ b/t/pt-table-checksum/samples/2-space-col.sql
@@ -0,0 +1,8 @@
+drop database if exists test;
+create database test;
+use test;
+create table t (
+  id int auto_increment primary key,
+  `a  b` int not null  -- 2 spaces between a and b
+);
+insert into t values (null, 1),(null, 2),(null, 3),(null, 4),(null, 5),(null, 6),(null, 7),(null, 8),(null, 9),(null, 10);