Merge lp:~percona-toolkit-dev/percona-toolkit/fix-885382 into lp:percona-toolkit/2.1

[Brian Fraser]

[Daniel Nichter (daniel-nichter)]

The option's value should be validated earlier, just after creating the OptionParser, so the tool doesn't get started only to die later. So instead of die() it will be $o->save_error().

It might also be nice to qr// the option's two values to ensure that they're valid regex.

Given that, the test in bugs.t can be moved to option_sanity.t.

review: Needs Fixing

[lp:~percona-toolkit-dev/percona-toolkit/fix-885382]

269. By Brian Fraser <email address hidden> on 2012-05-30

Fix for 885382: Check the cardinality of --embedded-attributes

[Daniel Nichter (daniel-nichter)]

review: Approve

[Brian Fraser (fraserbn)]

review: Approve

=== modified file 'bin/pt-query-digest'
--- bin/pt-query-digest	2012-05-24 17:25:20 +0000
+++ bin/pt-query-digest	2012-05-30 17:23:19 +0000
@@ -11974,6 +11974,18 @@
       if ( $o->get('apdex-threshold') <= 0 ) {
          $o->save_error("Apdex threshold must be a positive decimal value");
       }
+      if ( my $patterns = $o->get('embedded-attributes') ) {
+         $o->save_error("--embedded-attributes should be passed two "
+           . "comma-separated patterns, got " . scalar(@$patterns) )
+           unless scalar(@$patterns) == 2;
+         for my $re (@$patterns) {
+            no re 'eval';
+            eval { qr/$re/ };
+            if ( $EVAL_ERROR ) {
+               $o->save_error("--embedded-attributes $EVAL_ERROR")
+            }
+         }
+      }
    }
 
    # Set an orderby for each groupby; use the default orderby if there
@@ -12416,8 +12428,8 @@
       }  # get events from log file
 
       if ( my $patterns = $o->get('embedded-attributes') ) {
-         $misc->{embed}   = qr/$patterns->[0]/o;
-         $misc->{capture} = qr/$patterns->[1]/o;
+         $misc->{embed}   = qr/$patterns->[0]/;
+         $misc->{capture} = qr/$patterns->[1]/;
          PTDEBUG && _d('Patterns for embedded attributes:', $misc->{embed},
                $misc->{capture});
       }
 
=== modified file 't/pt-query-digest/option_sanity.t'
--- t/pt-query-digest/option_sanity.t	2011-07-12 22:56:55 +0000
+++ t/pt-query-digest/option_sanity.t	2012-05-30 17:23:19 +0000
@@ -9,7 +9,7 @@
 use strict;
 use warnings FATAL => 'all';
 use English qw(-no_match_vars);
-use Test::More tests => 2;
+use Test::More tests => 6;
 
 use PerconaTest;
 
@@ -22,7 +22,42 @@
 $output = `$trunk/bin/pt-query-digest --review h=127.1,P=12345,u=msandbox,p=msandbox,D=test`;
 like($output, qr/--review DSN requires a D/, 'Dies if no t part in --review DSN');
 
-
+# #############################################################################
+# https://bugs.launchpad.net/percona-toolkit/+bug/885382
+# pt-query-digest --embedded-attributes doesn't check cardinality
+# #############################################################################
+my $sample = "$trunk/t/lib/samples/slowlogs/";
+
+my @options = qw(
+   --report-format=query_report
+   --limit 10
+   --group-by file
+);
+
+$output = `$trunk/bin/pt-query-digest @options --embedded-attributes '-- .*' $sample.slow010.txt`;
+
+like $output,
+   qr/\Q--embedded-attributes should be passed two comma-separated patterns, got 1/,
+   'Bug 885382: --embedded-attributes cardinality';
+
+$output = `$trunk/bin/pt-query-digest @options --embedded-attributes '-- .*,(?{1234})' $sample.slow010.txt`;
+
+like $output,
+   qr/\Q--embedded-attributes Eval-group /,
+   "Bug 885382: --embedded-attributes rejects invalid patterns early";
+
+$output = `$trunk/bin/pt-query-digest @options --embedded-attributes '-- .*,(?*asdasd' $sample.slow010.txt`;
+
+like $output,
+   qr/\Q--embedded-attributes Sequence (?*...) not recognized/,
+   "Bug 885382: --embedded-attributes rejects invalid patterns early";
+
+$output = `$trunk/bin/pt-query-digest @options --embedded-attributes '-- .*,[:alpha:]' $sample.slow010.txt`;
+
+like $output,
+   qr/\Q--embedded-attributes POSIX syntax [: :] belongs inside character/,
+   "Bug 885382: --embedded-attributes rejects warning patterns early";;
+   
 # #############################################################################
 # Done.
 # #############################################################################