Merge lp:~percona-toolkit-dev/percona-toolkit/pt-upgrade-fails-on-SELECT-INTO-queries-1421781 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.14

[Frank Cizmich]

Proplem:

pt-upgrade fails when log contains queries of the form:
SELECT...INTO

This is because the tool treats it as a regular select that returns rows, so it performs first the execute(), which works fine, but then fails when attempting to fetch() since DBI considers the operation already over.

Solution:

Filter these out via regex. (excludes the three forms , OUTFILE, DUMPFILE and @variable)

Notes:
- changes made in 2 place: module ResultWriter and tool's main()
- also created test case

[Daniel Nichter (daniel-nichter)]

Would be good to document that these queries are skipped/ignored.

[Daniel Nichter (daniel-nichter)]

review: Needs Information

[lp:~percona-toolkit-dev/percona-toolkit/pt-upgrade-fails-on-SELECT-INTO-queries-1421781]

615. By Frank Cizmich on 2015-04-06

Added info in doc. Minor test tweak

[Daniel Nichter (daniel-nichter)]

"but dump output to a file or variable)" needs a period. :-) Otherwise code is fine.

review: Approve

[lp:~percona-toolkit-dev/percona-toolkit/pt-upgrade-fails-on-SELECT-INTO-queries-1421781]

616. By Frank Cizmich on 2015-04-07

minor typo

[Frank Cizmich (frank-cizmich)]

Added period. (Inside the parentheses like in this case.)

In other cases it would go outside, (like here for example).

:-)

=== modified file 'bin/pt-upgrade'
--- bin/pt-upgrade	2015-01-23 10:19:56 +0000
+++ bin/pt-upgrade	2015-04-07 20:07:53 +0000
@@ -6304,7 +6304,11 @@
    else {
       my $rows;
       if ( my $sth = $results->{sth} ) {
-         if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i ) {
+         # Only fetch rows of select statements
+         # *except* when they are directed INTO 
+         # a file or a variable. (issue lp:1421781)
+         if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i 
+            &&  $event->{arg} !~ /INTO\s*(?:OUTFILE|DUMPFILE|@)/ ) {
             $rows = $sth->fetchall_arrayref();
          }
          eval {
@@ -10120,9 +10124,11 @@
          ignore_warnings => $ignore_warnings,
       );
 
-      # Only SELECT statements return rows.
+      # Only SELECT statements return rows, *except* when they are directed 
+      # INTO a file or a variable.
       my $row_diffs;
-      if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i ) {
+      if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i 
+         &&  $event->{arg} !~ m/INTO\s*(?:OUTFILE|DUMPFILE|@)/i ) {
          $row_diffs = diff_rows(
             sth1 => $results1->{sth},
             sth2 => $results2->{sth},
@@ -10608,6 +10614,8 @@
 =head2 READ-ONLY
 
 By default, pt-upgrade only executes C<SELECT> and C<SET> statements.
+(This does not include 'SELECT...INTO' statements, which do not return
+rows but dump output to a file or variable.)
 If you're using recreatable test or development servers and wish to
 compare write statements too (e.g. C<INSERT>, C<UPDATE>, C<DELETE>),
 then specify C<--no-read-only>.  If using a binary log, you must

=== modified file 'lib/ResultWriter.pm'
--- lib/ResultWriter.pm	2013-02-20 16:37:43 +0000
+++ lib/ResultWriter.pm	2015-04-07 20:07:53 +0000
@@ -128,9 +128,11 @@
    }
    else {
       # Save rows, if any (i.e. if it's a SELECT statement).
+      # *except* if it's a SELECT...INTO (issue lp:1421781) 
       my $rows;
       if ( my $sth = $results->{sth} ) {
-         if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i ) {
+         if ( $event->{arg} =~ m/(?:^\s*SELECT|(?:\*\/\s*SELECT))/i 
+            &&  $event->{arg} !~ /INTO\s*(?:OUTFILE|DUMPFILE|@)/ ) {
             $rows = $sth->fetchall_arrayref();
          }
          eval {

=== added file 't/pt-upgrade/issue_1421781.t'
--- t/pt-upgrade/issue_1421781.t	1970-01-01 00:00:00 +0000
+++ t/pt-upgrade/issue_1421781.t	2015-04-07 20:07:53 +0000
@@ -0,0 +1,66 @@
+#!/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 File::Basename;
+use File::Temp qw(tempdir);
+
+$ENV{PERCONA_TOOLKIT_TEST_USE_DSN_NAMES} = 1; 
+$ENV{PRETTY_RESULTS} = 1; 
+
+use PerconaTest;
+use Sandbox;
+require "$trunk/bin/pt-upgrade";
+
+my $dp = new DSNParser(opts=>$dsn_opts);
+my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
+my $dbh1 = $sb->get_dbh_for('host1');
+
+
+if ( !$dbh1 ) {
+   plan skip_all => 'Cannot connect to sandbox host1'; 
+}
+
+my $host1_dsn   = $sb->dsn_for('host1');
+my $tmpdir      = tempdir("/tmp/pt-upgrade.$PID.XXXXXX", CLEANUP => 1);
+my $samples     = "$trunk/t/pt-upgrade/samples";
+my $lib_samples = "$trunk/t/lib/samples";
+my $exit_status = 0;
+my $output;
+
+# #############################################################################
+# genlog
+# #############################################################################
+
+`rm -f /tmp/test_select_into_*.log`;
+
+$output = output(
+   sub {
+      $exit_status = pt_upgrade::main($host1_dsn, '--save-results', $tmpdir,
+         qw(--type rawlog),
+         "$samples/select_into.log",
+   )},
+   stderr => 1,
+);
+
+is(
+   $exit_status,
+   0,
+   "Does not fail on SELECT...INTO statements"
+);
+
+
+# #############################################################################
+# Done.
+# #############################################################################
+$sb->wipe_clean($dbh1);
+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+done_testing;

=== added file 't/pt-upgrade/samples/select_into.log'
--- t/pt-upgrade/samples/select_into.log	1970-01-01 00:00:00 +0000
+++ t/pt-upgrade/samples/select_into.log	2015-04-07 20:07:53 +0000
@@ -0,0 +1,4 @@
+SELECT 1 INTO @foo;
+SELECT * FROM sakila.actor INTO OUTFILE '/tmp/test_select_into_1.log'; 
+SELECT actor_id,first_name FROM sakila.actor LIMIT 1 INTO DUMPFILE '/tmp/test_select_into_2.log'; 
+