From 6cbe8279453f2fc512fb911e61118ecddd374bdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henning=20P=C3=B6ttker?= Date: Thu, 12 Sep 2024 14:21:15 +0200 Subject: [PATCH] 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 | 2 +- bin/pt-deadlock-logger | 2 +- bin/pt-duplicate-key-checker | 2 +- bin/pt-find | 2 +- bin/pt-fk-error-logger | 2 +- bin/pt-heartbeat | 2 +- bin/pt-index-usage | 2 +- bin/pt-kill | 2 +- bin/pt-online-schema-change | 2 +- bin/pt-query-digest | 2 +- bin/pt-slave-restart | 2 +- bin/pt-table-checksum | 2 +- bin/pt-table-sync | 2 +- bin/pt-upgrade | 2 +- lib/Quoter.pm | 2 +- t/lib/Quoter.t | 1 + t/pt-table-sync/float_precision.t | 19 +++++- t/pt-table-sync/pt-2378.t | 96 +++++++++++++++++++++++++++++ t/pt-table-sync/samples/pt-2378.sql | 15 +++++ 19 files changed, 145 insertions(+), 16 deletions(-) create mode 100644 t/pt-table-sync/pt-2378.t create mode 100644 t/pt-table-sync/samples/pt-2378.sql diff --git a/bin/pt-archiver b/bin/pt-archiver index 290ce0414..16b4a086c 100755 --- a/bin/pt-archiver +++ b/bin/pt-archiver @@ -2993,7 +2993,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'"; diff --git a/bin/pt-deadlock-logger b/bin/pt-deadlock-logger index 8b5a0d5b3..f6018fdbe 100755 --- a/bin/pt-deadlock-logger +++ b/bin/pt-deadlock-logger @@ -2088,7 +2088,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'"; diff --git a/bin/pt-duplicate-key-checker b/bin/pt-duplicate-key-checker index 1b9e924a9..1043e58d7 100755 --- a/bin/pt-duplicate-key-checker +++ b/bin/pt-duplicate-key-checker @@ -133,7 +133,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'"; diff --git a/bin/pt-find b/bin/pt-find index 1b1adbb42..6680cd318 100755 --- a/bin/pt-find +++ b/bin/pt-find @@ -1690,7 +1690,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'"; diff --git a/bin/pt-fk-error-logger b/bin/pt-fk-error-logger index ed489d9f3..33d7ef7e3 100755 --- a/bin/pt-fk-error-logger +++ b/bin/pt-fk-error-logger @@ -1242,7 +1242,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'"; diff --git a/bin/pt-heartbeat b/bin/pt-heartbeat index 342b86f20..269089a79 100755 --- a/bin/pt-heartbeat +++ b/bin/pt-heartbeat @@ -3492,7 +3492,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'"; diff --git a/bin/pt-index-usage b/bin/pt-index-usage index 1d3df0e6b..f02229d5a 100755 --- a/bin/pt-index-usage +++ b/bin/pt-index-usage @@ -589,7 +589,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'"; diff --git a/bin/pt-kill b/bin/pt-kill index af64bb796..f54a9a06e 100755 --- a/bin/pt-kill +++ b/bin/pt-kill @@ -4833,7 +4833,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'"; diff --git a/bin/pt-online-schema-change b/bin/pt-online-schema-change index d7d118d95..171799dc8 100755 --- a/bin/pt-online-schema-change +++ b/bin/pt-online-schema-change @@ -2866,7 +2866,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'"; diff --git a/bin/pt-query-digest b/bin/pt-query-digest index 9274c5e62..089d925bc 100755 --- a/bin/pt-query-digest +++ b/bin/pt-query-digest @@ -1257,7 +1257,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'"; diff --git a/bin/pt-slave-restart b/bin/pt-slave-restart index 861398a64..d48191e91 100755 --- a/bin/pt-slave-restart +++ b/bin/pt-slave-restart @@ -135,7 +135,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'"; diff --git a/bin/pt-table-checksum b/bin/pt-table-checksum index fe0e7e22a..0273ebfff 100755 --- a/bin/pt-table-checksum +++ b/bin/pt-table-checksum @@ -4134,7 +4134,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'"; diff --git a/bin/pt-table-sync b/bin/pt-table-sync index 83b7a6533..2d4162581 100755 --- a/bin/pt-table-sync +++ b/bin/pt-table-sync @@ -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'"; diff --git a/bin/pt-upgrade b/bin/pt-upgrade index bff9fb96a..8b8b0b52f 100755 --- a/bin/pt-upgrade +++ b/bin/pt-upgrade @@ -1254,7 +1254,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'"; diff --git a/lib/Quoter.pm b/lib/Quoter.pm index cbbc8409c..43d4f474c 100644 --- a/lib/Quoter.pm +++ b/lib/Quoter.pm @@ -78,7 +78,7 @@ 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}; + return sprintf("%.17g", $val) if $args{is_float}; # Quote and return non-numeric vals. $val =~ s/(['\\])/\\$1/g; diff --git a/t/lib/Quoter.t b/t/lib/Quoter.t index b07c10af6..4b25eb3f0 100644 --- a/t/lib/Quoter.t +++ b/t/lib/Quoter.t @@ -70,6 +70,7 @@ 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'); # Splitting DB and tbl apart is_deeply( diff --git a/t/pt-table-sync/float_precision.t b/t/pt-table-sync/float_precision.t index e49a3e018..f48ffabdb 100644 --- a/t/pt-table-sync/float_precision.t +++ b/t/pt-table-sync/float_precision.t @@ -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)' ); @@ -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 diff --git a/t/pt-table-sync/pt-2378.t b/t/pt-table-sync/pt-2378.t new file mode 100644 index 000000000..7c5eadfd8 --- /dev/null +++ b/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 $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; diff --git a/t/pt-table-sync/samples/pt-2378.sql b/t/pt-table-sync/samples/pt-2378.sql new file mode 100644 index 000000000..c4615a1a9 --- /dev/null +++ b/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');