Merge fix-pt-dupe-key-bug-1036804

bin/pt-duplicate-key-checker

@@ -2188,8 +2188,9 @@ sub get_duplicate_keys {
 
    my $clustered_key = $args{clustered_key} ? $keys{$args{clustered_key}}
                      : undef;
-   PTDEBUG && _d('clustered key:', $clustered_key->{name},
+   PTDEBUG && _d('clustered key:',
-      $clustered_key->{colnames});
+      $clustered_key ? ($clustered_key->{name}, $clustered_key->{colnames})
+                     : 'none');
    if ( $clustered_key
         && $args{clustered}
         && $args{tbl_info}->{engine}
@@ -3404,74 +3405,74 @@ sub main {
    );
    TABLE:
    while ( my $tbl = $schema_itr->next() ) {
-      $tbl->{engine} = $tbl->{tbl_struct}->{engine};
+      eval {
+         $tbl->{engine} = $tbl->{tbl_struct}->{engine};
 
-      my ($keys, $clustered_key, $fks);
+         my ($keys, $clustered_key, $fks);
-      if ( $get_keys ) {
+         if ( $get_keys ) {
-         ($keys, $clustered_key)
+            ($keys, $clustered_key)
-            = $tp->get_keys($tbl->{ddl}, {});
+               = $tp->get_keys($tbl->{ddl}, {});
-      }
+         }
-      if ( $get_fks ) {
+         if ( $get_fks ) {
-         $fks = $tp->get_fks($tbl->{ddl},  {database => $tbl->{db}});
+            $fks = $tp->get_fks($tbl->{ddl},  {database => $tbl->{db}});
-      }
-
-      next TABLE unless ($keys && %$keys) || ($fks && %$fks);
-
-      if ( $o->got('verbose') ) {
-         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});
-         eval {
-            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,
-               );
-            }
-         };
-         if ( $EVAL_ERROR ) {
-            warn "Error checking `$tbl->{db}`.`$tbl->{tbl}` for duplicate keys: "
-               . $EVAL_ERROR;
-            next TABLE;
          }
-      }
 
-      # Always count Total Keys so print_key_summary won't die
+         if ( ($keys && %$keys) || ($fks && %$fks) ) {
-      # because %summary is empty.
+            if ( $o->got('verbose') ) {
-      $summary{'Total Indexes'} += (scalar keys %$keys) + (scalar keys %$fks)
+               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,
+                  );
+               }
+            }
+
+            # Always count Total Keys so print_key_summary won't die
+            # because %summary is empty.
+            $summary{'Total Indexes'} += (scalar keys %$keys)
+                                       + (scalar keys %$fks)
+         }
+      };
+      if ( $EVAL_ERROR ) {
+         warn "Error checking $tbl->{db}.$tbl->{tbl}: $EVAL_ERROR";
+      }
    }  # TABLE
 
    print_key_summary(%summary) if $o->get('summary');

lib/DuplicateKeyFinder.pm

@@ -148,8 +148,9 @@ sub get_duplicate_keys {
    # Remove clustered duplicates.
    my $clustered_key = $args{clustered_key} ? $keys{$args{clustered_key}}
                      : undef;
-   PTDEBUG && _d('clustered key:', $clustered_key->{name},
+   PTDEBUG && _d('clustered key:',
-      $clustered_key->{colnames});
+      $clustered_key ? ($clustered_key->{name}, $clustered_key->{colnames})
+                     : 'none');
    if ( $clustered_key
         && $args{clustered}
         && $args{tbl_info}->{engine}

t/lib/DuplicateKeyFinder.t

@@ -9,7 +9,7 @@ BEGIN {
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 38;
+use Test::More;
 
 use VersionParser;
 use DuplicateKeyFinder;
@@ -786,4 +786,5 @@ like(
    qr/Complete test coverage/,
    '_d() works'
 );
+done_testing;
 exit;

t/pt-duplicate-key-checker/clustered_keys.t

@@ -22,9 +22,6 @@ my $dbh = $sb->get_dbh_for('master');
 if ( !$dbh ) {
    plan skip_all => 'Cannot connect to sandbox master';
 }
-else {
-   plan tests => 2;
-}
 
 my $cnf    = "/tmp/12345/my.sandbox.cnf";
 my $sample = "t/pt-duplicate-key-checker/samples/";
@@ -46,9 +43,47 @@ ok(
    "Shorten, not remove, clustered dupes"
 );
 
+# #############################################################################
+# Error if InnoDB table has no PK or unique indexes
+# https://bugs.launchpad.net/percona-toolkit/+bug/1036804
+# #############################################################################
+$sb->load_file('master', "t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql");
+
+# PTDEBUG was auto-vivifying $clustered_key:
+#
+#    PTDEBUG && _d('clustered key:', $clustered_key->{name},
+#       $clustered_key->{colnames});
+#
+#    if ( $clustered_key
+#         && $args{clustered}
+#         && $args{tbl_info}->{engine}
+#         && $args{tbl_info}->{engine} =~ m/InnoDB/i )
+#    {
+#          push @dupes, $self->remove_clustered_duplicates($clustered_key...
+#
+#    sub remove_clustered_duplicates {
+#       my ( $self, $ck, $keys, %args ) = @_;
+#       die "I need a ck argument"   unless $ck;
+#       die "I need a keys argument" unless $keys;
+#       my $ck_cols = $ck->{colnames};
+#       my @dupes;
+#       KEY:
+#       for my $i ( 0 .. @$keys - 1 ) {
+#          my $key = $keys->[$i]->{colnames};
+#          if ( $key =~ m/$ck_cols$/ ) {
+
+my $output = `PTDEBUG=1 $trunk/bin/pt-duplicate-key-checker F=$cnf -d bug_1036804 2>&1`;
+
+unlike(
+   $output,
+   qr/Use of uninitialized value/,
+   'PTDEBUG doesn\'t auto-vivify cluster key hashref (bug 1036804)'
+);
+
 # #############################################################################
 # Done.
 # #############################################################################
 $sb->wipe_clean($dbh);
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+done_testing;
 exit;

t/pt-duplicate-key-checker/samples/idb-no-uniques-bug-894140.sql

@@ -0,0 +1,9 @@
+DROP DATABASE IF EXISTS bug_1036804;
+CREATE DATABASE bug_1036804;
+USE bug_1036804;
+CREATE TABLE `t` (
+  `col1` int(11) DEFAULT NULL,
+  `col2` int(11) DEFAULT NULL,
+  KEY `col1` (`col1`),
+  KEY `col2` (`col2`)
+) ENGINE=InnoDB DEFAULT CHARSET=latin1;