From 4230f411e4c3c1795fdbd944c0d4d9e07ed52f2c Mon Sep 17 00:00:00 2001
From: Carlos Salguero <carlos.salguero@percona.com>
Date: Tue, 27 Nov 2018 12:25:47 -0300
Subject: [PATCH] PT-157 Specifying the index to use for pt-archiver ignores
 --primary-key-only

---
 bin/pt-archiver                  |  2 +-
 t/pt-archiver/delete_more.t      | 45 ++++++++++++++++++++------------
 t/pt-archiver/purge.t            | 16 +++++++++++-
 t/pt-archiver/samples/pt_157.sql | 20 ++++++++++++++
 4 files changed, 65 insertions(+), 18 deletions(-)
 create mode 100644 t/pt-archiver/samples/pt_157.sql

diff --git a/bin/pt-archiver b/bin/pt-archiver
index 43d7265d..34ba4e26 100755
--- a/bin/pt-archiver
+++ b/bin/pt-archiver
@@ -6347,7 +6347,7 @@ sub main {
    $del_stmt = $nibbler->generate_del_stmt(
       tbl_struct => $src->{info},
       cols       => \@sel_cols,
-      index      => $src->{i},
+      index      => $o->get('primary-key-only') ? 'PRIMARY' : $src->{i},
    );
    @del_slice = @{$del_stmt->{slice}};
 
diff --git a/t/pt-archiver/delete_more.t b/t/pt-archiver/delete_more.t
index c632d61f..513de2b8 100644
--- a/t/pt-archiver/delete_more.t
+++ b/t/pt-archiver/delete_more.t
@@ -38,6 +38,7 @@ my $cmd = "perl -I $trunk/t/pt-archiver/samples $trunk/bin/pt-archiver";
 $sb->load_file('master', "t/pt-archiver/samples/delete_more.sql");
 $dbh->do('use dm');
 
+#1
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -49,7 +50,7 @@ is_deeply(
    ],
    'main_table-123 data before archiving'
 );
-
+#2
 is_deeply(
    $dbh->selectall_arrayref('select * from `other_table-123` order by id'),
    [
@@ -67,6 +68,10 @@ is_deeply(
 
 `$cmd --purge --primary-key-only --source F=$cnf,D=dm,t=main_table-123,i=pub_date,b=1,m=delete_more --where "pub_date < '2010-02-16'" --bulk-delete --limit 2`;
 
+
+# PT-157 has changed the program behavior when using --primary-key-only even if an index was
+# specified in the DSN
+#3
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -74,17 +79,19 @@ is_deeply(
       # [2, '2010-02-15', 'b'],
       # [3, '2010-02-15', 'c'],
       [4, '2010-02-16', 'd'],
-      # [5, '2010-02-14', 'e'],
+      [5, '2010-02-14', 'e'],
    ],
    'main_table-123 data after archiving (limit 2)'
 );
 
+#4
 is_deeply(
    $dbh->selectall_arrayref('select * from `other_table-123` order by id'),
    [
       [1, 'a'],
       [4, 'd'],
-      [6, 'ot1'],
+      [5, 'e'],
+      [6, 'ot1']
    ],
    'other_table-123 data after archiving (limit 2)'
 );
@@ -92,6 +99,7 @@ is_deeply(
 SKIP: {
    skip 'Cannot connect to slave sandbox', 6 unless $slave_dbh;
    $slave_dbh->do('use dm');
+#5
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `main_table-123` order by id'),
       [
@@ -103,7 +111,7 @@ SKIP: {
       ],
       'Slave main_table-123 not changed'
    );
-
+#6
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `other_table-123` order by id'),
       [
@@ -122,6 +130,7 @@ SKIP: {
    # Run it again without DSN b so changes should be made on slave.
    $sb->load_file('master', "t/pt-archiver/samples/delete_more.sql");
 
+#7
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `main_table-123` order by id'),
       [
@@ -133,7 +142,7 @@ SKIP: {
       ],
       'Reset slave main_table-123'
    );
-
+#8
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `other_table-123` order by id'),
       [
@@ -151,7 +160,7 @@ SKIP: {
 
    `$cmd --purge --primary-key-only --source F=$cnf,D=dm,t=main_table-123,i=pub_date,m=delete_more --where "pub_date < '2010-02-16'" --bulk-delete --limit 2`;
    sleep 1;
-
+#9
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `main_table-123` order by id'),
       [
@@ -159,16 +168,18 @@ SKIP: {
          # [2, '2010-02-15', 'b'],
          # [3, '2010-02-15', 'c'],
          [4, '2010-02-16', 'd'],
-         # [5, '2010-02-14', 'e'],
+         [5, '2010-02-14', 'e'],
       ],
       'Slave main_table-123 changed'
    );
 
+#10
    is_deeply(
       $slave_dbh->selectall_arrayref('select * from `other_table-123` order by id'),
       [
          [1, 'a'],
          [4, 'd'],
+         [5, 'e'],
          [6, 'ot1'],
       ],
       'Slave other_table-123 changed'
@@ -180,7 +191,7 @@ SKIP: {
 # ###########################################################################
 $sb->load_file('master', "t/pt-archiver/samples/delete_more.sql");
 $dbh->do('use dm');
-
+#11
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -192,7 +203,7 @@ is_deeply(
    ],
    'main_table-123 data before archiving'
 );
-
+#12
 is_deeply(
    $dbh->selectall_arrayref('select * from `other_table-123` order by id'),
    [
@@ -209,7 +220,7 @@ is_deeply(
 );
 
 `$cmd --purge --primary-key-only --source F=$cnf,D=dm,t=main_table-123,i=pub_date,b=1,m=delete_more --where "pub_date < '2010-02-16'" --bulk-delete --limit 100`;
-
+#13
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -217,16 +228,17 @@ is_deeply(
       # [2, '2010-02-15', 'b'],
       # [3, '2010-02-15', 'c'],
       [4, '2010-02-16', 'd'],
-      # [5, '2010-02-14', 'e'],
+      [5, '2010-02-14', 'e'],
    ],
    'main_table-123 data after archiving (limit 100)'
 );
-
+#14
 is_deeply(
    $dbh->selectall_arrayref('select * from `other_table-123` order by id'),
    [
       [1, 'a'],
       [4, 'd'],
+      [5, 'e'],
       [6, 'ot1'],
    ],
    'other_table-123 data after archiving (limit 100)'
@@ -237,7 +249,7 @@ is_deeply(
 # ###########################################################################
 $sb->load_file('master', "t/pt-archiver/samples/delete_more.sql");
 $dbh->do('use dm');
-
+#15
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -249,7 +261,7 @@ is_deeply(
    ],
    'main_table-123 data before archiving'
 );
-
+#16
 is_deeply(
    $dbh->selectall_arrayref('select * from `other_table-123` order by id'),
    [
@@ -267,7 +279,7 @@ is_deeply(
 
 `$cmd --purge --primary-key-only --source F=$cnf,D=dm,t=main_table-123,i=pub_date,b=1,m=delete_more --where "pub_date < '2010-02-16'"`;
 `$cmd --purge --primary-key-only --source F=$cnf,D=dm,t=main_table-123,i=pub_date,b=1,m=delete_more --where "pub_date < '2010-02-16'"`;
-
+#17
 is_deeply(
    $dbh->selectall_arrayref('select * from `main_table-123` order by id'),
    [
@@ -275,7 +287,7 @@ is_deeply(
       # [2, '2010-02-15', 'b'],
       # [3, '2010-02-15', 'c'],
       [4, '2010-02-16', 'd'],
-      # [5, '2010-02-14', 'e'],
+      [5, '2010-02-14', 'e'],
    ],
    'main_table-123 data after archiving (single delete)'
 );
@@ -285,6 +297,7 @@ is_deeply(
    [
       [1, 'a'],
       [4, 'd'],
+      [5, 'e'],
       [6, 'ot1'],
    ],
    'other_table-123 data after archiving (single delete)'
diff --git a/t/pt-archiver/purge.t b/t/pt-archiver/purge.t
index b1de5170..9b02aec4 100644
--- a/t/pt-archiver/purge.t
+++ b/t/pt-archiver/purge.t
@@ -23,7 +23,7 @@ if ( !$dbh ) {
    plan skip_all => 'Cannot connect to sandbox master';
 }
 else {
-   plan tests => 7;
+   plan tests => 8;
 }
 
 my $output;
@@ -38,8 +38,10 @@ $sb->load_file('master', 't/pt-archiver/samples/table1.sql');
 $output = output(
    sub { pt_archiver::main(qw(--where 1=1), "--source", "D=test,t=table_1,F=$cnf", qw(--purge)) },
 );
+#1
 is($output, '', 'Basic test run did not die');
 $output = `/tmp/12345/use -N -e "select count(*) from test.table_1"`;
+#2
 is($output + 0, 0, 'Purged ok');
 
 # Test basic functionality with --commit-each
@@ -47,8 +49,10 @@ $sb->load_file('master', 't/pt-archiver/samples/table1.sql');
 $output = output(
    sub { pt_archiver::main(qw(--where 1=1), "--source", "D=test,t=table_1,F=$cnf", qw(--commit-each --limit 1 --purge)) },
 );
+#3
 is($output, '', 'Commit-each did not die');
 $output = `/tmp/12345/use -N -e "select count(*) from test.table_1"`;
+#4
 is($output + 0, 0, 'Purged ok with --commit-each');
 
 # Archive only part of the table
@@ -56,10 +60,20 @@ $sb->load_file('master', 't/pt-archiver/samples/table1.sql');
 $output = output(
    sub { pt_archiver::main(qw(--where 1=1), "--source", "D=test,t=table_1,F=$cnf", qw(--where a<4 --purge)) },
 );
+#5
 is($output, '', 'No output for archiving only part of a table');
 $output = `/tmp/12345/use -N -e "select count(*) from test.table_1"`;
+#6
 is($output + 0, 1, 'Purged some rows ok');
 
+# Fail if --primary-key-only was specified and there is no PK in the table
+$sb->load_file('master', 't/pt-archiver/samples/pt_157.sql');
+$output = output(
+   sub { pt_archiver::main(qw(--where 1=1), "--source", "D=test,t=t1,F=$cnf", qw(--purge --primary-key-only)) },
+   stderr => 1,
+);
+like($output, qr/does not have a PRIMARY KEY/, 'Fail if --primary-key was specified and there is no PK');
+
 # #############################################################################
 # Done.
 # #############################################################################
diff --git a/t/pt-archiver/samples/pt_157.sql b/t/pt-archiver/samples/pt_157.sql
new file mode 100644
index 00000000..4bd1e18d
--- /dev/null
+++ b/t/pt-archiver/samples/pt_157.sql
@@ -0,0 +1,20 @@
+DROP DATABASE IF EXISTS test;
+CREATE DATABASE test;
+
+CREATE TABLE `test`.`t1` (
+  f1 INT NOT NULL,
+  f2 VARCHAR(10)
+) Engine=InnoDB;
+
+INSERT INTO `test`.`t1` VALUES
+(1, 'a'),
+(2, 'b'),
+(3, 'c'),
+(4, 'd'),
+(5, 'e'),
+(6, 'f'),
+(7, 'g'),
+(8, 'h'),
+(9, 'i'),
+(10, 'j');
+