-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
Fix zeros stripped in conversion from numeric #111
Conversation
Fixes a bug causing zeros to be stripped from numeric values such as 10234, producing 1234.
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.
Thanks for the PR! A test to ensure this new code works and to prevent regressions in the future would be great.
@@ -48,10 +48,10 @@ extension String: PostgreSQLDataConvertible { | |||
|
|||
/// depending on our offset, append the string to before or after the decimal point | |||
if offset < metadata.weight.bigEndian + 1 { | |||
integer += string | |||
integer += offset == 0 ? string : String(repeating: "0", count: 4 - string.count) + string |
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.
Breaking this up into one or two lines with some comments would make it a lot more readable I think
I can look into writing the test tomorrow (it’s late here). To be honest I don’t think a test should delay the merge of a critical data corruption bug fix. I could open a separate pull request.
… On 5 Nov 2018, at 21:18, Tanner ***@***.***> wrote:
I would love to include this change in 1.1.1. Let me know if you have any questions about writing the test. I'd also be happy to help write one.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub <#111 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACksh_-y7RCy5dYR30FVtpadQbEGgTdks5usJ0LgaJpZM4YL7M7>.
|
6bfd36d
to
fbb2238
Compare
fbb2238
to
1259ab4
Compare
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.
Thanks, @michal-tomlein. I really appreciate you taking the time to submit this patch and a thorough test case.
Hey @michal-tomlein, you just merged a pull request, have a coin! You now have 1 coins. |
Fixes a bug causing zeros to be stripped from numeric values such as 10234.543201, producing 1234.54321.