Merge lp:~percona-toolkit-dev/percona-toolkit/fix-821715-enable-local-infile-in-dsn into lp:percona-toolkit/2.1

Proposed by Brian Fraser
Status: Merged
Approved by: Daniel Nichter
Approved revision: 390
Merged at revision: 438
Proposed branch: lp:~percona-toolkit-dev/percona-toolkit/fix-821715-enable-local-infile-in-dsn
Merge into: lp:percona-toolkit/2.1
Diff against target: 512 lines (+132/-45) (has conflicts)
14 files modified
bin/pt-archiver (+29/-1)
bin/pt-upgrade (+25/-1)
lib/DSNParser.pm (+5/-1)
lib/PerconaTest.pm (+7/-1)
lib/Sandbox.pm (+13/-0)
sandbox/test-env (+1/-1)
t/lib/CompareResults.t (+0/-2)
t/lib/DSNParser.t (+38/-2)
t/pt-archiver/basics.t (+0/-1)
t/pt-archiver/bulk_insert.t (+2/-5)
t/pt-archiver/bulk_regular_insert.t (+2/-5)
t/pt-upgrade/basics.t (+3/-15)
t/pt-upgrade/warnings.t (+1/-8)
util/check-load-data (+6/-2)
Text conflict in lib/Sandbox.pm
To merge this branch: bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-821715-enable-local-infile-in-dsn
Reviewer Review Type Date Requested Status
Daniel Nichter Approve
Review via email: mp+130487@code.launchpad.net

This proposal supersedes a proposal from 2012-08-31.

Description of the change

Resubmitted with the changes in Daniel's review.

To post a comment you must log in.
Revision history for this message
Daniel Nichter (daniel-nichter) : Posted in a previous version of this proposal
review: Disapprove
Revision history for this message
Daniel Nichter (daniel-nichter) wrote :

L DSN part is a winner.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'bin/pt-archiver'
2--- bin/pt-archiver 2012-10-08 21:02:17 +0000
3+++ bin/pt-archiver 2012-10-19 06:40:30 +0000
4@@ -2145,7 +2145,8 @@
5 . join(';', map { "$opts{$_}->{dsn}=$info->{$_}" }
6 grep { defined $info->{$_} }
7 qw(F h P S A))
8- . ';mysql_read_default_group=client';
9+ . ';mysql_read_default_group=client'
10+ . ($info->{L} ? ';mysql_local_infile=1' : '');
11 }
12 PTDEBUG && _d($dsn);
13 return ($dsn, $info->{u}, $info->{p});
14@@ -2174,6 +2175,9 @@
15 mysql_enable_utf8 => ($cxn_string =~ m/charset=utf8/i ? 1 : 0),
16 };
17 @{$defaults}{ keys %$opts } = values %$opts;
18+ if (delete $defaults->{L}) { # L for LOAD DATA LOCAL INFILE, our own extension
19+ $defaults->{mysql_local_infile} = 1;
20+ }
21
22 if ( $opts->{mysql_use_result} ) {
23 $defaults->{mysql_use_result} = 1;
24@@ -6454,6 +6458,10 @@
25 The L<"--low-priority-insert">, L<"--replace">, and L<"--ignore"> options work
26 with this option, but L<"--delayed-insert"> does not.
27
28+If C<LOAD DATA LOCAL INFILE> throws an error in the lines of C<The used
29+command is not allowed with this MySQL version>, refer to the documentation
30+for the C<L> DSN option.
31+
32 =item --charset
33
34 short form: -A; type: string
35@@ -7184,6 +7192,26 @@
36
37 Index to use.
38
39+=item * L
40+
41+copy: yes
42+
43+Explicitly enable LOAD DATA LOCAL INFILE.
44+
45+For some reason, some vendors compile libmysql without the
46+--enable-local-infile option, which disables the statement. This can
47+lead to weird situations, like the server allowing LOCAL INFILE, but
48+the client throwing exceptions if it's used.
49+
50+However, as long as the server allows LOAD DATA, clients can easily
51+reenable it; See L<https://dev.mysql.com/doc/refman/5.0/en/load-data-local.html>
52+and L<http://search.cpan.org/~capttofu/DBD-mysql/lib/DBD/mysql.pm>.
53+This option does exactly that.
54+
55+Although we've not found a case where turning this option leads to errors or
56+differing behavior, to be on the safe side, this option is not
57+on by default.
58+
59 =item * m
60
61 copy: no
62
63=== modified file 'bin/pt-upgrade'
64--- bin/pt-upgrade 2012-10-08 21:02:17 +0000
65+++ bin/pt-upgrade 2012-10-19 06:40:30 +0000
66@@ -245,7 +245,8 @@
67 . join(';', map { "$opts{$_}->{dsn}=$info->{$_}" }
68 grep { defined $info->{$_} }
69 qw(F h P S A))
70- . ';mysql_read_default_group=client';
71+ . ';mysql_read_default_group=client'
72+ . ($info->{L} ? ';mysql_local_infile=1' : '');
73 }
74 PTDEBUG && _d($dsn);
75 return ($dsn, $info->{u}, $info->{p});
76@@ -274,6 +275,9 @@
77 mysql_enable_utf8 => ($cxn_string =~ m/charset=utf8/i ? 1 : 0),
78 };
79 @{$defaults}{ keys %$opts } = values %$opts;
80+ if (delete $defaults->{L}) { # L for LOAD DATA LOCAL INFILE, our own extension
81+ $defaults->{mysql_local_infile} = 1;
82+ }
83
84 if ( $opts->{mysql_use_result} ) {
85 $defaults->{mysql_use_result} = 1;
86@@ -12950,6 +12954,26 @@
87
88 Connect to host.
89
90+=item * L
91+
92+copy: yes
93+
94+Explicitly enable LOAD DATA LOCAL INFILE.
95+
96+For some reason, some vendors compile libmysql without the
97+--enable-local-infile option, which disables the statement. This can
98+lead to weird situations, like the server allowing LOCAL INFILE, but
99+the client throwing exceptions if it's used.
100+
101+However, as long as the server allows LOAD DATA, clients can easily
102+reenable it; See L<https://dev.mysql.com/doc/refman/5.0/en/load-data-local.html>
103+and L<http://search.cpan.org/~capttofu/DBD-mysql/lib/DBD/mysql.pm>.
104+This option does exactly that.
105+
106+Although we've not found a case where turning this option leads to errors or
107+differing behavior, to be on the safe side, this option is not
108+on by default.
109+
110 =item * p
111
112 dsn: password; copy: yes
113
114=== modified file 'lib/DSNParser.pm'
115--- lib/DSNParser.pm 2012-08-02 12:52:42 +0000
116+++ lib/DSNParser.pm 2012-10-19 06:40:30 +0000
117@@ -244,7 +244,8 @@
118 . join(';', map { "$opts{$_}->{dsn}=$info->{$_}" }
119 grep { defined $info->{$_} }
120 qw(F h P S A))
121- . ';mysql_read_default_group=client';
122+ . ';mysql_read_default_group=client'
123+ . ($info->{L} ? ';mysql_local_infile=1' : '');
124 }
125 PTDEBUG && _d($dsn);
126 return ($dsn, $info->{u}, $info->{p});
127@@ -277,6 +278,9 @@
128 mysql_enable_utf8 => ($cxn_string =~ m/charset=utf8/i ? 1 : 0),
129 };
130 @{$defaults}{ keys %$opts } = values %$opts;
131+ if (delete $defaults->{L}) { # L for LOAD DATA LOCAL INFILE, our own extension
132+ $defaults->{mysql_local_infile} = 1;
133+ }
134
135 # Only add this if explicitly set because we're not sure if
136 # mysql_use_result=0 would leave default mysql_store_result
137
138=== modified file 'lib/PerconaTest.pm'
139--- lib/PerconaTest.pm 2012-08-29 20:23:41 +0000
140+++ lib/PerconaTest.pm 2012-10-19 06:40:30 +0000
141@@ -138,6 +138,12 @@
142 dsn => 'user',
143 copy => 1,
144 },
145+ {
146+ key => 'L',
147+ desc => 'Pass mysql_local_infile to DBD::mysql',
148+ dsn => 'mysql_local_infile',
149+ copy => 1,
150+ },
151 ];
152
153 # Runs code, captures and returns its output.
154@@ -788,7 +794,7 @@
155
156 sub can_load_data {
157 my $output = `/tmp/12345/use -e "SELECT * FROM percona_test.load_data" 2>/dev/null`;
158- return ($output || '') =~ /42/;
159+ return ($output || '') =~ /1/;
160 }
161
162 1;
163
164=== modified file 'lib/Sandbox.pm'
165--- lib/Sandbox.pm 2012-10-15 16:53:37 +0000
166+++ lib/Sandbox.pm 2012-10-19 06:40:30 +0000
167@@ -118,6 +118,10 @@
168 my $dp = $self->{DSNParser};
169 my $dsn = $dp->parse('h=127.0.0.1,u=msandbox,p=msandbox,P=' . $port_for{$server});
170 my $dbh;
171+ # This is primarily for the benefit of CompareResults, but it's
172+ # also quite convenient when using an affected OS
173+ $cxn_ops->{L} = 1 if !exists $cxn_ops->{L}
174+ && !$self->can_load_data('master');
175 eval { $dbh = $dp->get_dbh($dp->get_cxn_params($dsn), $cxn_ops) };
176 if ( $EVAL_ERROR ) {
177 PTDEBUG && _d('Failed to get dbh for', $server, ':', $EVAL_ERROR);
178@@ -398,6 +402,7 @@
179 return;
180 }
181
182+<<<<<<< TREE
183 sub is_cluster_node {
184 my ($self, $server) = @_;
185
186@@ -412,6 +417,14 @@
187 : 0;
188 }
189
190+=======
191+sub can_load_data {
192+ my ($self, $server) = @_;
193+ my $output = $self->use($server, q{-e "SELECT * FROM percona_test.load_data"});
194+ return ($output || '') =~ /1/;
195+}
196+
197+>>>>>>> MERGE-SOURCE
198 1;
199 }
200 # ###########################################################################
201
202=== modified file 'sandbox/test-env'
203--- sandbox/test-env 2012-10-16 21:46:17 +0000
204+++ sandbox/test-env 2012-10-19 06:40:30 +0000
205@@ -318,7 +318,7 @@
206
207 # LOAD DATA is disabled or broken on some boxes.
208 # PerconaTest exports $can_load_data which is true
209- # if percona_test.load_data has the 42 row,
210+ # if percona_test.load_data has a row with 1,
211 # signaling that LOAD DATA LOCAL INFILE worked.
212 ../util/check-load-data
213
214
215=== modified file 't/lib/CompareResults.t'
216--- t/lib/CompareResults.t 2012-07-31 22:34:38 +0000
217+++ t/lib/CompareResults.t 2012-10-19 06:40:30 +0000
218@@ -303,7 +303,6 @@
219 # #############################################################################
220 my $tmpdir = '/tmp/mk-upgrade-res';
221 SKIP: {
222- skip "LOAD DATA LOCAL INFILE is disabled", 30 unless $can_load_data;
223
224 diag(`rm -rf $tmpdir 2>/dev/null; mkdir $tmpdir`);
225
226@@ -727,7 +726,6 @@
227 );
228
229 SKIP: {
230- skip "LOAD DATA LOCAL INFILE is disabled", 2 unless $can_load_data;
231
232 $cr = new CompareResults(
233 method => 'rows',
234
235=== modified file 't/lib/DSNParser.t'
236--- t/lib/DSNParser.t 2012-08-15 16:26:13 +0000
237+++ t/lib/DSNParser.t 2012-10-19 06:40:30 +0000
238@@ -218,7 +218,7 @@
239 { S => 'bar', h => 'host' } ))
240 ],
241 [
242- 'DBI:mysql:foo;host=me;mysql_socket=bar;mysql_read_default_group=client',
243+ 'DBI:mysql:foo;host=me;mysql_socket=bar;mysql_read_default_group=client;mysql_local_infile=1',
244 'a',
245 'b',
246 ],
247@@ -234,7 +234,7 @@
248 { S => 'bar', h => 'host' } ))
249 ],
250 [
251- 'DBI:mysql:foo;host=me;mysql_socket=bar;charset=foo;mysql_read_default_group=client',
252+ 'DBI:mysql:foo;host=me;mysql_socket=bar;charset=foo;mysql_read_default_group=client;mysql_local_infile=1',
253 'a',
254 'b',
255 ],
256@@ -568,6 +568,42 @@
257 qr/\QUnknown system variable 'time_zoen'/,
258 "get_dbh dies with an unknown system variable"
259 );
260+$dp->prop('set-vars', undef);
261+
262+# #############################################################################
263+# LOAD DATA LOCAL INFILE broken in some platforms
264+# https://bugs.launchpad.net/percona-toolkit/+bug/821715
265+# #############################################################################
266+
267+SKIP: {
268+ skip "LOAD DATA LOCAL INFILE already works here", 1 if $can_load_data;
269+ my $dbh = $dp->get_dbh( $dp->get_cxn_params( $dsn ) );
270+
271+ use File::Temp qw(tempfile);
272+
273+ my ($fh, $filename) = tempfile( 'load_data_test.XXXXXXX', TMPDIR => 1 );
274+ print { $fh } "42\n";
275+ close $fh or die "Cannot close $filename: $!";
276+
277+ $dbh->do(q{DROP DATABASE IF EXISTS bug_821715});
278+ $dbh->do(q{CREATE DATABASE bug_821715});
279+ $dbh->do(q{CREATE TABLE IF NOT EXISTS bug_821715.load_data (i int)});
280+
281+ eval {
282+ $dbh->do(qq{LOAD DATA LOCAL INFILE '$filename' INTO TABLE bug_821715.load_data});
283+ };
284+
285+ is(
286+ $EVAL_ERROR,
287+ '',
288+ "Even though LOCAL INFILE is off by default, the dbhs returned by DSNParser can use it"
289+ );
290+
291+ unlink $filename;
292+
293+ $dbh->do(q{DROP DATABASE IF EXISTS bug_821715});
294+ $dbh->disconnect();
295+}
296
297 # #############################################################################
298 # Done.
299
300=== modified file 't/pt-archiver/basics.t'
301--- t/pt-archiver/basics.t 2012-08-15 17:33:04 +0000
302+++ t/pt-archiver/basics.t 2012-10-19 06:40:30 +0000
303@@ -186,7 +186,6 @@
304 # Bug 903387: pt-archiver doesn't honor b=1 flag to create SQL_LOG_BIN statement
305 # #############################################################################
306 SKIP: {
307- skip('LOAD DATA LOCAL INFILE is disabled', 3) if !$can_load_data;
308 $sb->load_file('master', "t/pt-archiver/samples/bulk_regular_insert.sql");
309 $sb->wait_for_slaves();
310
311
312=== modified file 't/pt-archiver/bulk_insert.t'
313--- t/pt-archiver/bulk_insert.t 2012-07-31 21:37:02 +0000
314+++ t/pt-archiver/bulk_insert.t 2012-10-19 06:40:30 +0000
315@@ -22,9 +22,6 @@
316 if ( !$dbh ) {
317 plan skip_all => 'Cannot connect to sandbox master';
318 }
319-elsif ( !$can_load_data ) {
320- plan skip_all => 'LOAD DATA LOCAL INFILE is disabled';
321-}
322
323 my $output;
324 my $rows;
325@@ -41,7 +38,7 @@
326 $output = output(
327 sub { pt_archiver::main(qw(--no-ascend --limit 50 --bulk-insert),
328 qw(--bulk-delete --where 1=1 --statistics),
329- '--source', "D=test,t=table_5,F=$cnf",
330+ '--source', "L=1,D=test,t=table_5,F=$cnf",
331 '--dest', "t=table_5_dest") },
332 );
333 like($output, qr/SELECT 105/, 'Fetched 105 rows');
334@@ -66,7 +63,7 @@
335 sub { pt_archiver::main(
336 '--where', "id < 8", qw(--limit 100000 --txn-size 1000),
337 qw(--why-quit --statistics --bulk-insert),
338- '--source', "D=bri,t=t,F=$cnf",
339+ '--source', "L=1,D=bri,t=t,F=$cnf",
340 '--dest', "t=t_arch") },
341 );
342 $rows = $dbh->selectall_arrayref('select id from bri.t order by id');
343
344=== modified file 't/pt-archiver/bulk_regular_insert.t'
345--- t/pt-archiver/bulk_regular_insert.t 2012-07-31 21:37:02 +0000
346+++ t/pt-archiver/bulk_regular_insert.t 2012-10-19 06:40:30 +0000
347@@ -22,9 +22,6 @@
348 if ( !$dbh ) {
349 plan skip_all => 'Cannot connect to sandbox master';
350 }
351-elsif ( !$can_load_data ) {
352- plan skip_all => 'LOAD DATA LOCAL INFILE is disabled';
353-}
354
355 my $output;
356 my $cnf = "/tmp/12345/my.sandbox.cnf";
357@@ -39,7 +36,7 @@
358 $dbh->do('use bri');
359
360 output(
361- sub { pt_archiver::main("--source", "F=$cnf,D=bri,t=t", qw(--dest t=t_arch --where 1=1 --bulk-insert --limit 3)) },
362+ sub { pt_archiver::main("--source", "F=$cnf,D=bri,t=t,L=1", qw(--dest t=t_arch --where 1=1 --bulk-insert --limit 3)) },
363 );
364
365 my $t_rows = $dbh->selectall_arrayref('select * from t order by id');
366@@ -73,7 +70,7 @@
367 $sb->load_file('master', "t/pt-archiver/samples/bulk_regular_insert.sql");
368 $dbh->do('use bri');
369
370-`$cmd --source F=$cnf,D=bri,t=t --dest t=t_arch,m=bulk_regular_insert --where "1=1" --bulk-insert --limit 3`;
371+`$cmd --source F=$cnf,D=bri,t=t,L=1 --dest t=t_arch,m=bulk_regular_insert --where "1=1" --bulk-insert --limit 3`;
372
373 my $bri_t_rows = $dbh->selectall_arrayref('select * from t order by id');
374 my $bri_t_arch_rows = $dbh->selectall_arrayref('select * from t_arch order by id');
375
376=== modified file 't/pt-upgrade/basics.t'
377--- t/pt-upgrade/basics.t 2012-07-31 21:37:02 +0000
378+++ t/pt-upgrade/basics.t 2012-10-19 06:40:30 +0000
379@@ -30,7 +30,7 @@
380 plan skip_all => 'Cannot connect to second sandbox master';
381 }
382
383-my @host_args = ('h=127.1,P=12345', 'P=12348');
384+my @host_args = ('h=127.1,P=12345,L=1', 'P=12348');
385 my @op_args = (qw(-u msandbox -p msandbox),
386 '--compare', 'results,warnings',
387 '--zero-query-times',
388@@ -61,9 +61,6 @@
389 'Report for multiple queries (checksum method)'
390 );
391
392-SKIP: {
393- skip "LOAD DATA LOCAL INFILE is disabled", 2 unless $can_load_data;
394-
395 ok(
396 no_diff(
397 sub { pt_upgrade::main(@args, "$trunk/$sample/001/select-one.log",
398@@ -81,7 +78,6 @@
399 ),
400 'Report for multiple queries (rows method)'
401 );
402-}
403
404 ok(
405 no_diff(
406@@ -108,9 +104,6 @@
407 # Issue 951: mk-upgrade "I need a db argument" error with
408 # compare-results-method=rows
409 # #############################################################################
410-SKIP: {
411- skip "LOAD DATA LOCAL INFILE is disabled", 4 unless $can_load_data;
412-
413 $sb->load_file('master', "$sample/002/tables.sql");
414 $sb->load_file('master1', "$sample/002/tables.sql");
415
416@@ -120,7 +113,7 @@
417 ok(
418 no_diff(
419 sub { pt_upgrade::main(@op_args, "$log/002/no-db.log",
420- 'h=127.1,P=12345,D=test', 'P=12348,D=test',
421+ 'h=127.1,P=12345,D=test,L=1', 'P=12348,D=test',
422 qw(--compare-results-method rows --temp-database test)) },
423 "$sample/002/report-01.txt",
424 ),
425@@ -134,7 +127,7 @@
426 ok(
427 no_diff(
428 sub { pt_upgrade::main(@op_args, "$log/002/no-db.log",
429- 'h=127.1,P=12345,D=test', 'P=12348,D=test',
430+ 'h=127.1,P=12345,D=test,L=1', 'P=12348,D=test',
431 qw(--compare-results-method rows --temp-database tmp_db)) },
432 "$sample/002/report-01.txt",
433 ),
434@@ -155,15 +148,11 @@
435
436 $sb->wipe_clean($dbh1);
437 $sb->wipe_clean($dbh2);
438-}
439
440 # #############################################################################
441 # Bug 926598: DBD::mysql bug causes pt-upgrade to use wrong
442 # precision (M) and scale (D)
443 # #############################################################################
444-SKIP: {
445- skip "LOAD DATA LOCAL INFILE is disabled", 2 unless $can_load_data;
446-
447 $sb->load_file('master', "$sample/003/tables.sql");
448 $sb->load_file('master1', "$sample/003/tables.sql");
449
450@@ -185,7 +174,6 @@
451 qr/[`"]SUM\(total\)[`"]\s+double\sDEFAULT/i,
452 "No M,D in table def (bug 926598)"
453 );
454-}
455
456 # #############################################################################
457 # Done.
458
459=== modified file 't/pt-upgrade/warnings.t'
460--- t/pt-upgrade/warnings.t 2012-07-31 21:37:02 +0000
461+++ t/pt-upgrade/warnings.t 2012-10-19 06:40:30 +0000
462@@ -15,13 +15,6 @@
463 use Sandbox;
464 require "$trunk/bin/pt-upgrade";
465
466-# This test calls pt-upgrade with --compare-results-method rows
467-# which use LOAD DATA LOCAL INFILE. If LOAD DATA is disabled,
468-# then this this test can't run.
469-if ( !$can_load_data ) {
470- plan skip_all => 'LOAD DATA LOCAL INFILE is disabled';
471-}
472-
473 # This runs immediately if the server is already running, else it starts it.
474 diag(`$trunk/sandbox/start-sandbox master 12348 >/dev/null`);
475
476@@ -43,7 +36,7 @@
477 $sb->load_file('master1', 't/pt-upgrade/samples/001/tables.sql');
478
479 my $output;
480-my $cmd = "$trunk/bin/pt-upgrade h=127.1,P=12345,u=msandbox,p=msandbox P=12348 --compare results,warnings --zero-query-times --compare-results-method rows --limit 10";
481+my $cmd = "$trunk/bin/pt-upgrade h=127.1,P=12345,u=msandbox,p=msandbox,L=1 P=12348 --compare results,warnings --zero-query-times --compare-results-method rows --limit 10";
482
483 # This test really deals with,
484 # http://code.google.com/p/maatkit/issues/detail?id=754
485
486=== modified file 'util/check-load-data'
487--- util/check-load-data 2012-07-31 21:57:50 +0000
488+++ util/check-load-data 2012-10-19 06:40:30 +0000
489@@ -41,17 +41,21 @@
490
491 $dbh->do("CREATE TABLE IF NOT EXISTS percona_test.load_data (i int)");
492
493-`echo 42 > /tmp/load_data_test.$$`;
494+`echo 1 > /tmp/load_data_test.$$`;
495
496 eval {
497 $dbh->do("LOAD DATA LOCAL INFILE '/tmp/load_data_test.$$' INTO TABLE percona_test.load_data");
498 };
499
500+if ( $EVAL_ERROR ) {
501+ $dbh->do("INSERT INTO percona_test.load_data (i) VALUES (0)");
502+}
503+
504 unlink "/tmp/load_data_test.$$";
505
506 my ($val) = $dbh->selectrow_array("SELECT i FROM percona_test.load_data");
507
508-if ( ($val || 0) == 42 ) {
509+if ( ($val || 0) == 1 ) {
510 print "LOAD DATA LOCAL INFILE is enabled\n";
511 }
512 else {

Subscribers

People subscribed via source and target branches