-
-
Notifications
You must be signed in to change notification settings - Fork 935
Improved precision encoding support for float32 and float64 #197
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
Conversation
|
(Moving discussion here) @MattNewberry wrote:
Shouldn't we be looking at the values for binary32 and binary64? So 8 and 16 digits for float32 and float64, respectively. |
|
I'll have to defer to your knowledge on that, but am happy to update the request accordingly. |
|
Given that Postgres is using 18 ( |
Oh please don't. I'm quite clueless about floats; I only know not to trust one when I see it. As evidenced just now: I believe I was reading the wrong table. Near the bottom (which I believe you were reading before) it says: So I think we still need to +1 both of these. Sorry about that. |
|
@johto Yeah, I think that's right. See this e-mail prompting the bump of |
Yeah, looks like the numbers postgres is using are 9 and 18. Maybe the 18 isn't really necessary, and the extra digits needs to be 3 just to make float4 work correctly? shrug I guess this would be where someone dug up the discussion where the project last changed extra_float_digits to see what their reasoning was.. |
Yeah, I had a vague recollection of that conversation taking place around that time. I think we have an agreement on 9 and 17 digits for float4 and float8, respectively? |
Yes. |
|
I'm happy to submit a new pull request with the final commit if needed to prevent needless overlap. |
|
@MattNewberry Nah, it's fine. I'll squash and commit. Thanks! |
|
Squashed, rebased, added test, and added you to the contributors list. Thanks! |
|
Good deal, thank you! |
Adjustment to sprintf format to better handle more precise float values, as noted in #196