From 0c50d5b1cd824b6c066e9eeb531b77aa3762a026 Mon Sep 17 00:00:00 2001 From: Frank Cizmich Date: Mon, 23 Mar 2015 14:00:01 -0300 Subject: [PATCH 1/3] pt-duplicate-key-checker skipped reporting when verbose option on --- bin/pt-duplicate-key-checker | 78 +++++++++---------- t/pt-duplicate-key-checker/basics.t | 6 +- .../samples/simple_dupe_bug_1217013.txt | 4 + 3 files changed, 46 insertions(+), 42 deletions(-) diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 6600b7c9..38f31548 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -4972,46 +4972,44 @@ sub main { print_all_keys($keys, $tbl, \%seen_tbl) if $keys; print_all_keys($fks, $tbl, \%seen_tbl) if $fks; } - else { - PTDEBUG && _d('Getting duplicate keys on', - $tbl->{db}, $tbl->{tbl}); - if ( $keys ) { - $dk->get_duplicate_keys( - $keys, - clustered_key => $clustered_key, - tbl_info => $tbl, - callback => \&print_duplicate_key, - %tp_opts, - # get_duplicate_keys() ignores these args but passes them - # to the callback: - dbh => $dbh, - is_fk => 0, - o => $o, - ks => $ks, - tp => $tp, - q => $q, - seen_tbl => \%seen_tbl, - summary => \%summary, - ); - } - if ( $fks ) { - $dk->get_duplicate_fks( - $fks, - tbl_info => $tbl, - callback => \&print_duplicate_key, - %tp_opts, - # get_duplicate_fks() ignores these args but passes them - # to the callback: - dbh => $dbh, - is_fk => 1, - o => $o, - ks => $ks, - tp => $tp, - q => $q, - seen_tbl => \%seen_tbl, - summary => \%summary, - ); - } + PTDEBUG && _d('Getting duplicate keys on', + $tbl->{db}, $tbl->{tbl}); + if ( $keys ) { + $dk->get_duplicate_keys( + $keys, + clustered_key => $clustered_key, + tbl_info => $tbl, + callback => \&print_duplicate_key, + %tp_opts, + # get_duplicate_keys() ignores these args but passes them + # to the callback: + dbh => $dbh, + is_fk => 0, + o => $o, + ks => $ks, + tp => $tp, + q => $q, + seen_tbl => \%seen_tbl, + summary => \%summary, + ); + } + if ( $fks ) { + $dk->get_duplicate_fks( + $fks, + tbl_info => $tbl, + callback => \&print_duplicate_key, + %tp_opts, + # get_duplicate_fks() ignores these args but passes them + # to the callback: + dbh => $dbh, + is_fk => 1, + o => $o, + ks => $ks, + tp => $tp, + q => $q, + seen_tbl => \%seen_tbl, + summary => \%summary, + ); } # Always count Total Keys so print_key_summary won't die diff --git a/t/pt-duplicate-key-checker/basics.t b/t/pt-duplicate-key-checker/basics.t index 3999ee78..ac730a0e 100644 --- a/t/pt-duplicate-key-checker/basics.t +++ b/t/pt-duplicate-key-checker/basics.t @@ -136,14 +136,16 @@ ok( # ############################################################################# # Exact unique dupes # https://bugs.launchpad.net/percona-toolkit/+bug/1217013 +# Also added --verbose option (-v) to test for: +# https://bugs.launchpad.net/percona-toolkit/+bug/1402730 # ############################################################################# $sb->load_file('master', 't/lib/samples/dupekeys/simple_dupe_bug_1217013.sql', 'test'); ok( no_diff( - sub { pt_duplicate_key_checker::main(@args, qw(-t test.domains)) }, - "$sample/simple_dupe_bug_1217013.txt"), + sub { pt_duplicate_key_checker::main(@args, qw(-t test.domains -v)) }, + "$sample/simple_dupe_bug_1217013.txt", keep_output=>1), 'Exact unique dupes (bug 1217013)' ) or diag($test_diff); diff --git a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt index 628b1a61..b8a73b69 100644 --- a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt +++ b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt @@ -2,6 +2,10 @@ # test.domains # ######################################################################## +# unique_key_domain (`domain`) +# domain (`domain`) +# PRIMARY (`id`) + # Uniqueness of domain ignored because unique_key_domain is a duplicate constraint # domain is a duplicate of unique_key_domain # Key definitions: From 793ff24a6944df0930e5f58b49973b1010cdabef Mon Sep 17 00:00:00 2001 From: Frank Cizmich Date: Wed, 25 Mar 2015 15:02:22 -0300 Subject: [PATCH 2/3] Added warning about size of fulltext keys not being computed, if present. Sort key report by key name for ease of testing. --- bin/pt-duplicate-key-checker | 28 +++++++++++++------ t/pt-duplicate-key-checker/basics.t | 15 ++++++++-- .../samples/simple_dupe_bug_1217013.txt | 4 +-- 3 files changed, 34 insertions(+), 13 deletions(-) diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 38f31548..56925bbb 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -4881,7 +4881,7 @@ my $hdr_fmt = "# %-${hdr_width}s\n"; sub main { local @ARGV = @_; # set global ARGV for this package - my %summary = ( 'Total Indexes' => 0 ); + my %summary = ( 'items' => {'Total Indexes' => 0} ); my %seen_tbl; my $q = new Quoter(); @@ -5014,7 +5014,7 @@ sub main { # Always count Total Keys so print_key_summary won't die # because %summary is empty. - $summary{'Total Indexes'} += (scalar keys %$keys) + $summary{items}->{'Total Indexes'} += (scalar keys %$keys) + (scalar keys %$fks) } }; @@ -5042,7 +5042,8 @@ sub print_all_keys { printf $hdr_fmt, "$db.$tbl"; printf $hdr_fmt, ('#' x $hdr_width); } - foreach my $key ( values %$keys ) { + # Print keys sorted by name (easier to test) + foreach my $key ( sort {$a->{name} cmp $b->{name}} values %$keys ) { print "\n# $key->{name} ($key->{colnames})"; } print "\n"; @@ -5067,6 +5068,10 @@ sub print_duplicate_key { my $summary = $args{summary}; my $struct = $tp->parse($args{tbl_info}->{ddl}); + if ($dupe->{ddl} =~ /FULLTEXT/) { + $summary->{has_fulltext_dupe}++; + } + if ( !$seen_tbl->{"$db$tbl"}++ ) { printf $hdr_fmt, ('#' x $hdr_width); printf $hdr_fmt, "$db.$tbl"; @@ -5109,7 +5114,7 @@ sub print_duplicate_key { print "\n"; if ( $o->get('summary') && $summary ) { - $summary->{'Total Duplicate Indexes'} += 1; + $summary->{'items'}->{'Total Duplicate Indexes'} += 1; my ($size, $chosen_key) = $ks->get_key_size( name => $dupe->{key}, cols => $dupe->{cols}, @@ -5126,7 +5131,7 @@ sub print_duplicate_key { $size ||= 0; # Create Size Duplicate Keys summary even if there's no valid keys. - $summary->{'Size Duplicate Indexes'} += $size; + $summary->{'items'}->{'Size Duplicate Indexes'} += $size; if ( $size ) { if ( $chosen_key && $chosen_key ne $dupe->{key} ) { @@ -5141,14 +5146,19 @@ sub print_duplicate_key { sub print_key_summary { my ( %summary ) = @_; + my $items = $summary{items}; printf $hdr_fmt, ('#' x $hdr_width); printf $hdr_fmt, 'Summary of indexes'; printf $hdr_fmt, ('#' x $hdr_width); print "\n"; - my $max_item = max(map { length($_) } keys %summary); - my $line_fmt = "# %-${max_item}s %-s\n"; - foreach my $item ( sort keys %summary ) { - printf $line_fmt, $item, $summary{$item}; + my $max_item = max(map { length($_) } keys %$items); + my $line_fmt = "# %-${max_item}s %-s"; + foreach my $item ( sort keys %$items ) { + printf $line_fmt, $item, $items->{$item}; + if ( $item eq 'Size Duplicate Indexes' && $summary{has_fulltext_dupe} ) { + print ' (not including FULLTEXT indexes)'; + } + print "\n"; } return; } diff --git a/t/pt-duplicate-key-checker/basics.t b/t/pt-duplicate-key-checker/basics.t index ac730a0e..45a014a8 100644 --- a/t/pt-duplicate-key-checker/basics.t +++ b/t/pt-duplicate-key-checker/basics.t @@ -136,8 +136,6 @@ ok( # ############################################################################# # Exact unique dupes # https://bugs.launchpad.net/percona-toolkit/+bug/1217013 -# Also added --verbose option (-v) to test for: -# https://bugs.launchpad.net/percona-toolkit/+bug/1402730 # ############################################################################# $sb->load_file('master', 't/lib/samples/dupekeys/simple_dupe_bug_1217013.sql', 'test'); @@ -149,6 +147,19 @@ ok( 'Exact unique dupes (bug 1217013)' ) or diag($test_diff); +# ############################################################################# +# Same thing, but added --verbose option to test for: +# https://bugs.launchpad.net/percona-toolkit/+bug/1402730 +# ############################################################################# + +ok( + no_diff( + sub { pt_duplicate_key_checker::main(@args, qw(-t test.domains --verbose)) }, + "$sample/simple_dupe_bug_1217013.txt", keep_output=>1), + q[--verbose option doesn't skip dupes reporting (bug 1402730)] +) or diag($test_diff); + + # ############################################################################# # Done. # ############################################################################# diff --git a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt index b8a73b69..4d5d0c4d 100644 --- a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt +++ b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt @@ -2,9 +2,9 @@ # test.domains # ######################################################################## -# unique_key_domain (`domain`) -# domain (`domain`) # PRIMARY (`id`) +# domain (`domain`) +# unique_key_domain (`domain`) # Uniqueness of domain ignored because unique_key_domain is a duplicate constraint # domain is a duplicate of unique_key_domain From 2459053634cb9c60b482802a2b0a062186d1086e Mon Sep 17 00:00:00 2001 From: Frank Cizmich Date: Thu, 26 Mar 2015 15:42:11 -0300 Subject: [PATCH 3/3] Added sort to some hash loops to make tests consistent. Modified test files accordingly --- bin/pt-duplicate-key-checker | 6 ++++-- t/pt-duplicate-key-checker/issue_331.t | 4 ++-- t/pt-duplicate-key-checker/samples/issue_331.txt | 6 +++--- .../samples/simple_dupe_bug_1217013.txt | 8 ++++---- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 56925bbb..575a8b0c 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -2336,7 +2336,8 @@ sub get_duplicate_keys { my @dupes; KEY: - foreach my $key ( values %keys ) { + # sort by key name for consistent testing + foreach my $key ( sort {$a->{name} cmp $b->{name}} values %keys ) { $key->{real_cols} = [ @{$key->{cols}} ]; $key->{len_cols} = length $key->{colnames}; @@ -2412,7 +2413,8 @@ sub get_duplicate_keys { sub get_duplicate_fks { my ( $self, $fks, %args ) = @_; die "I need a fks argument" unless $fks; - my @fks = values %$fks; + # sort by name for consistent testing + my @fks = sort {$a->{name} cmp $b->{name}} values %$fks; my @dupes; foreach my $i ( 0..$#fks - 1 ) { diff --git a/t/pt-duplicate-key-checker/issue_331.t b/t/pt-duplicate-key-checker/issue_331.t index d3151084..aa46c7ab 100644 --- a/t/pt-duplicate-key-checker/issue_331.t +++ b/t/pt-duplicate-key-checker/issue_331.t @@ -41,10 +41,10 @@ $sb->load_file('master', 't/pt-duplicate-key-checker/samples/issue_331.sql', 'te ok( no_diff( sub { pt_duplicate_key_checker::main(@args, qw(-d issue_331)) }, - 't/pt-duplicate-key-checker/samples/issue_331.txt', + 't/pt-duplicate-key-checker/samples/issue_331.txt' ), 'Issue 331 crash on fks' -); +) or diag($test_diff); # ############################################################################# # Done. diff --git a/t/pt-duplicate-key-checker/samples/issue_331.txt b/t/pt-duplicate-key-checker/samples/issue_331.txt index 58cbc881..3ab09ac8 100644 --- a/t/pt-duplicate-key-checker/samples/issue_331.txt +++ b/t/pt-duplicate-key-checker/samples/issue_331.txt @@ -2,14 +2,14 @@ # issue_331.issue_331_t2 # ######################################################################## -# FOREIGN KEY fk_1 (`id`) REFERENCES `issue_331`.`issue_331_t1` (`t1_id`) is a duplicate of FOREIGN KEY fk_2 (`id`) REFERENCES `issue_331`.`issue_331_t1` (`t1_id`) +# FOREIGN KEY fk_2 (`id`) REFERENCES `issue_331`.`issue_331_t1` (`t1_id`) is a duplicate of FOREIGN KEY fk_1 (`id`) REFERENCES `issue_331`.`issue_331_t1` (`t1_id`) # Key definitions: -# CONSTRAINT `fk_1` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`) # CONSTRAINT `fk_2` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`) +# CONSTRAINT `fk_1` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`) # Column types: # `id` bigint(20) not null default '0' # To remove this duplicate foreign key, execute: -ALTER TABLE `issue_331`.`issue_331_t2` DROP FOREIGN KEY `fk_1`; +ALTER TABLE `issue_331`.`issue_331_t2` DROP FOREIGN KEY `fk_2`; # MySQL uses the PRIMARY index for this foreign key constraint diff --git a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt index 4d5d0c4d..e58b04f2 100644 --- a/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt +++ b/t/pt-duplicate-key-checker/samples/simple_dupe_bug_1217013.txt @@ -6,15 +6,15 @@ # domain (`domain`) # unique_key_domain (`domain`) -# Uniqueness of domain ignored because unique_key_domain is a duplicate constraint -# domain is a duplicate of unique_key_domain +# Uniqueness of unique_key_domain ignored because domain is a duplicate constraint +# unique_key_domain is a duplicate of domain # Key definitions: -# UNIQUE KEY `domain` (`domain`), # UNIQUE KEY `unique_key_domain` (`domain`) +# UNIQUE KEY `domain` (`domain`), # Column types: # `domain` varchar(175) collate utf8_bin not null # To remove this duplicate index, execute: -ALTER TABLE `test`.`domains` DROP INDEX `domain`; +ALTER TABLE `test`.`domains` DROP INDEX `unique_key_domain`; # ######################################################################## # Summary of indexes