default number type parsing

[rpedela]

I would like to reopen the discussion because I strongly disagree with the current implementation.

Relevant issues: #107, #266, #271, #296, #301

By default, I think any PG type which maps directly to a Javascript type should be converted to that type. If there isn't a direct mapping, then simply returning a string is fine. Of course, the user can override the type parser if they have different needs. This is how node-mysql handles type parsing which I believe to be correct.

What is the problem? Right now all decimal numbers are returned as strings, and 64-bit integers are parsed as integers. Some of the former have a direct mapping to a Javascript type, and the latter loses precision.

The Postgres docs specify that real and double precision are "implementations of IEEE Standard 754 for Binary Floating-Point Arithmetic (single and double precision, respectively)". Javascript only has one number type: IEEE Standard 754 double precision. All Javascript numbers whether they are integers or decimals are converted to IEEE Standard 754 double precision.

This means that parseFloat() can safely be used for real and double precision. Using strings is confusing to the user since it is not expected and requires them to do more work by overriding the type parser without any real benefit.

Javascript number representation causes a problem for numeric/decimal, bigint, bigserial since there could be a loss of precision. This is a case where a string is appropriate. All other PG number types can be represented properly with a Javascript number without a loss of precision.

Below shows what I think to be the correct, default parsing.

smallint:         parseInt()
integer:          parseInt()
bigint:           string
decimal:          string
numeric:          string
real:             parseFloat()
double precision: parseFloat()
smallserial:      parseInt()
serial:           parseInt()
bigserial:        string

[brianc]

Your post is well thought out, well researched, and I appreciate the time you put into it. I removed parseFloat due to some people loudly complaining it was causing data loss. There were discussions, and I did what I thought the majority was in favor of. To be honest I dropped https://github.com/brianc/node-pg-parse-float into my app as soon as I upgraded to node-postgres 1.0, but I don't do anything with large numbers.

I think the best way to move forward would be to build a test suite that demonstrates all of these types being parsed properly per your suggestions and also demonstrates the edge cases where parsing as another type (say parsing a decimal with parseFloat() or a bigint with parseInt()) fails. We could do this in another module called pg-numerics or something. Once the type parsing has been demonstrated correct with tests this module can be used in the spirit of the pg-parse-float module to bring node-postgres v1.0 in line with what we agrees is the correct way to parse numbers of various types. We can include documentation around this issue citing the legacy "out of the box" behavior. Once it gets some adoption and the tumult subsides, we can bump node-postgres to v2.0 and include pg-numerics as a dependency or just throw those type parsers into the code base as default.

I'm not sure everyone will agree on any way to do it, and like you mentioned the type parsers are exposed for the user, but I would definitely like to remove as many "wtfs" as possible from a standard node-postgres install. What do you think?

[rpedela]

Based on the issues I referenced, most of the "complaining" is specific to numeric/decimal. I would hope developers understand that IEEE 754 floating-point arithmetic should not be used for financial or scientific applications since you will lose precision from rounding. That is the reason why numeric/decimal exists because it exactly represents any number. Unfortunately Javascript cannot handle these so using a string makes sense.

Regardless, I can create a pg-numerics if you want. I will likely add an option to use classes instead of strings so you can do the following which I mentioned in #266.

if (num instanceof PgBigInt) {
    // use favorite big integer library
}

[brianc]

I really appreciate the time you put into this and the outcome. Thank you a million.

[rpedela]

No problemo. :)