Skip to content

Commit

Permalink
PT-2378 - extended FP precision in pt-table-sync
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hpoettker committed Sep 12, 2024
1 parent 206404f commit c0f3c96
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 2 deletions.
2 changes: 1 addition & 1 deletion bin/pt-table-sync
Original file line number Diff line number Diff line change
Expand Up @@ -1909,7 +1909,7 @@ sub quote_val {
return $val if $val =~ m/^0x[0-9a-fA-F]+$/ # quote hex data
&& !$args{is_char}; # unless is_char is true

return $val if $args{is_float};
return sprintf("%.17g", $val) if $args{is_float};

$val =~ s/(['\\])/\\$1/g;
return "'$val'";
Expand Down
19 changes: 18 additions & 1 deletion t/pt-table-sync/float_precision.t
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ $output = `$trunk/bin/pt-table-sync --sync-to-master h=127.1,P=12346,u=msandbox,
$output = remove_traces($output);
is(
$output,
"REPLACE INTO `test`.`fl`(`id`, `f`, `d`) VALUES ('1', 1.0000011921, 2.0000012);
"REPLACE INTO `test`.`fl`(`id`, `f`, `d`) VALUES ('1', 1.0000011921, 2.0000011999999998);
",
'No --float-precision so double col diff at high precision (issue 410)'
);
Expand All @@ -63,6 +63,23 @@ is(
'--float-precision so no more diff (issue 410)'
);

# Although the SQL statement contains serialized values with more than necessary decimal digits
# we produce the expected value on execution
$output = `$trunk/bin/pt-table-sync --sync-to-master h=127.1,P=12346,u=msandbox,p=msandbox,D=test,t=fl --execute 2>&1`;
is(
$output,
'',
'REPLACE statement can be successfully applied'
);

$sb->wait_for_slaves();
my @rows = $slave_dbh->selectrow_array('SELECT `d` FROM `test`.`fl` WHERE `d` = 2.0000012');
is_deeply(
\@rows,
[2.0000012],
'Floating point values are set correctly in round trip'
);

# #############################################################################
# pt-table-sync quotes floats, prevents syncing
# https://bugs.launchpad.net/percona-toolkit/+bug/1229861
Expand Down
96 changes: 96 additions & 0 deletions t/pt-table-sync/pt-2378.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
#!/usr/bin/env perl

BEGIN {
die "The PERCONA_TOOLKIT_BRANCH environment variable is not set.\n"
unless $ENV{PERCONA_TOOLKIT_BRANCH} && -d $ENV{PERCONA_TOOLKIT_BRANCH};
unshift @INC, "$ENV{PERCONA_TOOLKIT_BRANCH}/lib";
};

use strict;
use warnings FATAL => 'all';
use English qw(-no_match_vars);
use Test::More;

use PerconaTest;
use Sandbox;
require "$trunk/bin/pt-table-sync";

my $dp = new DSNParser(opts=>$dsn_opts);
my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp);
my $master_dbh = $sb->get_dbh_for('master');
my $slave1_dbh = $sb->get_dbh_for('slave1');

if ( !$master_dbh ) {
plan skip_all => 'Cannot connect to sandbox master';
}
elsif ( !$slave1_dbh ) {
plan skip_all => 'Cannot connect to sandbox slave1';
}
else {
plan tests => 5;
}

my ($output, @rows);

# #############################################################################
# Test generated REPLACE statements.
# #############################################################################
$sb->load_file('master', "t/pt-table-sync/samples/pt-2378.sql");
$sb->wait_for_slaves();
$slave1_dbh->do("update `test`.`test_table` set `some_string` = 'c' where `id` = 1");

$output = remove_traces(output(
sub { pt_table_sync::main('--sync-to-master',
'h=127.0.0.1,P=12346,u=msandbox,p=msandbox',
qw(-t test.test_table --print --execute))
},
));
chomp($output);
is(
$output,
"REPLACE INTO `test`.`test_table`(`id`, `value1`, `value2`, `some_string`) VALUES ('1', 315.25999999999942, 2.6919444444444447, 'a');",
"Floating point numbers are generated with sufficient precision in REPLACE statements"
);

$sb->wait_for_slaves();
my $query = 'SELECT * FROM `test`.`test_table` WHERE `value1` = 315.2599999999994 AND `value2` = 2.6919444444444447';
@rows = $slave1_dbh->selectrow_array($query);
is_deeply(
\@rows,
[1, 315.2599999999994, 2.6919444444444447, 'a'],
'Floating point values are set correctly in round trip'
);

# #############################################################################
# Test generated UPDATE statements.
# #############################################################################
$sb->load_file('master', "t/pt-table-sync/samples/pt-2378.sql");
$sb->wait_for_slaves();
$slave1_dbh->do("update `test`.`test_table` set `some_string` = 'c' where `id` = 1");

$output = remove_traces(output(
sub { pt_table_sync::main(qw(--print --execute),
"h=127.0.0.1,P=12346,u=msandbox,p=msandbox,D=test,t=test_table",
"h=127.0.0.1,P=12345,u=msandbox,p=msandbox,D=test,t=test_table");
}
));
chomp($output);
is(
$output,
"UPDATE `test`.`test_table` SET `value1`=315.25999999999942, `value2`=2.6919444444444447, `some_string`='c' WHERE `id`='1' LIMIT 1;",
"Floating point numbers are generated with sufficient precision in UPDATE statements"
);

@rows = $master_dbh->selectrow_array($query);
is_deeply(
\@rows,
[1, 315.2599999999994, 2.6919444444444447, 'c'],
'Floating point values are set correctly in round trip'
);

# #############################################################################
# Done.
# #############################################################################
$sb->wipe_clean($master_dbh);
ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
exit;
15 changes: 15 additions & 0 deletions t/pt-table-sync/samples/pt-2378.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
DROP DATABASE IF EXISTS test;
CREATE DATABASE test;
USE test;

CREATE TABLE `test_table` (
`id` BIGINT AUTO_INCREMENT PRIMARY KEY,
`value1` DOUBLE NOT NULL,
`value2` DOUBLE NOT NULL,
`some_string` VARCHAR(32) NOT NULL
) ENGINE=InnoDB;

INSERT INTO `test_table`
(`value1`, `value2`, `some_string`)
VALUES
(315.2599999999994, 2.6919444444444447, 'a');

0 comments on commit c0f3c96

Please sign in to comment.