-
Notifications
You must be signed in to change notification settings - Fork 65
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
Very large numeric results are getting rounded by the SQL API #113
Comments
The problem is that Javascript doesn't support numbers that large, so the only way to encode them in JSON as numbers is by giving up precision. The alternative is to threat them as strings. \cc @jmalczyk |
Note that threating results as strings were previously considered a regression, see #100 |
So in PostgresSQL we have the following numeric types:
Serial types, like "cartodb_id" is of the int8 type. I couldn't find any real-user case for the regression discussed in #100 so maybe it is fine for us to just switch to using strings for int8 and numeric types, shall we try ? I think such a change should warrant a minor version increment, so a 1.7.0. Upstream is still confused with conversions too: brianc/node-postgres#452 -- but we can override type parsers ourselves, as we did to fix #100 |
There's another discussion here about why always converting to string is annoying for some people: In particular, count(*) returns bigint, so it'll always return a string if we switched back ... To be honest, I'm not sure we want to change this. By keeping it as a number the user has the option to downcast or cast to text using SQL. If we forced it to be rendered a string the only user option to get a number would be a downcast to int4. What if we returned an error when a numeric rounding happens ? Would that help users make an explicit choice about how to threat the values ? The error message might recommend to cast to string or to another numerical type... |
I've documented the problem with numerical rounding in 0251714. For 1.7.0 I think this is enough. |
Ok, closing as won't fix. |
See CartoDB/cartodb#247
The text was updated successfully, but these errors were encountered: