-
Notifications
You must be signed in to change notification settings - Fork 329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PT-2378 - extended FP precision in pt-table-sync #863
base: 3.x
Are you sure you want to change the base?
PT-2378 - extended FP precision in pt-table-sync #863
Conversation
c0f3c96
to
687f24b
Compare
I updated the PR such that the change is also included in This affected the source code of many tools. However, the only tool except for |
687f24b
to
6cbe827
Compare
6cbe827
to
ad350a2
Compare
Thanks so much for the massive effort to adjust the code base to MySQL 8.4! I've rebased the changes on the latest commit in 3.x. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proposed change caused failure of t/pt-table-sync/float_precision.t due to extra digits, added to value 31.6: it was formatted as 31.600000000000001 As a result, test "Sync rows with float values (bug 1229861)" failed.
I suggest making the fix more complicated. Please check my latest commit and let me know if you agree or not.
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.
…ements with insufficient precision - Modified proposed patch, so it does not add extra digits
use serialization error as criterion for additional precision
37b9e2c
to
634ed46
Compare
I completely agree on the problem. With the extended precision, also the number I don't think it can't be decided purely on the length of the string representation of a floating point number whether the full precision is required. This would be a counter example for testing against a length of 15:
I've changed the criterion to be a non-zero serialization error with the default format, which should work more broadly and captures the intent. And I've rebased on the latest commit of |
The PR is intended to resolve this issue: https://perconadev.atlassian.net/browse/PT-2378
The tests added with
pt-2378.t
all fail with the current code base. With the proposed change, they run successfully. They test that bothREPLACE
andUPDATE
statements are generated bypt-table-sync
such that floating point numbers that are not intended to change indeed don't change.The code is my own creation and it can be distributed under the GPL2 licence.