-
Notifications
You must be signed in to change notification settings - Fork 826
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: numeric binary decode for even 10 thousands #2327
Conversation
binary numeric values which represented integers multiples of 10,000 from 10,000-9,990,000 were not decoded correctly fixes pgjdbc#2326
more test values
@bokken thanks! Once this passes we can push it and release later today. |
Can this be tested with a wide set of different scales (not just 20)? |
Glad to figure it out. It is a bit of a strange corner case. This binary format for numeric is... interesting. I am working on additional tests which go through the database, to catch cases where the database's binary encoding might not match how the driver does the binary encoding. |
@jorsol we can test with whatever scale we want. It turns out it just matters that it is not 0. |
more test cases
Any other tests to consider? |
That does not appear to be related to any of these changes, right? |
No, it's not, AppVeyor probably needs an additional configuration for the scram test, probably we could just stick with GH Actions and drop appveyor near in the future. |
What I would like to see is some documentation on where this algorithm came from. But not before we release. |
@bokken I'm inclined to merge this and release. Any reason why I shouldn't ? |
@davecramer, I say go for it. |
@davecramer, are you wanting more than what is described here: |
Well this algorithm must have come from somewhere. Perhaps even adding comments in the code as to what some of the parts are doing ? It's pretty straightforward what scale and precision are. I don't think we need any more language about that. |
pgjdbc v42.3.1-rc1 is ready for preview. Git SHA: b3050e6 |
I tested with that version and the bug appears to be fixed. |
binary numeric values which represented integers multiples of 10,000
from 10,000-9,990,000 were not decoded correctly
fixes #2326
All Submissions:
New Feature Submissions:
./gradlew autostyleCheck checkstyleAll
pass ?Changes to Existing Features: