diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index e3457327..ede82b06 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -2335,12 +2335,18 @@ sub quote_val { sub split_unquote { my ( $self, $db_tbl, $default_db ) = @_; - $db_tbl =~ s/`//g; my ( $db, $tbl ) = split(/[.]/, $db_tbl); if ( !$tbl ) { $tbl = $db; $db = $default_db; } + for ($db, $tbl) { + next unless $_; + s/\A`//; + s/`\z//; + s/``/`/; + } + return ($db, $tbl); } @@ -3437,7 +3443,9 @@ sub is_cluster_node { PTDEBUG && _d($sql); my $row = $self->{dbh}->selectrow_arrayref($sql); PTDEBUG && _d(defined $row ? @$row : 'undef'); - $self->{is_cluster_node} = $row && $row->[0] ? 1 : 0; + $self->{is_cluster_node} = $row && $row->[1] + ? ($row->[1] eq 'ON' || $row->[1] eq '1') + : 0; return $self->{is_cluster_node}; } diff --git a/lib/Quoter.pm b/lib/Quoter.pm index a9916d7f..66b9e170 100644 --- a/lib/Quoter.pm +++ b/lib/Quoter.pm @@ -93,12 +93,18 @@ sub quote_val { # sub split_unquote { my ( $self, $db_tbl, $default_db ) = @_; - $db_tbl =~ s/`//g; my ( $db, $tbl ) = split(/[.]/, $db_tbl); if ( !$tbl ) { $tbl = $db; $db = $default_db; } + for ($db, $tbl) { + next unless $_; + s/\A`//; + s/`\z//; + s/``/`/; + } + return ($db, $tbl); } diff --git a/t/pt-online-schema-change/find_renamed_cols.t b/t/pt-online-schema-change/find_renamed_cols.t index f03651f3..b47cdba5 100644 --- a/t/pt-online-schema-change/find_renamed_cols.t +++ b/t/pt-online-schema-change/find_renamed_cols.t @@ -23,7 +23,8 @@ sub test_func { my ($alter, $renamed_cols) = @_; die "No alter arg" unless $alter; die "No renamed_cols arg" unless $renamed_cols; - + (my $show_alter = $alter) =~ s/\n/\\n/g; + my %got_renamed_cols = eval { pt_online_schema_change::find_renamed_cols($alter, $tp); }; @@ -31,14 +32,14 @@ sub test_func { is_deeply( undef, $renamed_cols, - $alter, + $show_alter, ) or diag($EVAL_ERROR); } else { is_deeply( \%got_renamed_cols, $renamed_cols, - $alter, + $show_alter, ) or diag(Dumper(\%got_renamed_cols)); } } @@ -62,6 +63,14 @@ test_func( }, ); +# Optional COLUMN? +test_func( + "CHANGE column old_column_name new_column_name VARCHAR(255) NULL", + { + old_column_name => 'new_column_name', + }, +); + # Space-sensitive? test_func( "CHANGE a z VARCHAR(255) NULL", @@ -78,6 +87,15 @@ test_func( }, ); +# Column named column? +test_func( + "CHANGE `column` new_column_name VARCHAR(255) NULL", + { + column => 'new_column_name', + }, +); + + # Extended ascii? test_func( "CHANGE `café` `tête-à-tête` INT", @@ -86,6 +104,71 @@ test_func( }, ); +# UTF-8? +if( Test::Builder->VERSION < 2 ) { + foreach my $method ( qw(output failure_output) ) { + binmode Test::More->builder->$method(), ':encoding(UTF-8)'; + } +} +test_func( + "CHANGE `\x{30cb}` `\x{30cd}` INT", + { + "\x{30cb}" => "\x{30cd}", + }, +); +# Mixed backtick-sensitive? +test_func( + "CHANGE `a` z VARCHAR(255) NULL", + { + a => 'z', + }, +); + +test_func( + "CHANGE a `z` VARCHAR(255) NULL", + { + a => 'z', + }, +); + +# Ansi quotes-sensitive? (should matter) +test_func( + qq{CHANGE "a" "z" VARCHAR(255) NULL}, + { + a => 'z', + }, +); + +# Embedded backticks? +test_func( + "CHANGE `a``a` z VARCHAR(255) NULL", + { + 'a`a' => 'z', + }, +); + +# Emebedded spaces? +test_func( + "CHANGE `a yes ` z VARCHAR(255) NULL", + { + 'a yes ' => 'z', + }, +); + +test_func( + "CHANGE ` yes ` `\nyes!\na` VARCHAR(255) NULL", + { + ' yes ' => "\nyes!\na", + }, +); + +test_func( + "CHANGE ` yes ` `\nyes!\na` VARCHAR(255) NULL", + { + ' yes ' => "\nyes!\na", + }, +); + # ############################################################################# # Two column alters # ############################################################################# @@ -98,6 +181,16 @@ test_func( }, ); +# Pathological +test_func( + "CHANGE a `CHANGE a z VARCHAR(255) NOT NULL` VARCHAR(255) NULL, CHANGE foo bar INT", + { + a => 'CHANGE a z VARCHAR(255) NOT NULL', + foo => 'bar', + }, +); + + # ############################################################################# # Fake alters # ############################################################################# @@ -109,6 +202,23 @@ test_func( }, ); +# Not really renamed, should ignore case +test_func( + "CHANGE foo FOO FLOAT", + { + }, +); + +TODO: { + local $::TODO = "We don't parse the entire alter statement, what looks like a CHANGE COLUMNS"; + # Not really an alter, pathological + test_func( + "MODIFY `CHANGE a z VARCHAR(255) NOT NULL` FLOAT", + { + }, + ); +} + # ############################################################################# # Done. # #############################################################################