Merge pull request #103 from percona/bug-1593265

Bug-1593265 Fixed pt-archiver deletes wrong rows

bin/pt-archiver

@@ -3201,7 +3201,7 @@ sub generate_del_stmt {
    my $index = $tp->find_best_index($tbl, $args{index});
    die "Cannot find an ascendable index in table" unless $index;
 
-   if ( $index ) {
+   if ( $index && $tbl->{keys}->{$index}->{is_unique}) {
       @del_cols = @{$tbl->{keys}->{$index}->{cols}};
    }
    else {

bin/pt-online-schema-change

@@ -3052,7 +3052,7 @@ sub generate_del_stmt {
    my $index = $tp->find_best_index($tbl, $args{index});
    die "Cannot find an ascendable index in table" unless $index;
 
-   if ( $index ) {
+   if ( $index && $tbl->{keys}->{$index}->{is_unique}) {
       @del_cols = @{$tbl->{keys}->{$index}->{cols}};
    }
    else {

bin/pt-table-checksum

@@ -4901,7 +4901,7 @@ sub generate_del_stmt {
    my $index = $tp->find_best_index($tbl, $args{index});
    die "Cannot find an ascendable index in table" unless $index;
 
-   if ( $index ) {
+   if ( $index && $tbl->{keys}->{$index}->{is_unique}) {
       @del_cols = @{$tbl->{keys}->{$index}->{cols}};
    }
    else {

bin/pt-table-sync

@@ -6483,7 +6483,7 @@ sub generate_del_stmt {
    my $index = $tp->find_best_index($tbl, $args{index});
    die "Cannot find an ascendable index in table" unless $index;
 
-   if ( $index ) {
+   if ( $index && $tbl->{keys}->{$index}->{is_unique}) {
       @del_cols = @{$tbl->{keys}->{$index}->{cols}};
    }
    else {

lib/TableNibbler.pm

@@ -247,7 +247,7 @@ sub generate_del_stmt {
    die "Cannot find an ascendable index in table" unless $index;
 
    # These are the columns needed for the DELETE statement's WHERE clause.
-   if ( $index ) {
+   if ( $index && $tbl->{keys}->{$index}->{is_unique}) {
       @del_cols = @{$tbl->{keys}->{$index}->{cols}};
    }
    else {

t/lib/TableNibbler.t

@@ -134,11 +134,13 @@ is_deeply(
       cols   => [qw(film_id)],
    ),
    {
-      cols  => [qw(film_id title)],
+      cols  => [qw(film_id title description release_year language_id original_language_id rental_duration rental_rate length replacement_cost rating special_features last_update)],
       index => 'idx_title',
-      where => '(`title` = ?)',
-      slice => [1],
-      scols => [qw(title)],
+      where => '(`film_id` = ? AND `title` = ? AND ((? IS NULL AND `description` IS NULL) OR (`description` = ?)) AND ((? IS NULL AND `release_year` IS NULL) OR (`release_year` = ?)) AND `language_id` = ? AND ((? IS NULL AND `original_language_id` IS NULL) OR (`original_language_id` = ?)) AND `rental_duration` = ? AND `rental_rate` = ? AND ((? IS NULL AND `length` IS NULL) OR (`length` = ?)) AND `replacement_cost` = ? AND ((? IS NULL AND `rating` IS NULL) OR (`rating` = ?)) AND ((? IS NULL AND `special_features` IS NULL) OR (`special_features` = ?)) AND `last_update` = ?)',
+      slice => [ 0, 1, 2, 2, 3, 3, 4, 5, 5, 6, 7, 8, 8, 9, 10, 10, 11, 11, 12 ],
+      scols  => [qw( film_id title description description release_year release_year language_id original_language_id original_language_id
+                     rental_duration rental_rate length length replacement_cost rating rating special_features special_features last_update)],
+
    },
    'del stmt on sakila.film with different index and extra column',
 );

t/pt-archiver/issue_1166.t

@@ -56,7 +56,7 @@ $output = output(
 );
 like(
    $output,
-   qr/DELETE FROM `test`\.`issue_1166` WHERE \(`id` = \?\) LIMIT 1$/m,
+   qr/LIMIT 1$/m,
    "LIMIT 1 with non-unique index (issue 1166)"
 );
 

t/pt-archiver/issue_1593265.t

@@ -0,0 +1,64 @@
+#!/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 PerconaTest;
+use Sandbox;
+require "$trunk/bin/pt-archiver";
+
+my $dp  = new DSNParser(opts=>$dsn_opts);
+my $sb  = new Sandbox(basedir => '/tmp', DSNParser => $dp);
+my $dbh = $sb->get_dbh_for('master');
+
+if ( !$dbh ) {
+   plan skip_all => 'Cannot connect to sandbox master';
+}
+else {
+   plan tests => 3;
+}
+
+my $output;
+
+# #############################################################################
+# Issue 1152: mk-archiver columns option resulting in null archived table data
+# #############################################################################
+$sb->load_file('master', 't/pt-archiver/samples/issue_1593265.sql');
+
+$dbh->do('set names "utf8"');
+
+$output = output(
+   sub { pt_archiver::main(
+      '--source',  'h=127.1,P=12345,D=test,t=t1,u=msandbox,p=msandbox',
+      '--dest', 't=t2', '--where', 'b in (1,2,3)')
+   },
+);
+
+my $untouched_rows = $dbh->selectall_arrayref('SELECT a, b FROM test.t1');
+is_deeply(
+   $untouched_rows,
+   [ ['10', '5'], ['10', '4'] ],
+   "Rows were left on the original table"
+);
+
+my $new_rows = $dbh->selectall_arrayref('SELECT a, b FROM test.t2');
+is_deeply(
+   $new_rows,
+   [ ['10', '3'], ['10', '2'], ['10', '1'] ],
+   "Rows were archived into the new table"
+);
+
+# #############################################################################
+# Done.
+# #############################################################################
+$sb->wipe_clean($dbh);
+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+exit;

t/pt-archiver/samples/issue_1593265.sql

@@ -0,0 +1,13 @@
+CREATE DATABASE IF NOT EXISTS test;
+USE test;
+DROP TABLE IF EXISTS t1;
+DROP TABLE IF EXISTS t2;
+
+create table t1 (a int, b int, key (a),key(b));
+create table t2 like t1;
+
+insert into t1 (a,b) values (10,5);
+insert into t1 (a,b) values (10,4);
+insert into t1 (a,b) values (10,3);
+insert into t1 (a,b) values (10,2);
+insert into t1 (a,b) values (10,1);