diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 95cf9319..ebbdc282 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -2339,8 +2339,9 @@ sub get_duplicate_keys { my @dupes; KEY: - # sort by key name for consistent testing - foreach my $key ( sort {$a->{name} cmp $b->{name}} values %keys ) { + foreach my $keyname ( reverse sort keys %keys ) { + my $key = $keys{$keyname}; + $key->{real_cols} = [ @{$key->{cols}} ]; $key->{len_cols} = length $key->{colnames}; @@ -2416,8 +2417,10 @@ sub get_duplicate_keys { sub get_duplicate_fks { my ( $self, $fks, %args ) = @_; die "I need a fks argument" unless $fks; - # sort by name for consistent testing - my @fks = sort {$a->{name} cmp $b->{name}} values %$fks; + my @fks = (); + foreach my $key ( sort keys %$fks ) { + push @fks, $fks->{$key}; + } my @dupes; foreach my $i ( 0..$#fks - 1 ) { diff --git a/lib/DuplicateKeyFinder.pm b/lib/DuplicateKeyFinder.pm index ca99ca36..15ee4dea 100644 --- a/lib/DuplicateKeyFinder.pm +++ b/lib/DuplicateKeyFinder.pm @@ -64,7 +64,9 @@ sub get_duplicate_keys { my @dupes; KEY: - foreach my $key ( values %keys ) { + foreach my $keyname ( reverse sort keys %keys ) { + my $key = $keys{$keyname}; + # Save real columns before we potentially re-order them. These are # columns we want to print if the key is a duplicate. $key->{real_cols} = [ @{$key->{cols}} ]; @@ -169,7 +171,10 @@ sub get_duplicate_keys { sub get_duplicate_fks { my ( $self, $fks, %args ) = @_; die "I need a fks argument" unless $fks; - my @fks = values %$fks; + my @fks = (); + foreach my $key ( sort keys %$fks ) { + push @fks, $fks->{$key}; + } my @dupes; foreach my $i ( 0..$#fks - 1 ) { diff --git a/t/lib/DuplicateKeyFinder.t b/t/lib/DuplicateKeyFinder.t index a699fdf9..7ad829d3 100644 --- a/t/lib/DuplicateKeyFinder.t +++ b/t/lib/DuplicateKeyFinder.t @@ -18,6 +18,9 @@ use Quoter; use TableParser; use PerconaTest; +$Data::Dumper::Purity = 1; +$Data::Dumper::Terse = 1; + my $dk = new DuplicateKeyFinder(); my $q = new Quoter(); my $tp = new TableParser(Quoter => $q); @@ -216,19 +219,25 @@ $dk->get_duplicate_keys( is_deeply( $dupes, [ - { - 'key' => 'ft_idx_a_b', - 'cols' => [qw(a b)], - ddl => 'FULLTEXT KEY `ft_idx_a_b` (`a`,`b`),', - 'duplicate_of' => 'ft_idx_b_a', - 'duplicate_of_cols' => [qw(b a)], - duplicate_of_ddl => 'FULLTEXT KEY `ft_idx_b_a` (`b`,`a`)', - 'reason' => 'ft_idx_a_b is a duplicate of ft_idx_b_a', - dupe_type => 'exact', - } + { + cols => [ + 'b', + 'a' + ], + ddl => 'FULLTEXT KEY `ft_idx_b_a` (`b`,`a`)', + dupe_type => 'exact', + duplicate_of => 'ft_idx_a_b', + duplicate_of_cols => [ + 'a', + 'b' + ], + duplicate_of_ddl => 'FULLTEXT KEY `ft_idx_a_b` (`a`,`b`),', + key => 'ft_idx_b_a', + reason => 'ft_idx_b_a is a duplicate of ft_idx_a_b' + } ], 'Dupe reverse order fulltext keys (issue 10)' -); +) or diag(Dumper($dupes)); $ddl = load_file('t/lib/samples/dupe_key_unordered.sql'); $dupes = []; @@ -252,19 +261,25 @@ $dk->get_duplicate_keys( is_deeply( $dupes, [ - { - 'key' => 'a', - 'cols' => [qw(b a)], - ddl => 'KEY `a` (`b`,`a`),', - 'duplicate_of' => 'a_2', - 'duplicate_of_cols' => [qw(a b)], - duplicate_of_ddl => 'KEY `a_2` (`a`,`b`),', - 'reason' => 'a is a duplicate of a_2', - dupe_type => 'exact', - } - ], + { + cols => [ + 'a', + 'b' + ], + ddl => 'KEY `a_2` (`a`,`b`),', + dupe_type => 'exact', + duplicate_of => 'a', + duplicate_of_cols => [ + 'b', + 'a' + ], + duplicate_of_ddl => 'KEY `a` (`b`,`a`),', + key => 'a_2', + reason => 'a_2 is a duplicate of a' + } + ], 'Two dupe keys when ignoring order' -); +) or diag(Dumper($dupes)); # ############################################################################# # Clustered key tests. @@ -350,20 +365,26 @@ $dk->get_duplicate_fks( callback => $callback); is_deeply( $dupes, - [ + [ { - 'key' => 't1_ibfk_1', - 'cols' => [qw(a b)], - ddl => 'CONSTRAINT `t1_ibfk_1` FOREIGN KEY (`a`, `b`) REFERENCES `t2` (`a`, `b`)', - 'duplicate_of' => 't1_ibfk_2', - 'duplicate_of_cols' => [qw(b a)], - duplicate_of_ddl => 'CONSTRAINT `t1_ibfk_2` FOREIGN KEY (`b`, `a`) REFERENCES `t2` (`b`, `a`)', - 'reason' => 'FOREIGN KEY t1_ibfk_1 (`a`, `b`) REFERENCES `test`.`t2` (`a`, `b`) is a duplicate of FOREIGN KEY t1_ibfk_2 (`b`, `a`) REFERENCES `test`.`t2` (`b`, `a`)', - dupe_type => 'fk', + cols => [ + 'b', + 'a' + ], + ddl => 'CONSTRAINT `t1_ibfk_2` FOREIGN KEY (`b`, `a`) REFERENCES `t2` (`b`, `a`)', + dupe_type => 'fk', + duplicate_of => 't1_ibfk_1', + duplicate_of_cols => [ + 'a', + 'b' + ], + duplicate_of_ddl => 'CONSTRAINT `t1_ibfk_1` FOREIGN KEY (`a`, `b`) REFERENCES `t2` (`a`, `b`)', + key => 't1_ibfk_2', + reason => 'FOREIGN KEY t1_ibfk_2 (`b`, `a`) REFERENCES `test`.`t2` (`b`, `a`) is a duplicate of FOREIGN KEY t1_ibfk_1 (`a`, `b`) REFERENCES `test`.`t2` (`a`, `b`)' } - ], + ], 'Two duplicate foreign keys' -); +) or diag(Dumper($dupes)); $ddl = load_file('t/lib/samples/sakila_film.sql'); $dupes = []; @@ -546,28 +567,36 @@ is_deeply( ddl => 'KEY `a_b` (`a`,`b`),', }, { - 'duplicate_of' => 'PRIMARY', - 'reason' => "Uniqueness of ua_b ignored because ua is a stronger constraint\nua_b is a duplicate of PRIMARY", - dupe_type => 'exact', - 'duplicate_of_cols' => [qw(a b)], - duplicate_of_ddl => 'PRIMARY KEY (`a`,`b`),', - 'cols' => [qw(a b)], - 'key' => 'ua_b', - ddl => 'UNIQUE KEY `ua_b` (`a`,`b`),', + cols => [ + 'a', + 'b' + ], + ddl => 'UNIQUE KEY `ua_b2` (`a`,`b`),', + dupe_type => 'exact', + duplicate_of => 'PRIMARY', + duplicate_of_cols => [qw(a b)], + duplicate_of_ddl => 'PRIMARY KEY (`a`,`b`),', + key => 'ua_b2', + reason => 'Uniqueness of ua_b2 ignored because ua is a stronger constraint +ua_b2 is a duplicate of PRIMARY' }, { - 'duplicate_of' => 'PRIMARY', - 'reason' => "Uniqueness of ua_b2 ignored because ua is a stronger constraint\nua_b2 is a duplicate of PRIMARY", - dupe_type => 'exact', - 'duplicate_of_cols' => [qw(a b)], - duplicate_of_ddl => 'PRIMARY KEY (`a`,`b`),', - 'cols' => [qw(a b)], - 'key' => 'ua_b2', - ddl => 'UNIQUE KEY `ua_b2` (`a`,`b`),', + cols => [ + 'a', + 'b' + ], + ddl => 'UNIQUE KEY `ua_b` (`a`,`b`),', + dupe_type => 'exact', + duplicate_of => 'PRIMARY', + duplicate_of_cols => [qw(a b)], + duplicate_of_ddl => 'PRIMARY KEY (`a`,`b`),', + key => 'ua_b', + reason => 'Uniqueness of ua_b ignored because ua is a stronger constraint +ua_b is a duplicate of PRIMARY' } ], 'Very pathological case', -); +) or diag(Dumper($dupes)); # ############################################################################# # Issue 269: mk-duplicate-key-checker: Wrongly suggesting removing index @@ -596,20 +625,24 @@ $dk->get_duplicate_fks( callback => $callback); is_deeply( $dupes, - [ + [ { - 'key' => 'fk_1', - 'cols' => [qw(id)], - ddl => 'CONSTRAINT `fk_1` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`)', - 'duplicate_of' => 'fk_2', - 'duplicate_of_cols' => [qw(id)], - duplicate_of_ddl => 'CONSTRAINT `fk_2` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`)', - 'reason' => 'FOREIGN KEY fk_1 (`id`) REFERENCES `test`.`issue_331_t1` (`t1_id`) is a duplicate of FOREIGN KEY fk_2 (`id`) REFERENCES `test`.`issue_331_t1` (`t1_id`)', - dupe_type => 'fk', + cols => [ + 'id' + ], + ddl => 'CONSTRAINT `fk_2` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`)', + dupe_type => 'fk', + duplicate_of => 'fk_1', + duplicate_of_cols => [ + 'id' + ], + duplicate_of_ddl => 'CONSTRAINT `fk_1` FOREIGN KEY (`id`) REFERENCES `issue_331_t1` (`t1_id`)', + key => 'fk_2', + reason => 'FOREIGN KEY fk_2 (`id`) REFERENCES `test`.`issue_331_t1` (`t1_id`) is a duplicate of FOREIGN KEY fk_1 (`id`) REFERENCES `test`.`issue_331_t1` (`t1_id`)' } - ], + ], 'fk col not in referencing table (issue 331)' -); +) or diag(Dumper($dupes)); # ############################################################################# # Issue 295: Enhance rules for clustered keys in mk-duplicate-key-checker diff --git a/t/pt-duplicate-key-checker/basics.t b/t/pt-duplicate-key-checker/basics.t index 45a014a8..325f5c5b 100644 --- a/t/pt-duplicate-key-checker/basics.t +++ b/t/pt-duplicate-key-checker/basics.t @@ -143,7 +143,7 @@ $sb->load_file('master', 't/lib/samples/dupekeys/simple_dupe_bug_1217013.sql', ' ok( no_diff( sub { pt_duplicate_key_checker::main(@args, qw(-t test.domains -v)) }, - "$sample/simple_dupe_bug_1217013.txt", keep_output=>1), + "$sample/simple_dupe_bug_1217013.txt"), 'Exact unique dupes (bug 1217013)' ) or diag($test_diff); @@ -159,7 +159,6 @@ ok( 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 e58b04f2..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 @@ -6,15 +6,15 @@ # domain (`domain`) # unique_key_domain (`domain`) -# Uniqueness of unique_key_domain ignored because domain is a duplicate constraint -# unique_key_domain is a duplicate of domain +# Uniqueness of domain ignored because unique_key_domain is a duplicate constraint +# domain is a duplicate of unique_key_domain # Key definitions: -# UNIQUE KEY `unique_key_domain` (`domain`) # UNIQUE KEY `domain` (`domain`), +# UNIQUE KEY `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 `unique_key_domain`; +ALTER TABLE `test`.`domains` DROP INDEX `domain`; # ######################################################################## # Summary of indexes