Merged fix-1038276-changehandler-hex-looking-vals

bin/pt-archiver

@@ -2559,11 +2559,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-deadlock-logger

@@ -1741,11 +1741,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-duplicate-key-checker

@@ -76,11 +76,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-find

@@ -1470,11 +1470,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-fk-error-logger

@@ -1097,11 +1097,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-heartbeat

@@ -2396,11 +2396,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-index-usage

@@ -455,11 +455,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-kill

@@ -4148,11 +4148,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-online-schema-change

@@ -2318,11 +2318,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-query-advisor

@@ -1481,11 +1481,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-query-digest

@@ -473,11 +473,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-slave-restart

@@ -74,11 +74,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-table-checksum

@@ -3015,11 +3015,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-table-sync

@@ -1564,11 +1564,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";
@@ -3463,10 +3464,13 @@ sub make_UPDATE {
    else {
       @cols = $self->sort_cols($row);
    }
+   my $types = $self->{tbl_struct}->{type_for};
    return "UPDATE $self->{dst_db_tbl} SET "
       . join(', ', map {
+            my $is_char = ($types->{$_} || '') =~ m/char|text/i;
             $self->{Quoter}->quote($_)
-            . '=' .  $self->{Quoter}->quote_val($row->{$_})
+            . '=' .  $self->{Quoter}->quote_val($row->{$_},
+                                              is_char => $is_char);
          } grep { !$in_where{$_} } @cols)
       . " WHERE $where LIMIT 1";
 }
@@ -3501,10 +3505,14 @@ sub make_row {
       @cols = $self->sort_cols($row);
    }
    my $q     = $self->{Quoter};
+   my $type_for = $self->{tbl_struct}->{type_for};
    return "$verb INTO $self->{dst_db_tbl}("
       . join(', ', map { $q->quote($_) } @cols)
       . ') VALUES ('
-      . join(', ', map { $q->quote_val($_) } @{$row}{@cols} )
+      . join(', ', map {
+               my $is_char = ($type_for->{$_} || '') =~ m/char|text/i;
+               $q->quote_val($row->{$_},
+                           is_char => $is_char) } @cols )
       . ')';
 }
 
@@ -3513,7 +3521,9 @@ sub make_where_clause {
    my @clauses = map {
       my $val = $row->{$_};
       my $sep = defined $val ? '=' : ' IS ';
-      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val);
+      my $is_char = ($self->{tbl_struct}->{type_for}->{$_} || '') =~ m/char|text/i;
+      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val,
+                                              is_char => $is_char);
    } @$cols;
    return join(' AND ', @clauses);
 }

bin/pt-table-usage

@@ -5486,11 +5486,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-upgrade

@@ -909,11 +909,12 @@ sub quote {
 }
 
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";
@@ -4702,10 +4703,13 @@ sub make_UPDATE {
    else {
       @cols = $self->sort_cols($row);
    }
+   my $types = $self->{tbl_struct}->{type_for};
    return "UPDATE $self->{dst_db_tbl} SET "
       . join(', ', map {
+            my $is_char = ($types->{$_} || '') =~ m/char|text/i;
             $self->{Quoter}->quote($_)
-            . '=' .  $self->{Quoter}->quote_val($row->{$_})
+            . '=' .  $self->{Quoter}->quote_val($row->{$_},
+                                              is_char => $is_char);
          } grep { !$in_where{$_} } @cols)
       . " WHERE $where LIMIT 1";
 }
@@ -4740,10 +4744,14 @@ sub make_row {
       @cols = $self->sort_cols($row);
    }
    my $q     = $self->{Quoter};
+   my $type_for = $self->{tbl_struct}->{type_for};
    return "$verb INTO $self->{dst_db_tbl}("
       . join(', ', map { $q->quote($_) } @cols)
       . ') VALUES ('
-      . join(', ', map { $q->quote_val($_) } @{$row}{@cols} )
+      . join(', ', map {
+               my $is_char = ($type_for->{$_} || '') =~ m/char|text/i;
+               $q->quote_val($row->{$_},
+                           is_char => $is_char) } @cols )
       . ')';
 }
 
@@ -4752,7 +4760,9 @@ sub make_where_clause {
    my @clauses = map {
       my $val = $row->{$_};
       my $sep = defined $val ? '=' : ' IS ';
-      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val);
+      my $is_char = ($self->{tbl_struct}->{type_for}->{$_} || '') =~ m/char|text/i;
+      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val,
+                                              is_char => $is_char);
    } @$cols;
    return join(' AND ', @clauses);
 }

lib/ChangeHandler.pm

@@ -323,10 +323,13 @@ sub make_UPDATE {
    else {
       @cols = $self->sort_cols($row);
    }
+   my $types = $self->{tbl_struct}->{type_for};
    return "UPDATE $self->{dst_db_tbl} SET "
       . join(', ', map {
+            my $is_char = ($types->{$_} || '') =~ m/char|text/i;
             $self->{Quoter}->quote($_)
-            . '=' .  $self->{Quoter}->quote_val($row->{$_})
+            . '=' .  $self->{Quoter}->quote_val($row->{$_},
+                                              is_char => $is_char);
          } grep { !$in_where{$_} } @cols)
       . " WHERE $where LIMIT 1";
 }
@@ -392,10 +395,14 @@ sub make_row {
       @cols = $self->sort_cols($row);
    }
    my $q     = $self->{Quoter};
+   my $type_for = $self->{tbl_struct}->{type_for};
    return "$verb INTO $self->{dst_db_tbl}("
       . join(', ', map { $q->quote($_) } @cols)
       . ') VALUES ('
-      . join(', ', map { $q->quote_val($_) } @{$row}{@cols} )
+      . join(', ', map {
+               my $is_char = ($type_for->{$_} || '') =~ m/char|text/i;
+               $q->quote_val($row->{$_},
+                           is_char => $is_char) } @cols )
       . ')';
 }
 
@@ -413,7 +420,9 @@ sub make_where_clause {
    my @clauses = map {
       my $val = $row->{$_};
       my $sep = defined $val ? '=' : ' IS ';
-      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val);
+      my $is_char = ($self->{tbl_struct}->{type_for}->{$_} || '') =~ m/char|text/i;
+      $self->{Quoter}->quote($_) . $sep . $self->{Quoter}->quote_val($val,
+                                              is_char => $is_char);
    } @$cols;
    return join(' AND ', @clauses);
 }

lib/Quoter.pm

@@ -65,11 +65,12 @@ sub quote {
 # Returns:
 #   Quoted value
 sub quote_val {
-   my ( $self, $val ) = @_;
+   my ( $self, $val, %args ) = @_;
 
    return 'NULL' unless defined $val;          # undef = NULL
    return "''" if $val eq '';                  # blank string = ''
-   return $val if $val =~ m/^0x[0-9a-fA-F]+$/;  # hex data
+   return $val if $val =~ m/^0x[0-9a-fA-F]+$/  # quote hex data
+                  && !$args{is_char};          # unless is_char is true
 
    # Quote and return non-numeric vals.
    $val =~ s/(['\\])/\\$1/g;

t/lib/ChangeHandler.t

@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 33;
+use Test::More;
 
 use ChangeHandler;
 use Quoter;
@@ -468,10 +468,52 @@ is_deeply(
    "process_rows() appends trace msg to SQL statements"
 );
 
+# #############################################################################
+# ChangeHandler doesn't quote varchar columns with hex-looking values
+# https://bugs.launchpad.net/percona-toolkit/+bug/1038276
+# #############################################################################
+SKIP: {
+   skip 'Cannot connect to sandbox master', 1 unless $master_dbh;
+   $sb->load_file('master', "t/lib/samples/bug_1038276.sql");
+
+   @rows = ();
+   $tbl_struct = {
+      cols      => [qw(id b)],
+      col_posn  => {id=>0, b=>1},
+      type_for  => {id=>'int', b=>'varchar'},
+   };
+   $ch = new ChangeHandler(
+      Quoter     => $q,
+      left_db    => 'bug_1038276',
+      left_tbl   => 'lt',
+      right_db   => 'bug_1038276',
+      right_tbl  => 'rt',
+      actions   => [ sub { push @rows, $_[0]; } ],
+      replace    => 0,
+      queue      => 0,
+      tbl_struct => $tbl_struct,
+   );
+   $ch->fetch_back($master_dbh);
+
+   $ch->change('UPDATE', {id=>1}, [qw(id)] );
+   $ch->change('INSERT', {id=>1}, [qw(id)] );
+
+   is_deeply(
+      \@rows,
+      [
+         "UPDATE `bug_1038276`.`rt` SET `b`='0x89504E470D0A1A0A0000000D4948445200000079000000750802000000E55AD965000000097048597300000EC300000EC301C76FA8640000200049444154789C4CBB7794246779FFBBF78F7B7EBE466177677772CE3D9D667AA67BA62776CE39545557CE3974EE9EB049AB9556392210414258083' WHERE `id`='1' LIMIT 1",
+         "INSERT INTO `bug_1038276`.`rt`(`id`, `b`) VALUES ('1', '0x89504E470D0A1A0A0000000D4948445200000079000000750802000000E55AD965000000097048597300000EC300000EC301C76FA8640000200049444154789C4CBB7794246779FFBBF78F7B7EBE466177677772CE3D9D667AA67BA62776CE39545557CE3974EE9EB049AB9556392210414258083')",
+      ],
+      "UPDATE and INSERT quote data regardless of how it looks if tbl_struct->quote_val is true"
+   );
+}
+
 # #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($master_dbh);
 $sb->wipe_clean($slave1_dbh);
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
-exit;
+
+done_testing;
+   

t/lib/Quoter.t

@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 55;
+use Test::More;
 
 use Quoter;
 use PerconaTest;
@@ -59,6 +59,8 @@ is( $q->quote_val('\\\''), "'\\\\\\\''", 'embedded backslash');
 is( $q->quote_val('123-abc'), "'123-abc'", 'looks numeric but is string');
 is( $q->quote_val('123abc'), "'123abc'", 'looks numeric but is string');
 is( $q->quote_val('0x89504E470'), '0x89504E470', 'hex string');
+is( $q->quote_val('0x89504E470', is_char => 0), '0x89504E470', 'hex string, with is_char => 0');
+is( $q->quote_val('0x89504E470', is_char => 1), "'0x89504E470'", 'hex string, with is_char => 1');
 is( $q->quote_val('0x89504I470'), "'0x89504I470'", 'looks like hex string');
 is( $q->quote_val('eastside0x3'), "'eastside0x3'", 'looks like hex str (issue 1110');
 
@@ -219,4 +221,5 @@ SKIP: {
 # Done.
 # ###########################################################################
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
-exit;
+
+done_testing;

t/lib/samples/bug_1038276.sql

@@ -0,0 +1,12 @@
+DROP DATABASE IF EXISTS bug_1038276;
+CREATE DATABASE bug_1038276;
+USE bug_1038276;
+CREATE TABLE lt (
+  id int not null auto_increment primary key,
+  b  varchar(300)
+) ENGINE=InnoDB;
+CREATE TABLE rt (
+  id int not null auto_increment primary key,
+  b  varchar(300)
+) ENGINE=InnoDB;
+INSERT INTO lt VALUES (null, "0x89504E470D0A1A0A0000000D4948445200000079000000750802000000E55AD965000000097048597300000EC300000EC301C76FA8640000200049444154789C4CBB7794246779FFBBF78F7B7EBE466177677772CE3D9D667AA67BA62776CE39545557CE3974EE9EB049AB9556392210414258083");