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.

bin/pt-archiver

@@ -2993,7 +2993,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-deadlock-logger

@@ -2088,7 +2088,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-duplicate-key-checker

@@ -133,7 +133,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-find

@@ -1690,7 +1690,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-fk-error-logger

@@ -1242,7 +1242,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-heartbeat

@@ -3564,7 +3564,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-index-usage

@@ -589,7 +589,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-kill

@@ -4905,7 +4905,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-online-schema-change

@@ -2866,7 +2866,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-query-digest

@@ -1257,7 +1257,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-replica-restart

@@ -135,7 +135,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-table-checksum

@@ -4134,7 +4134,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-table-sync

@@ -1909,7 +1909,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-table-usage

@@ -6680,7 +6680,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

bin/pt-upgrade

@@ -1254,7 +1254,10 @@ 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};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    $val =~ s/(['\\])/\\$1/g;
    return "'$val'";

lib/Quoter.pm

@@ -78,7 +78,10 @@ sub quote_val {
                   && !$args{is_char};          # unless is_char is true
 
    # https://bugs.launchpad.net/percona-toolkit/+bug/1229861
-   return $val if $args{is_float};
+   if ( $args{is_float} ) {
+      return sprintf("%.17g", $val) if $val - "$val" != 0;
+      return $val;
+   }
 
    # Quote and return non-numeric vals.
    $val =~ s/(['\\])/\\$1/g;

t/lib/Quoter.t

@@ -71,6 +71,8 @@ is( $q->quote_val('0x89504E470', is_char => 0), '0x89504E470', 'hex string, with
 is( $q->quote_val('0x89504E470', is_char => 1), "'0x89504E470'", 'hex string, with is_char => 1');
 is( $q->quote_val('0x89504I470'), "'0x89504I470'", 'looks like hex string');
 is( $q->quote_val('eastside0x3'), "'eastside0x3'", 'looks like hex str (issue 1110');
+is( $q->quote_val(969.1 / 360, is_float => 1), "2.6919444444444447", 'float has full precision');
+is( $q->quote_val(0.1, is_float => 1), "0.1", 'full precision for float only used when required');
 
 # Splitting DB and tbl apart
 is_deeply(

t/pt-table-sync/float_precision.t

@@ -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-source 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_replicas();
+my @rows = $replica_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
@@ -86,7 +103,6 @@ is_deeply(
    [],
    "Sync rows with float values (bug 1229861)"
 ) or diag(Dumper($rows), $output);
-
 # #############################################################################
 # Done.
 # #############################################################################

t/pt-table-sync/pt-2378.t

@@ -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 $source_dbh = $sb->get_dbh_for('source');
+my $replica1_dbh = $sb->get_dbh_for('replica1');
+
+if ( !$source_dbh ) {
+   plan skip_all => 'Cannot connect to sandbox source';
+}
+elsif ( !$replica1_dbh ) {
+   plan skip_all => 'Cannot connect to sandbox replica1';
+}
+else {
+   plan tests => 5;
+}
+
+my ($output, @rows);
+
+# #############################################################################
+# Test generated REPLACE statements.
+# #############################################################################
+$sb->load_file('source', "t/pt-table-sync/samples/pt-2378.sql");
+$sb->wait_for_replicas();
+$replica1_dbh->do("update `test`.`test_table` set `some_string` = 'c' where `id` = 1");
+
+$output = remove_traces(output(
+   sub { pt_table_sync::main('--sync-to-source',
+      '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_replicas();
+my $query = 'SELECT * FROM `test`.`test_table` WHERE `value1` = 315.2599999999994 AND `value2` = 2.6919444444444447';
+@rows = $replica1_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('source', "t/pt-table-sync/samples/pt-2378.sql");
+$sb->wait_for_replicas();
+$replica1_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 = $source_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($source_dbh);
+ok($sb->ok(), "Sandbox servers") or BAIL_OUT(__FILE__ . " broke the sandbox");
+exit;

t/pt-table-sync/samples/pt-2378.sql

@@ -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');