diff --git a/bin/pt-archiver b/bin/pt-archiver index 364992f4..6e4965e2 100755 --- a/bin/pt-archiver +++ b/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'"; diff --git a/bin/pt-deadlock-logger b/bin/pt-deadlock-logger index 8b77d81f..c9fdbf8d 100755 --- a/bin/pt-deadlock-logger +++ b/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'"; diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index a23256a1..97d81f68 100755 --- a/bin/pt-duplicate-key-checker +++ b/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'"; diff --git a/bin/pt-find b/bin/pt-find index eb9956c3..7a370127 100755 --- a/bin/pt-find +++ b/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'"; diff --git a/bin/pt-fk-error-logger b/bin/pt-fk-error-logger index f29d0a60..5024c340 100755 --- a/bin/pt-fk-error-logger +++ b/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'"; diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index 1b169695..9cdee033 100755 --- a/bin/pt-heartbeat +++ b/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'"; diff --git a/bin/pt-index-usage b/bin/pt-index-usage index 2930487c..e473edee 100755 --- a/bin/pt-index-usage +++ b/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'"; diff --git a/bin/pt-kill b/bin/pt-kill index 875a772a..dceffbac 100755 --- a/bin/pt-kill +++ b/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'"; diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index d6a83b59..ad4cb524 100755 --- a/bin/pt-online-schema-change +++ b/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'"; diff --git a/bin/pt-query-advisor b/bin/pt-query-advisor index c9fbabaa..32ee4565 100755 --- a/bin/pt-query-advisor +++ b/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'"; diff --git a/bin/pt-query-digest b/bin/pt-query-digest index f82509eb..4e8bdf96 100755 --- a/bin/pt-query-digest +++ b/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'"; diff --git a/bin/pt-slave-restart b/bin/pt-slave-restart index ff585fb5..f76d526e 100755 --- a/bin/pt-slave-restart +++ b/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'"; diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 1633a8bd..cc81c6b9 100755 --- a/bin/pt-table-checksum +++ b/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'"; diff --git a/bin/pt-table-sync b/bin/pt-table-sync index e733447e..6ee71bd6 100755 --- a/bin/pt-table-sync +++ b/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"; } @@ -3500,11 +3504,15 @@ sub make_row { else { @cols = $self->sort_cols($row); } - my $q = $self->{Quoter}; + 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); } diff --git a/bin/pt-table-usage b/bin/pt-table-usage index cabf587e..1806fd9d 100755 --- a/bin/pt-table-usage +++ b/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'"; diff --git a/bin/pt-upgrade b/bin/pt-upgrade index 3e4c5f69..29edff55 100755 --- a/bin/pt-upgrade +++ b/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"; } @@ -4739,11 +4743,15 @@ sub make_row { else { @cols = $self->sort_cols($row); } - my $q = $self->{Quoter}; + 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); } diff --git a/lib/ChangeHandler.pm b/lib/ChangeHandler.pm index cbba6f66..9314f362 100644 --- a/lib/ChangeHandler.pm +++ b/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"; } @@ -391,11 +394,15 @@ sub make_row { else { @cols = $self->sort_cols($row); } - my $q = $self->{Quoter}; + 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); } diff --git a/lib/Quoter.pm b/lib/Quoter.pm index 659d420f..a9916d7f 100644 --- a/lib/Quoter.pm +++ b/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; diff --git a/t/lib/ChangeHandler.t b/t/lib/ChangeHandler.t index 2f7aa676..1659b18e 100644 --- a/t/lib/ChangeHandler.t +++ b/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; + \ No newline at end of file diff --git a/t/lib/Quoter.t b/t/lib/Quoter.t index a843ecb9..6e3aaaae 100644 --- a/t/lib/Quoter.t +++ b/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; diff --git a/t/lib/samples/bug_1038276.sql b/t/lib/samples/bug_1038276.sql new file mode 100644 index 00000000..be577804 --- /dev/null +++ b/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");