Skip to content

Commit 1e581be

Browse files
hpoettkerhpoettker
committed
PT-2378 - extended FP precision in pt-table-sync
pt-table-sync now uses up to 17 decimal digits when writing floating point numbers in the generated SQL statements. This is necessary to prevent unintended data changes.
1 parent ce20bc7 commit 1e581be

20 files changed

+194
-17
lines changed

Diff for: ‎bin/pt-archiver

+4-1
Original file line numberDiff line numberDiff line change
@@ -2993,7 +2993,10 @@ sub quote_val {
29932993
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
29942994
&& !$args{is_char}; # unless is_char is true
29952995

2996-
return $val if $args{is_float};
2996+
if ( $args{is_float} ) {
2997+
return sprintf("%.17g", $val) if $val - "$val" != 0;
2998+
return $val;
2999+
}
29973000

29983001
$val =~ s/(['\\])/\\$1/g;
29993002
return "'$val'";

Diff for: ‎bin/pt-deadlock-logger

+4-1
Original file line numberDiff line numberDiff line change
@@ -2088,7 +2088,10 @@ sub quote_val {
20882088
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
20892089
&& !$args{is_char}; # unless is_char is true
20902090

2091-
return $val if $args{is_float};
2091+
if ( $args{is_float} ) {
2092+
return sprintf("%.17g", $val) if $val - "$val" != 0;
2093+
return $val;
2094+
}
20922095

20932096
$val =~ s/(['\\])/\\$1/g;
20942097
return "'$val'";

Diff for: ‎bin/pt-duplicate-key-checker

+4-1
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ sub quote_val {
133133
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
134134
&& !$args{is_char}; # unless is_char is true
135135

136-
return $val if $args{is_float};
136+
if ( $args{is_float} ) {
137+
return sprintf("%.17g", $val) if $val - "$val" != 0;
138+
return $val;
139+
}
137140

138141
$val =~ s/(['\\])/\\$1/g;
139142
return "'$val'";

Diff for: ‎bin/pt-find

+4-1
Original file line numberDiff line numberDiff line change
@@ -1690,7 +1690,10 @@ sub quote_val {
16901690
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
16911691
&& !$args{is_char}; # unless is_char is true
16921692

1693-
return $val if $args{is_float};
1693+
if ( $args{is_float} ) {
1694+
return sprintf("%.17g", $val) if $val - "$val" != 0;
1695+
return $val;
1696+
}
16941697

16951698
$val =~ s/(['\\])/\\$1/g;
16961699
return "'$val'";

Diff for: ‎bin/pt-fk-error-logger

+4-1
Original file line numberDiff line numberDiff line change
@@ -1242,7 +1242,10 @@ sub quote_val {
12421242
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
12431243
&& !$args{is_char}; # unless is_char is true
12441244

1245-
return $val if $args{is_float};
1245+
if ( $args{is_float} ) {
1246+
return sprintf("%.17g", $val) if $val - "$val" != 0;
1247+
return $val;
1248+
}
12461249

12471250
$val =~ s/(['\\])/\\$1/g;
12481251
return "'$val'";

Diff for: ‎bin/pt-heartbeat

+4-1
Original file line numberDiff line numberDiff line change
@@ -3564,7 +3564,10 @@ sub quote_val {
35643564
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
35653565
&& !$args{is_char}; # unless is_char is true
35663566

3567-
return $val if $args{is_float};
3567+
if ( $args{is_float} ) {
3568+
return sprintf("%.17g", $val) if $val - "$val" != 0;
3569+
return $val;
3570+
}
35683571

35693572
$val =~ s/(['\\])/\\$1/g;
35703573
return "'$val'";

Diff for: ‎bin/pt-index-usage

+4-1
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,10 @@ sub quote_val {
589589
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
590590
&& !$args{is_char}; # unless is_char is true
591591

592-
return $val if $args{is_float};
592+
if ( $args{is_float} ) {
593+
return sprintf("%.17g", $val) if $val - "$val" != 0;
594+
return $val;
595+
}
593596

594597
$val =~ s/(['\\])/\\$1/g;
595598
return "'$val'";

Diff for: ‎bin/pt-kill

+4-1
Original file line numberDiff line numberDiff line change
@@ -4905,7 +4905,10 @@ sub quote_val {
49054905
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
49064906
&& !$args{is_char}; # unless is_char is true
49074907

4908-
return $val if $args{is_float};
4908+
if ( $args{is_float} ) {
4909+
return sprintf("%.17g", $val) if $val - "$val" != 0;
4910+
return $val;
4911+
}
49094912

49104913
$val =~ s/(['\\])/\\$1/g;
49114914
return "'$val'";

Diff for: ‎bin/pt-online-schema-change

+4-1
Original file line numberDiff line numberDiff line change
@@ -2866,7 +2866,10 @@ sub quote_val {
28662866
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
28672867
&& !$args{is_char}; # unless is_char is true
28682868

2869-
return $val if $args{is_float};
2869+
if ( $args{is_float} ) {
2870+
return sprintf("%.17g", $val) if $val - "$val" != 0;
2871+
return $val;
2872+
}
28702873

28712874
$val =~ s/(['\\])/\\$1/g;
28722875
return "'$val'";

Diff for: ‎bin/pt-query-digest

+4-1
Original file line numberDiff line numberDiff line change
@@ -1257,7 +1257,10 @@ sub quote_val {
12571257
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
12581258
&& !$args{is_char}; # unless is_char is true
12591259

1260-
return $val if $args{is_float};
1260+
if ( $args{is_float} ) {
1261+
return sprintf("%.17g", $val) if $val - "$val" != 0;
1262+
return $val;
1263+
}
12611264

12621265
$val =~ s/(['\\])/\\$1/g;
12631266
return "'$val'";

Diff for: ‎bin/pt-replica-restart

+4-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,10 @@ sub quote_val {
135135
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
136136
&& !$args{is_char}; # unless is_char is true
137137

138-
return $val if $args{is_float};
138+
if ( $args{is_float} ) {
139+
return sprintf("%.17g", $val) if $val - "$val" != 0;
140+
return $val;
141+
}
139142

140143
$val =~ s/(['\\])/\\$1/g;
141144
return "'$val'";

Diff for: ‎bin/pt-table-checksum

+4-1
Original file line numberDiff line numberDiff line change
@@ -4134,7 +4134,10 @@ sub quote_val {
41344134
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
41354135
&& !$args{is_char}; # unless is_char is true
41364136

4137-
return $val if $args{is_float};
4137+
if ( $args{is_float} ) {
4138+
return sprintf("%.17g", $val) if $val - "$val" != 0;
4139+
return $val;
4140+
}
41384141

41394142
$val =~ s/(['\\])/\\$1/g;
41404143
return "'$val'";

Diff for: ‎bin/pt-table-sync

+4-1
Original file line numberDiff line numberDiff line change
@@ -1909,7 +1909,10 @@ sub quote_val {
19091909
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
19101910
&& !$args{is_char}; # unless is_char is true
19111911

1912-
return $val if $args{is_float};
1912+
if ( $args{is_float} ) {
1913+
return sprintf("%.17g", $val) if $val - "$val" != 0;
1914+
return $val;
1915+
}
19131916

19141917
$val =~ s/(['\\])/\\$1/g;
19151918
return "'$val'";

Diff for: ‎bin/pt-table-usage

+4-1
Original file line numberDiff line numberDiff line change
@@ -6680,7 +6680,10 @@ sub quote_val {
66806680
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
66816681
&& !$args{is_char}; # unless is_char is true
66826682

6683-
return $val if $args{is_float};
6683+
if ( $args{is_float} ) {
6684+
return sprintf("%.17g", $val) if $val - "$val" != 0;
6685+
return $val;
6686+
}
66846687

66856688
$val =~ s/(['\\])/\\$1/g;
66866689
return "'$val'";

Diff for: ‎bin/pt-upgrade

+4-1
Original file line numberDiff line numberDiff line change
@@ -1254,7 +1254,10 @@ sub quote_val {
12541254
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
12551255
&& !$args{is_char}; # unless is_char is true
12561256

1257-
return $val if $args{is_float};
1257+
if ( $args{is_float} ) {
1258+
return sprintf("%.17g", $val) if $val - "$val" != 0;
1259+
return $val;
1260+
}
12581261

12591262
$val =~ s/(['\\])/\\$1/g;
12601263
return "'$val'";

Diff for: ‎lib/Quoter.pm

+4-1
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,10 @@ sub quote_val {
7878
&& !$args{is_char}; # unless is_char is true
7979

8080
# https://bugs.launchpad.net/percona-toolkit/+bug/1229861
81-
return $val if $args{is_float};
81+
if ( $args{is_float} ) {
82+
return sprintf("%.17g", $val) if $val - "$val" != 0;
83+
return $val;
84+
}
8285

8386
# Quote and return non-numeric vals.
8487
$val =~ s/(['\\])/\\$1/g;

Diff for: ‎t/lib/Quoter.t

+2
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ is( $q->quote_val('0x89504E470', is_char => 0), '0x89504E470', 'hex string, with
7171
is( $q->quote_val('0x89504E470', is_char => 1), "'0x89504E470'", 'hex string, with is_char => 1');
7272
is( $q->quote_val('0x89504I470'), "'0x89504I470'", 'looks like hex string');
7373
is( $q->quote_val('eastside0x3'), "'eastside0x3'", 'looks like hex str (issue 1110');
74+
is( $q->quote_val(969.1 / 360, is_float => 1), "2.6919444444444447", 'float has full precision');
75+
is( $q->quote_val(0.1, is_float => 1), "0.1", 'full precision for float only used when required');
7476

7577
# Splitting DB and tbl apart
7678
is_deeply(

Diff for: ‎t/pt-table-sync/float_precision.t

+17-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,23 @@ is(
6363
'--float-precision so no more diff (issue 410)'
6464
);
6565

66+
# Although the SQL statement contains serialized values with more than necessary decimal digits
67+
# we produce the expected value on execution
68+
$output = `$trunk/bin/pt-table-sync --sync-to-source h=127.1,P=12346,u=msandbox,p=msandbox,D=test,t=fl --execute 2>&1`;
69+
is(
70+
$output,
71+
'',
72+
'REPLACE statement can be successfully applied'
73+
);
74+
75+
$sb->wait_for_replicas();
76+
my @rows = $replica_dbh->selectrow_array('SELECT `d` FROM `test`.`fl` WHERE `d` = 2.0000012');
77+
is_deeply(
78+
\@rows,
79+
[2.0000012],
80+
'Floating point values are set correctly in round trip'
81+
);
82+
6683
# #############################################################################
6784
# pt-table-sync quotes floats, prevents syncing
6885
# https://bugs.launchpad.net/percona-toolkit/+bug/1229861
@@ -86,7 +103,6 @@ is_deeply(
86103
[],
87104
"Sync rows with float values (bug 1229861)"
88105
) or diag(Dumper($rows), $output);
89-
90106
# #############################################################################
91107
# Done.
92108
# #############################################################################

Diff for: ‎t/pt-table-sync/pt-2378.t

+96
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#!/usr/bin/env perl
2+
3+
BEGIN {
4+
die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n"
5+
unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH};
6+
unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib";
7+
};
8+
9+
use strict;
10+
use warnings FATAL => 'all';
11+
use English qw(-no_match_vars);
12+
use Test::More;
13+
14+
use PerconaTest;
15+
use Sandbox;
16+
require "$trunk/bin/pt-table-sync";
17+
18+
my $dp = new DSNParser(opts=>$dsn_opts);
19+
my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
20+
my $source_dbh = $sb->get_dbh_for('source');
21+
my $replica1_dbh = $sb->get_dbh_for('replica1');
22+
23+
if ( !$source_dbh ) {
24+
plan skip_all => 'Cannot connect to sandbox source';
25+
}
26+
elsif ( !$replica1_dbh ) {
27+
plan skip_all => 'Cannot connect to sandbox replica1';
28+
}
29+
else {
30+
plan tests => 5;
31+
}
32+
33+
my ($output, @rows);
34+
35+
# #############################################################################
36+
# Test generated REPLACE statements.
37+
# #############################################################################
38+
$sb->load_file('source', "t/pt-table-sync/samples/pt-2378.sql");
39+
$sb->wait_for_replicas();
40+
$replica1_dbh->do("update `test`.`test_table` set `some_string` = 'c' where `id` = 1");
41+
42+
$output = remove_traces(output(
43+
sub { pt_table_sync::main('--sync-to-source',
44+
'h=127.0.0.1,P=12346,u=msandbox,p=msandbox',
45+
qw(-t test.test_table --print --execute))
46+
},
47+
));
48+
chomp($output);
49+
is(
50+
$output,
51+
"REPLACE INTO `test`.`test_table`(`id`, `value1`, `value2`, `some_string`) VALUES ('1', 315.25999999999942, 2.6919444444444447, 'a');",
52+
"Floating point numbers are generated with sufficient precision in REPLACE statements"
53+
);
54+
55+
$sb->wait_for_replicas();
56+
my $query = 'SELECT * FROM `test`.`test_table` WHERE `value1` = 315.2599999999994 AND `value2` = 2.6919444444444447';
57+
@rows = $replica1_dbh->selectrow_array($query);
58+
is_deeply(
59+
\@rows,
60+
[1, 315.2599999999994, 2.6919444444444447, 'a'],
61+
'Floating point values are set correctly in round trip'
62+
);
63+
64+
# #############################################################################
65+
# Test generated UPDATE statements.
66+
# #############################################################################
67+
$sb->load_file('source', "t/pt-table-sync/samples/pt-2378.sql");
68+
$sb->wait_for_replicas();
69+
$replica1_dbh->do("update `test`.`test_table` set `some_string` = 'c' where `id` = 1");
70+
71+
$output = remove_traces(output(
72+
sub { pt_table_sync::main(qw(--print --execute),
73+
"h=127.0.0.1,P=12346,u=msandbox,p=msandbox,D=test,t=test_table",
74+
"h=127.0.0.1,P=12345,u=msandbox,p=msandbox,D=test,t=test_table");
75+
}
76+
));
77+
chomp($output);
78+
is(
79+
$output,
80+
"UPDATE `test`.`test_table` SET `value1`=315.25999999999942, `value2`=2.6919444444444447, `some_string`='c' WHERE `id`='1' LIMIT 1;",
81+
"Floating point numbers are generated with sufficient precision in UPDATE statements"
82+
);
83+
84+
@rows = $source_dbh->selectrow_array($query);
85+
is_deeply(
86+
\@rows,
87+
[1, 315.2599999999994, 2.6919444444444447, 'c'],
88+
'Floating point values are set correctly in round trip'
89+
);
90+
91+
# #############################################################################
92+
# Done.
93+
# #############################################################################
94+
$sb->wipe_clean($source_dbh);
95+
ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
96+
exit;

Diff for: ‎t/pt-table-sync/samples/pt-2378.sql

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
DROP DATABASE IF EXISTS test;
2+
CREATE DATABASE test;
3+
USE test;
4+
5+
CREATE TABLE `test_table` (
6+
`id` BIGINT AUTO_INCREMENT PRIMARY KEY,
7+
`value1` DOUBLE NOT NULL,
8+
`value2` DOUBLE NOT NULL,
9+
`some_string` VARCHAR(32) NOT NULL
10+
) ENGINE=InnoDB;
11+
12+
INSERT INTO `test_table`
13+
(`value1`, `value2`, `some_string`)
14+
VALUES
15+
(315.2599999999994, 2.6919444444444447, 'a');

0 commit comments

Comments
 (0)