From c375fd068b1ed4a6a83d632e6e22b7673b269ff7 Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Mon, 14 Aug 2017 21:01:30 -0300 Subject: [PATCH 1/2] PT-193 Fixed regex in TableParser TableParser's parse function was failing while trying to lowercase column names in the provided 'SHOW CREATE TABLE'. The problem was it was trying to lowercase everything between backticks but lines like these: `field_name` int comment "here is a ` in the comment" `second_field_name` int made the original regex to fail, matching `in the coment"` as an expression to be lowercased while second_file_name was considered as outside backticks. --- lib/TableParser.pm | 2 +- t/lib/TableParser.t | 26 +++++++++++++++++++ .../issue_pt-193_backtick_in_col_comments.sql | 6 +++++ 3 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 t/lib/samples/issue_pt-193_backtick_in_col_comments.sql diff --git a/lib/TableParser.pm b/lib/TableParser.pm index 2f7521f6..4b5904bb 100644 --- a/lib/TableParser.pm +++ b/lib/TableParser.pm @@ -145,7 +145,7 @@ sub parse { # Lowercase identifiers to avoid issues with case-sensitivity in Perl. # (Bug #1910276). - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); diff --git a/t/lib/TableParser.t b/t/lib/TableParser.t index 52bdd52b..8a063ee6 100644 --- a/t/lib/TableParser.t +++ b/t/lib/TableParser.t @@ -806,6 +806,32 @@ is_deeply( 'issue with pairing backticks in column comments (issue 330)' ); + +$tbl = $tp->parse( load_file('t/lib/samples/issue_pt-193_backtick_in_col_comments.sql') ); +is_deeply( + $tbl, + { cols => [qw(id f22abcde f23abc)], + col_posn => { id => 0, f22abcde => 1, f23abc => 2 }, + is_col => { id => 1, f22abcde => 1, f23abc => 1 }, + is_autoinc => { id => 1, f22abcde => 0, f23abc => 0 }, + null_cols => [qw(f22abcde)], + is_nullable => { f22abcde => 1}, + clustered_key => undef, + keys => {}, + defs => { id => " `id` int(11) NOT NULL AUTO_INCREMENT", + "f22abcde" => " `f22abcde` int(10) unsigned DEFAULT NULL COMMENT 'xxx`XXx'", + "f23abc" => " `f23abc` int(10) unsigned NOT NULL DEFAULT '255' COMMENT \"`yyy\"" + }, + numeric_cols => [qw(id f22abcde f23abc)], + is_numeric => { id => 1, f22abcde => 1, f23abc => 1 }, + engine => 'InnoDB', + type_for => { id => 'int', f22abcde => 'int', f23abc => 'int' }, + name => 't3', + charset => 'latin1', + }, + 'issue with pairing backticks in column comments (issue 330)' +); + # ############################################################################# # Issue 170: mk-parallel-dump dies when table-status Data_length is NULL # ############################################################################# diff --git a/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql b/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql new file mode 100644 index 00000000..e6f123d3 --- /dev/null +++ b/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql @@ -0,0 +1,6 @@ +Create Table: CREATE TABLE `t3` ( + `id` int(11) NOT NULL AUTO_INCREMENT, + `f22aBcDe` int(10) unsigned DEFAULT NULL COMMENT 'xxx`XXx', + `f23aBc` int(10) unsigned NOT NULL DEFAULT '255' COMMENT "`yyy", + PRIMARY KEY (`id`) +) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=latin1) From 54dfcf36de2e2aaf0e6a59cec385457ac816ba2a Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Tue, 15 Aug 2017 12:06:24 -0300 Subject: [PATCH 2/2] PT-193 New test and updated libraries Added a specific test for this issue and updated all binaries to the latest TableParser version --- bin/pt-archiver | 9 ++- bin/pt-duplicate-key-checker | 9 ++- bin/pt-find | 9 ++- bin/pt-heartbeat | 9 ++- bin/pt-index-usage | 9 ++- bin/pt-kill | 9 ++- bin/pt-online-schema-change | 2 +- bin/pt-query-digest | 9 ++- bin/pt-table-checksum | 2 +- bin/pt-table-sync | 9 ++- bin/pt-table-usage | 9 ++- .../issue_pt-193_backtick_in_col_comments.sql | 9 ++- t/pt-table-checksum/pt-193.t | 66 +++++++++++++++++++ 13 files changed, 129 insertions(+), 31 deletions(-) create mode 100644 t/pt-table-checksum/pt-193.t diff --git a/bin/pt-archiver b/bin/pt-archiver index 6fac0681..768af530 100755 --- a/bin/pt-archiver +++ b/bin/pt-archiver @@ -1954,7 +1954,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -2085,6 +2085,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -2092,8 +2094,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 3b4c8f9a..f288a596 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -341,7 +341,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -472,6 +472,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -479,8 +481,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-find b/bin/pt-find index 444d6042..4f9c7bf4 100755 --- a/bin/pt-find +++ b/bin/pt-find @@ -1869,7 +1869,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -2000,6 +2000,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -2007,8 +2009,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index 412ef46d..667b037a 100755 --- a/bin/pt-heartbeat +++ b/bin/pt-heartbeat @@ -3488,7 +3488,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -3619,6 +3619,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -3626,8 +3628,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-index-usage b/bin/pt-index-usage index 3a95a702..c58cacaa 100755 --- a/bin/pt-index-usage +++ b/bin/pt-index-usage @@ -3108,7 +3108,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -3239,6 +3239,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -3246,8 +3248,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-kill b/bin/pt-kill index a3c98aaa..6d1fb47e 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -2934,7 +2934,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -3065,6 +3065,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -3072,8 +3074,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 4974fd1c..95cfbef9 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -3298,7 +3298,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); diff --git a/bin/pt-query-digest b/bin/pt-query-digest index 3ede8cb7..b90f3426 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -8834,7 +8834,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -8965,6 +8965,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -8972,8 +8974,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 70eb2051..558fe6e3 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -4435,7 +4435,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); diff --git a/bin/pt-table-sync b/bin/pt-table-sync index 79887322..0f2bdd05 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -2853,7 +2853,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -2984,6 +2984,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -2991,8 +2993,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/bin/pt-table-usage b/bin/pt-table-usage index 91831b4a..5dcdd8e9 100755 --- a/bin/pt-table-usage +++ b/bin/pt-table-usage @@ -6784,7 +6784,7 @@ sub parse { my ($name) = $ddl =~ m/CREATE (?:TEMPORARY )?TABLE\s+(`.+?`)/; (undef, $name) = $self->{Quoter}->split_unquote($name) if $name; - $ddl =~ s/(`[^`]+`)/\L$1/g; + $ddl =~ s/(`[^`\n]+`)/\L$1/gm; my $engine = $self->get_engine($ddl); @@ -6915,6 +6915,8 @@ sub check_table { my $db_tbl = $q->quote($db, $tbl); PTDEBUG && _d('Checking', $db_tbl); + $self->{check_table_error} = undef; + my $sql = "SHOW TABLES FROM " . $q->quote($db) . ' LIKE ' . $q->literal_like($tbl); PTDEBUG && _d($sql); @@ -6922,8 +6924,9 @@ sub check_table { eval { $row = $dbh->selectrow_arrayref($sql); }; - if ( $EVAL_ERROR ) { - PTDEBUG && _d($EVAL_ERROR); + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; return 0; } if ( !$row->[0] || $row->[0] ne $tbl ) { diff --git a/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql b/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql index e6f123d3..6f37b92c 100644 --- a/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql +++ b/t/lib/samples/issue_pt-193_backtick_in_col_comments.sql @@ -1,6 +1,11 @@ -Create Table: CREATE TABLE `t3` ( +DROP DATABASE IF EXISTS test; +CREATE DATABASE test; +USE test; + +CREATE TABLE `t3` ( `id` int(11) NOT NULL AUTO_INCREMENT, `f22aBcDe` int(10) unsigned DEFAULT NULL COMMENT 'xxx`XXx', `f23aBc` int(10) unsigned NOT NULL DEFAULT '255' COMMENT "`yyy", PRIMARY KEY (`id`) -) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=latin1) +) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=latin1; + diff --git a/t/pt-table-checksum/pt-193.t b/t/pt-table-checksum/pt-193.t new file mode 100644 index 00000000..7f3af8d6 --- /dev/null +++ b/t/pt-table-checksum/pt-193.t @@ -0,0 +1,66 @@ +#!/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; +use SqlModes; +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 => 3; +} + +$sb->load_file('master', 't/lib/samples/issue_pt-193_backtick_in_col_comments.sql'); + +# The sandbox servers run with lock_wait_timeout=3 and it's not dynamic +# so we need to specify --set-vars innodb_lock_wait_timeout=3 else the tool will die. +# And --max-load "" prevents waiting for status variables. +my $master_dsn = $sb->dsn_for('master'); +my @args = ($master_dsn, "--set-vars", "innodb_lock_wait_timeout=50", + "--no-check-binlog-format", "--ignore-databases", "mysql", + "--nocheck-replication-filters"); +my $output; +my $exit_status; + +# Test #1 +$output = output( + sub { $exit_status = pt_table_checksum::main(@args) }, + stderr => 1, +); + +is( + $exit_status, + 0, + "PT-193 use single backtick in comments", +); + +like( + $output, + qr/test\.t3/, + "PT-193 table t3 was checksumed", +); + + +# ############################################################################# +# Done. +# ############################################################################# +$sb->wipe_clean($dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +exit;