Merge lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-does-not-work-with-sql_mode-ONLY_FULL_GROUP_BY-1019479 into lp:~percona-toolkit-dev/percona-toolkit/release-2.2.13

Proposed by Frank Cizmich
Status: Merged
Approved by: Daniel Nichter
Approved revision: 612
Merged at revision: 620
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-does-not-work-with-sql_mode-ONLY_FULL_GROUP_BY-1019479
Merge into: lp:~percona-toolkit-dev/percona-toolkit/release-2.2.13
Diff against target: 67 lines (+39/-0)
2 files modified
bin/pt-table-checksum (+20/-0)
t/pt-table-checksum/basics.t (+19/-0)
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/pt-table-checksum-does-not-work-with-sql_mode-ONLY_FULL_GROUP_BY-1019479
Reviewer Review Type Date Requested Status
Daniel Nichter Approve
Review via email: mp+245681@code.launchpad.net

Description of the change

Problem:
pt-table-checksum fails when the sql_mode ONLY_FULL_GROUP_BY is set, even though the query that fails is deterministic.

Solution:
disable this mode during the session. (leave all other modes on)

Notes:
- tried to modify the query to comply, but it is dynamically generated on the fly according to different parameters so a solution along these lines was either not elegant or not efficient.

- important issue to fix for upcoming 5.7 version since this sql_mode will be on by default.

To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-table-checksum'
2--- bin/pt-table-checksum 2014-11-11 13:28:27 +0000
3+++ bin/pt-table-checksum 2015-01-06 18:03:52 +0000
4@@ -9131,6 +9131,26 @@
5 return if $o->get('explain');
6 my $sql;
7
8+ # https://bugs.launchpad.net/percona-toolkit/+bug/1019479
9+ # sql_mode ONLY_FULL_GROUP_BY often raises error even when query is
10+ # safe and deterministic. It's best to turn it off for the session
11+ # at this point.
12+ $sql = 'SELECT @@SQL_MODE';
13+ PTDEBUG && _d($dbh, $sql);
14+ my ($sql_mode) = eval { $dbh->selectrow_array($sql) };
15+ if ( $EVAL_ERROR ) {
16+ die "Error getting the current SQL_MODE: $EVAL_ERROR";
17+ }
18+ $sql_mode =~ s/ONLY_FULL_GROUP_BY//i;
19+ $sql = qq[SET SQL_MODE='$sql_mode'];
20+ PTDEBUG && _d($dbh, $sql);
21+ eval { $dbh->do($sql) };
22+ if ( $EVAL_ERROR ) {
23+ die "Error setting SQL_MODE"
24+ . ": $EVAL_ERROR";
25+ }
26+
27+
28 # https://bugs.launchpad.net/percona-toolkit/+bug/919352
29 # The tool shouldn't blindly attempt to change binlog_format;
30 # instead, it should check if it's already set to STATEMENT.
31
32=== modified file 't/pt-table-checksum/basics.t'
33--- t/pt-table-checksum/basics.t 2014-08-05 19:51:34 +0000
34+++ t/pt-table-checksum/basics.t 2015-01-06 18:03:52 +0000
35@@ -56,6 +56,7 @@
36 $master_dbh->do("use $repl_db");
37 }
38
39+
40 # ############################################################################
41 # Default checksum and results. The tool does not technically require any
42 # options on well-configured systems (which the test env cannot be). With
43@@ -509,6 +510,24 @@
44 );
45
46 # #############################################################################
47+# Bug 1019479: does not work with sql_mode ONLY_FULL_GROUP_BY
48+# #############################################################################
49+
50+# add a couple more modes to test that commas don't affect setting
51+$master_dbh->do("SET sql_mode = 'NO_ZERO_DATE,ONLY_FULL_GROUP_BY,STRICT_ALL_TABLES'");
52+
53+# force chunk-size because bug doesn't show up if table done in one chunk
54+$exit_status = pt_table_checksum::main(@args,
55+ qw(--quiet --quiet -t sakila.actor --chunk-size=50));
56+
57+is(
58+ $exit_status,
59+ 0,
60+ "sql_mode ONLY_FULL_GROUP_BY is overidden"
61+);
62+
63+
64+# #############################################################################
65 # Done.
66 # #############################################################################
67 $sb->wipe_clean($master_dbh);

Subscribers

People subscribed via source and target branches

to all changes: