pt-query-digest : Redundant argument in sprintf

[Romain GUINOT]

The current version of Perl (5.22.1) on Fedora has apparently stricter checks for arguments provided to sprintf.

Using the default report format, and using percona-toolkit 2.2.16 or previous versions, output of : pt-query-digest test.log returns :

Pipeline process 5 (iteration) caused an error: Redundant argument in sprintf at /opt/percona/toolkit/2.2.16/bin/pt-query-digest line 2556.
Will retry pipeline process 4 (iteration) 2 more times.

It is likely something introduced in recent versions of PerL.
Changing the return statement in sub shorten to :

   my $sprintf_expression = ($num =~ m/\./ || $n
         ? "%.${p}f%s"
         : '%d');

   return $units[$n] ? sprintf($sprintf_expression, $num, $units[$n]) : sprintf($sprintf_expression, $num);

makes the tool go a bit further without complaining but there are other sprintf statements to change as well.

Passing --report-format=profile returns without errors, so only specific / detailed modes are affected.
Let me know if you need more information.

[Frederic Descamps (lefred)]

similar issue than related in this bug: https://bugs.launchpad.net/percona-toolkit/+bug/1480719

[Romain GUINOT (romainguinot)]

adding the partial patch mentioned in the original comment, in patch format.

[Romain GUINOT (romainguinot)]

adding an output sample exhibiting further sprintf problems, even after the partial patch has been applied.

[Frank Cizmich (frank-cizmich)]

Hi Romain

Ugh! yes. Will have to weed out the offending printf's one by one.

Thanks for the partial fix.

Meanwhile, a (lazy) workaround is to use

no warnings 'redundant';

in the affected packages within the file.

[Frank Cizmich (frank-cizmich)]

Related to:

https://bugs.launchpad.net/percona-toolkit/+bug/1480719

[Romain GUINOT (romainguinot)]

Hi Frank,

I've just tried and yes the temporary workaround you mentioned works!
Much appreciated.

I've just needed to add it below the QueryReportFormatter package formatter

[Romain GUINOT (romainguinot)]

meant QueryReportFormatter package declaration

[Frank Cizmich (frank-cizmich)]

great!

thanks for the feedback.

[Romain GUINOT (romainguinot)]

Hi Frank,

just to clarify, the "no warnings" i've added was on top of my previous sprintf partial fix.
Did you take this into account ?

Also, where is the percona toolkit code now ? launchpad says you've committed a fix but couldn't find a corresponding commit on LP or GitHub.

Thanks!
Romain.

[Frank Cizmich (frank-cizmich)]

Hi Romain,

Yes, I had that into account.
The sprintf issue appeared in more than one place.
The "unified" commit for that is here:
https://github.com/percona/percona-toolkit/pull/73/

Since the printf formats and values are dynamically assembled (by design), I opted for using indexes in the format symbols, which keeps Perl happy, although at the cost of readability. I admit I was on the balance on whether to use "no warnings 'redundant'" as a simpler "definite" solution, but that would theoretically break if the wording of the warning changes. :-/

If you encounter problems please report. Feedback is crucial.

Regards and again thanks Romain

[Shahriyar Rzayev (rzayev-sehriyar)]

Percona now uses JIRA for bug reports so this bug report is migrated to: https://jira.percona.com/browse/PT-395