-
Notifications
You must be signed in to change notification settings - Fork 714
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
Convert bigints that exceed js number limits to strings #109
Conversation
Convert bigints that exceed js number limits to strings
Thanks so much! Any chance you'll release a 0.7.1 soon? |
I'll release 0.7.1 in a day or so, working on a few other things that should go in too. |
val := col.(int64) | ||
if val < -9007199254740991 || val > 9007199254740991 { | ||
res.Rows[i][j] = strconv.FormatInt(col.(int64), 10) | ||
} |
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.
For what its worth, I don't really agree with dynamically changing the shape of the json object based on the values.
For example, consider this code:
while (true) {
var result = JSON.parse(getValueFromApi());
var oneHigher = result.usuallyNumber + 1;
saveToDb({usuallyNumber: oneHigher});
}
If result.usuallyNumber
is all of a sudden a string (because your values got too big), the +
operator in javascript ends up becoming string concatenation.
1
2
3
...
9007199254740989
9007199254740990
9007199254740991
90071992547409911
900719925474099111
9007199254740991111
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.
Pgweb is not considered to be used as a "real" api. So its api is exposed solely to be consumed by the frontend application.
All that said, I'm not going to complain about free software :) Thanks for you're awesome contribution to the OSS community. |
FYI 0.8.0 is published to homebrew cask. |
Awesome! Muchas gracias, could you push to dockerhub too when you have a chance? |
Done |
Should fix #108