From 01daf0030e945a825729bbea065c08c8493383db Mon Sep 17 00:00:00 2001 From: Carlos Salguero Date: Wed, 22 Jun 2016 13:17:57 -0300 Subject: [PATCH] bug-1593265 Fixed pt-archiver deletes wrong rows In the case we are trying to migrate a table with no PK nor an unique key, pt-archiver was failing to select the correct rows for deletion. This fix implemented here is to add ALL columns in the WHERE clause of the DELETE command. This way, we are deleting only the exact same row we just migrated but using columns instead of an index. --- bin/pt-archiver | 2 +- bin/pt-online-schema-change | 2 +- bin/pt-table-checksum | 2 +- bin/pt-table-sync | 2 +- lib/TableNibbler.pm | 2 +- t/pt-archiver/issue_1166.t | 2 +- t/pt-archiver/issue_1593265.t | 64 +++++++++++++++++++++++++ t/pt-archiver/samples/issue_1593265.sql | 13 +++++ 8 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 t/pt-archiver/issue_1593265.t create mode 100644 t/pt-archiver/samples/issue_1593265.sql diff --git a/bin/pt-archiver b/bin/pt-archiver index a061f0fa..13e9b415 100755 --- a/bin/pt-archiver +++ b/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 { diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index 7d96752b..20ad24e3 100755 --- a/bin/pt-online-schema-change +++ b/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 { diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index 6be21987..cb5efd80 100755 --- a/bin/pt-table-checksum +++ b/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 { diff --git a/bin/pt-table-sync b/bin/pt-table-sync index 0db00dbd..d4819928 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -6480,7 +6480,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 { diff --git a/lib/TableNibbler.pm b/lib/TableNibbler.pm index 0eed5177..945da85f 100644 --- a/lib/TableNibbler.pm +++ b/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 { diff --git a/t/pt-archiver/issue_1166.t b/t/pt-archiver/issue_1166.t index 749c90b6..fb511d3b 100644 --- a/t/pt-archiver/issue_1166.t +++ b/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)" ); diff --git a/t/pt-archiver/issue_1593265.t b/t/pt-archiver/issue_1593265.t new file mode 100644 index 00000000..2ba5a340 --- /dev/null +++ b/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; diff --git a/t/pt-archiver/samples/issue_1593265.sql b/t/pt-archiver/samples/issue_1593265.sql new file mode 100644 index 00000000..ac329ead --- /dev/null +++ b/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);