From 8d38d0900aa1da2eb1736cdcddca720b2914e3ad Mon Sep 17 00:00:00 2001 From: Ivan Kruglov Date: Wed, 8 Nov 2023 20:16:31 +0100 Subject: [PATCH 1/2] PT-1860 make pt-osc respect case insesitive lookup on Windows and osx --- lib/TableParser.pm | 26 +++++- t/pt-online-schema-change/case-insensitive.t | 84 ++++++++++++++++++++ 2 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 t/pt-online-schema-change/case-insensitive.t diff --git a/lib/TableParser.pm b/lib/TableParser.pm index eba7d153..6f6e4fcd 100644 --- a/lib/TableParser.pm +++ b/lib/TableParser.pm @@ -325,11 +325,33 @@ sub check_table { } my ($dbh, $db, $tbl) = @args{@required_args}; my $q = $self->{Quoter} || 'Quoter'; + $self->{check_table_error} = undef; + + # https://dev.mysql.com/doc/refman/8.0/en/identifier-case-sensitivity.html + # MySQL may use use case-insensitive table lookup, this is controller by + # @@lower_case_table_names. 0 means case sensitive search, 1 or 2 means + # case insensitive lookup. + + my $lctn_sql = 'SELECT @@lower_case_table_names'; + PTDEBUG && _d($lctn_sql); + + my $lower_case_table_names; + eval { ($lower_case_table_names) = $dbh->selectrow_array($lctn_sql); }; + if ( my $e = $EVAL_ERROR ) { + PTDEBUG && _d($e); + $self->{check_table_error} = $e; + return 0; + } + + PTDEBUG && _d("lower_case_table_names=$lower_case_table_names"); + if ($lower_case_table_names > 0) { + PTDEBUG && _d("MySQL uses case-insensitive lookup, converting '$tbl' to lowercase"); + $tbl = lc $tbl; + } + 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); diff --git a/t/pt-online-schema-change/case-insensitive.t b/t/pt-online-schema-change/case-insensitive.t new file mode 100644 index 00000000..3b47bba1 --- /dev/null +++ b/t/pt-online-schema-change/case-insensitive.t @@ -0,0 +1,84 @@ +#!/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 Time::HiRes qw(sleep); + +$ENV{PTTEST_FAKE_TS} = 1; +$ENV{PERCONA_TOOLKIT_TEST_USE_DSN_NAMES} = 1; + +use PerconaTest; +use Sandbox; +require "$trunk/bin/pt-online-schema-change"; +require VersionParser; + +use Data::Dumper; +$Data::Dumper::Indent = 1; +$Data::Dumper::Sortkeys = 1; +$Data::Dumper::Quotekeys = 0; + +my $dp = new DSNParser(opts=>$dsn_opts); +my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); +my $master_dbh = $sb->get_dbh_for('master'); + +if ( !$master_dbh ) { + plan skip_all => 'Cannot connect to sandbox master'; +} + +my $q = new Quoter(); +my $tp = new TableParser(Quoter => $q); +my @args = qw(--set-vars innodb_lock_wait_timeout=3); +my $output = ""; +my $dsn = "h=127.1,P=12345,u=msandbox,p=msandbox"; +my $exit = 0; +my $sample = "t/pt-online-schema-change/samples"; +my $rows; + +# ############################################################################# +# Tool shouldn't run without --execute (bug 933232). +# ############################################################################# + +my ($lower_case_table_names) = $master_dbh->selectrow_array('SELECT @@lower_case_table_names'); +plan skip_all => 'MySQL uses case sensitive lookup' if $lower_case_table_names == 0; + +$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); + +($output, $exit) = full_output( + sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=T", + '--alter', 'drop column d', '--execute') } +); + +like( + $output, + qr/Successfully altered/, + "Table is altered using capital name 'T'" +) or diag($output); + +my $ddl = $master_dbh->selectrow_arrayref("show create table pt_osc.T"); +unlike( + $ddl->[1], + qr/^\s+["`]d["`]/m, + "'D' column is dropped" +); + +is( + $exit, + 0, + "Exit 0" +); + +# ############################################################################# +# Done. +# ############################################################################# + +$sb->wipe_clean($master_dbh); +ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); +done_testing; From 3e1e9b842545f698741ed20f45a8e620dd5fd707 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Thu, 9 Nov 2023 17:16:43 +0300 Subject: [PATCH 2/2] PT-1860 make pt-osc respect case insesitive lookup on Windows and osx #516 - Run update-modules - Adjusted fix, so it does not create extra variable to hold $EVAL_ERROR - Adjusted test case, so it runs on Linux --- bin/pt-online-schema-change | 22 +++++++++++-- lib/TableParser.pm | 6 ++-- t/pt-online-schema-change/case-insensitive.t | 31 ++++++++----------- .../samples/lower_case_table_names_1.cnf | 2 ++ 4 files changed, 38 insertions(+), 23 deletions(-) create mode 100644 t/pt-online-schema-change/samples/lower_case_table_names_1.cnf diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 6ab96703..9eb6b769 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -3470,11 +3470,29 @@ sub check_table { } my ($dbh, $db, $tbl) = @args{@required_args}; my $q = $self->{Quoter} || 'Quoter'; + $self->{check_table_error} = undef; + + + my $lctn_sql = 'SELECT @@lower_case_table_names'; + PTDEBUG && _d($lctn_sql); + + my $lower_case_table_names; + eval { ($lower_case_table_names) = $dbh->selectrow_array($lctn_sql); }; + if ( $EVAL_ERROR ) { + PTDEBUG && _d($EVAL_ERROR); + $self->{check_table_error} = $EVAL_ERROR; + return 0; + } + + PTDEBUG && _d("lower_case_table_names=$lower_case_table_names"); + if ($lower_case_table_names > 0) { + PTDEBUG && _d("MySQL uses case-insensitive lookup, converting '$tbl' to lowercase"); + $tbl = lc $tbl; + } + 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); diff --git a/lib/TableParser.pm b/lib/TableParser.pm index 58b30a66..896a13f4 100644 --- a/lib/TableParser.pm +++ b/lib/TableParser.pm @@ -339,9 +339,9 @@ sub check_table { my $lower_case_table_names; eval { ($lower_case_table_names) = $dbh->selectrow_array($lctn_sql); }; - if ( my $e = $EVAL_ERROR ) { - PTDEBUG && _d($e); - $self->{check_table_error} = $e; + if ( $EVAL_ERROR ) { + PTDEBUG && _d($EVAL_ERROR); + $self->{check_table_error} = $EVAL_ERROR; return 0; } diff --git a/t/pt-online-schema-change/case-insensitive.t b/t/pt-online-schema-change/case-insensitive.t index 3b47bba1..9aa026e1 100644 --- a/t/pt-online-schema-change/case-insensitive.t +++ b/t/pt-online-schema-change/case-insensitive.t @@ -12,19 +12,11 @@ use English qw(-no_match_vars); use Test::More; use Time::HiRes qw(sleep); -$ENV{PTTEST_FAKE_TS} = 1; -$ENV{PERCONA_TOOLKIT_TEST_USE_DSN_NAMES} = 1; - use PerconaTest; use Sandbox; require "$trunk/bin/pt-online-schema-change"; require VersionParser; -use Data::Dumper; -$Data::Dumper::Indent = 1; -$Data::Dumper::Sortkeys = 1; -$Data::Dumper::Quotekeys = 0; - my $dp = new DSNParser(opts=>$dsn_opts); my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); my $master_dbh = $sb->get_dbh_for('master'); @@ -33,23 +25,25 @@ if ( !$master_dbh ) { plan skip_all => 'Cannot connect to sandbox master'; } -my $q = new Quoter(); -my $tp = new TableParser(Quoter => $q); my @args = qw(--set-vars innodb_lock_wait_timeout=3); my $output = ""; my $dsn = "h=127.1,P=12345,u=msandbox,p=msandbox"; my $exit = 0; my $sample = "t/pt-online-schema-change/samples"; -my $rows; -# ############################################################################# -# Tool shouldn't run without --execute (bug 933232). -# ############################################################################# +my $lower_case_table_names = $master_dbh->selectrow_array('SELECT @@lower_case_table_names'); -my ($lower_case_table_names) = $master_dbh->selectrow_array('SELECT @@lower_case_table_names'); -plan skip_all => 'MySQL uses case sensitive lookup' if $lower_case_table_names == 0; +if ( $lower_case_table_names == 0) { + # Preparing test setup on Linux + diag(`$trunk/sandbox/stop-sandbox 12348 >/dev/null`); + diag(`EXTRA_DEFAULTS_FILE="$trunk/t/pt-online-schema-change/samples/lower_case_table_names_1.cnf" $trunk/sandbox/start-sandbox master 12348 >/dev/null`); -$sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); + $master_dbh = $sb->get_dbh_for('master1'); + $dsn = "h=127.1,P=12348,u=msandbox,p=msandbox"; + $sb->load_file('master1', "$sample/basic_no_fks_innodb.sql"); +} else { + $sb->load_file('master', "$sample/basic_no_fks_innodb.sql"); +} ($output, $exit) = full_output( sub { pt_online_schema_change::main(@args, "$dsn,D=pt_osc,t=T", @@ -78,7 +72,8 @@ is( # ############################################################################# # Done. # ############################################################################# - +diag(`$trunk/sandbox/stop-sandbox 12348 >/dev/null`); +$master_dbh = $sb->get_dbh_for('master'); $sb->wipe_clean($master_dbh); ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox"); done_testing; diff --git a/t/pt-online-schema-change/samples/lower_case_table_names_1.cnf b/t/pt-online-schema-change/samples/lower_case_table_names_1.cnf new file mode 100644 index 00000000..36313cc7 --- /dev/null +++ b/t/pt-online-schema-change/samples/lower_case_table_names_1.cnf @@ -0,0 +1,2 @@ +[mysqld] +lower_case_table_names=1