Skip to content
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

improve support for int arrays and float arrays #501

Merged
merged 7 commits into from
Feb 26, 2014

Conversation

lalitkapoor
Copy link
Contributor

addresses #452

@brianc
Copy link
Owner

brianc commented Jan 8, 2014

Cool! Thanks! This actually changes the return type from big integers from their old, incorrect type of JavaScript ints into a new, correct type of JavaScript strings, right? Since int64 can technically overflow? I wanna make sure I bump the major version if it is indeed a non-backwards compatible change. I think there are a few more weirdnesses in #452 I'll try to address as well before I bump the major version component.

@lalitkapoor
Copy link
Contributor Author

Yes that is what it does. I agree too, it's a breaking change requiring a bump in the major. Just some more details:

If a user has been selecting int8 values within the number range that javascript supports they wouldn't have been experiencing any problems. Shifting parsing int8s over to strings as the default makes sense, but would be a breaking change for anyone selecting int8 values and working with them as numbers in their code.

Currently we parse int8 datatypes as javascript numbers. int8 can go up to (2^63)-1 (9223372036854775807) but javascript's numbers can only go up to 2^53 (9007199254740992) without issues. Any int8 value above what javascript supports may not be the correct value due to an overflow.

References
http://en.wikipedia.org/wiki/Integer_(computer_science)
http://stackoverflow.com/questions/307179/what-is-javascripts-max-int-whats-the-highest-integer-value-a-number-can-go-t

@brianc
Copy link
Owner

brianc commented Jan 25, 2014

Just as a note to myself - maybe I'll try to get a patch for this in before bumping the major version too?

#508

@brianc
Copy link
Owner

brianc commented Feb 26, 2014

@lalitkapoor thanks for this so much! 👍 sorry it took so long for me to merge it - needed to have some time & the mental fortitude to commit to doing a little refactoring in prep for [email protected].

❤️

brianc added a commit that referenced this pull request Feb 26, 2014
improve support for int arrays and float arrays
@brianc brianc merged commit ff8fb61 into brianc:master Feb 26, 2014
@lalitkapoor lalitkapoor deleted the GH-452 branch February 26, 2014 19:20
@brianc brianc added this to the v3.0 milestone Mar 12, 2014
@mariusa
Copy link

mariusa commented Jul 21, 2014

Hello,

What's the recommended way to handle bigint postgres type?

Right now pg.defaults.parseInt8 is inconsistent:

CREATE TABLE testbigints(
    x bigint,
    a bigint[] DEFAULT array[]::bigint[]
);

When pg.defaults.parseInt8 = true:

  • for bigint column types, typeof x == "number" (correct)
  • for bigint[] column types, the values resulting in nodejs still have typeof a[0] == "string" instead of "number".

Looks like both should be strings by default, and both should be numbers when pg.defaults.parseInt8 = true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants