Merged fix-1016131-ptc-crash-with-columns

bin/pt-table-checksum

@@ -5146,6 +5146,9 @@ sub make_row_checksum {
    my $func       = $args{func} || uc($o->get('function'));
    my $cols       = $self->get_checksum_columns(%args);
 
+   die "all columns are excluded by --columns or --ignore-columns"
+      unless @{$cols->{select}};
+      
    my $query;
    if ( !$args{no_cols} ) {
       $query = join(', ',
@@ -9340,11 +9343,21 @@ sub main {
          # Make a nibble iterator for this table.  This should only fail
          # if the table has no indexes and is too large to checksum in
          # one chunk.
-         my $checksum_cols = $rc->make_chunk_checksum(
+         local $EVAL_ERROR;
+         my $checksum_cols = eval {
+            $rc->make_chunk_checksum(
                dbh => $master_cxn->dbh(),
                tbl => $tbl,
                %crc_args
             );
+         };
+
+         if ( $EVAL_ERROR ) {
+            warn ts("Skipping table $tbl->{db}.$tbl->{tbl} because "
+                  . "$EVAL_ERROR\n");
+            return;
+         }
+
          my $nibble_iter;
          eval {
             $nibble_iter = new OobNibbleIterator(
@@ -10664,7 +10677,8 @@ of leaving it at the default.
 
 short form: -c; type: array; group: Filter
 
-Checksum only this comma-separated list of columns.
+Checksum only this comma-separated list of columns. If a table doesn't have
+any of the specified columns it will be skipped.
 
 =item --config
 
@@ -10780,6 +10794,8 @@ Host to connect to.
 type: Hash; group: Filter
 
 Ignore this comma-separated list of columns when calculating the checksum.
+If a table has all of its columns filtered by --ignore-columns, it will
+be skipped.
 
 =item --ignore-databases
 

lib/RowChecksum.pm

@@ -67,6 +67,11 @@ sub make_row_checksum {
    my $func       = $args{func} || uc($o->get('function'));
    my $cols       = $self->get_checksum_columns(%args);
 
+   # Skip tables that have all their columns skipped; See
+   # https://bugs.launchpad.net/percona-toolkit/+bug/1016131
+   die "all columns are excluded by --columns or --ignore-columns"
+      unless @{$cols->{select}};
+      
    # Prepend columns to query, resulting in "col1, col2, FUNC(..col1, col2...)",
    # unless caller says not to.  The only caller that says not to is
    # make_chunk_checksum() which uses this row checksum as part of a larger

t/lib/RowChecksum.t

@@ -26,9 +26,6 @@ my $dbh = $sb->get_dbh_for('master');
 if ( !$dbh ) {
    plan skip_all => "Cannot connect to sandbox master";
 }
-else {
-   plan tests => 29;
-}
 
 $sb->create_dbs($dbh, ['test']);
 
@@ -421,9 +418,53 @@ is(
    'Ignores specified columns'
 );
 
+# #############################################################################
+# crash with --columns if none match / --ignore-columns if everything is ignored
+# https://bugs.launchpad.net/percona-toolkit/+bug/1016131
+# #############################################################################
+# Re-using the $tbl from the previous test!
+local @ARGV = ('--ignore-columns', 'a,b,c');
+$o->get_opts();
+local $EVAL_ERROR;
+eval {
+   $c->make_row_checksum(
+      tbl  => $tbl,
+      func => 'CRC32',
+   );
+};
+
+like(
+   $EVAL_ERROR,
+   qr/all columns are excluded by --columns or --ignore-columns/,
+   "Dies if all columns are ignored by --ignore-columns"
+);
+
+
+$tbl = {
+   db         => 'mysql',
+   tbl        => 'user',
+   tbl_struct => $tp->parse($tp->get_create_table($dbh, 'mysql', 'user')),
+};
+local @ARGV = qw(--columns some_column_that_doesnt_exist);
+$o->get_opts();
+local $EVAL_ERROR;
+eval {
+   $c->make_row_checksum(
+      tbl  => $tbl,
+      func => 'SHA1',
+   );
+};
+
+like(
+   $EVAL_ERROR,
+   qr/all columns are excluded by --columns or --ignore-columns/,
+   'Dies if all columns are ignored by --columns'
+);
+
 # ############################################################################
 # Done.
 # ############################################################################
 $sb->wipe_clean($dbh);
 ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
-exit;
+
+done_testing;

t/pt-table-checksum/bugs.t

@@ -175,6 +175,22 @@ is(
    "Bug 1030031 (wrong DIFFS): 3 diffs"
 );
 
+# #############################################################################
+# pt-table-checksum can crash with --columns if none match
+# https://bugs.launchpad.net/percona-toolkit/+bug/1016131
+# #############################################################################
+
+($output) = full_output(
+   sub { pt_table_checksum::main(@args, '--tables', 'mysql.user,mysql.host',
+                                 '--columns', 'some_fale_column') },
+);
+
+like(
+   $output,
+   qr/\QSkipping table mysql.user because all columns are excluded by --columns or --ignore-columns/,
+   "Bug 1016131: ptc should skip tables where all columns are excluded"
+);
+
 # #############################################################################
 # Done.
 # #############################################################################