Comments (23)
Neat, didn't know about this!
big-integer
and others don't seem like true polyfills to me. Even if they use BigInt
internally if available, they don't return values that act like a BigInt
.
So we're in a bit of an odd position where it would be nice for node 10 users to have BigInts but we can't make that backwards compatible for <10. If you upgrade your app from node 8 to 10, do the types suddenly change? I don't think that's ok. That would mean adding BigInt support as opt in, e.g. types.bigInt()
auto-configures it. Happy to review a PR there.
from node-pg-types.
I have done some research in this area, as I am currently adding support of BigInt
within pg-promise, see this issue.
First of all, since BigInt
support was officially added in Node.js v10.4.0, it is simply too new to be breaking compatibility with so many clients out there. I'd say, we should give it another couple years at least, before considering such a change. I'm currently supporting Node.js v7.6.0 as the minimum, because it was the first version that made support for async/await
official, which is indeed a huge milestone, and a very important aspect when writing database client code.
And since this driver does not do its own query formatting, its support for BigInt
is already there.
For regular BigInt
support, you just do this:
pg.types.setTypeParser(20, BigInt); // Type Id 20 = BIGINT | BIGSERIAL
And also to get arrays of BigInt
natively, you add this:
// 1016 = Type Id for arrays of BigInt values
const parseBigIntArray = pg.types.getTypeParser(1016);
pg.types.setTypeParser(1016, a => parseBigIntArray(a).map(BigInt));
And that is it, the rest should just work as it is.
from node-pg-types.
while at the same time bringing in nothing new, because BigInt can be used already as it is.
The "something new" is that node-pg does what you expect out of the box.
from node-pg-types.
That would mean adding BigInt support as opt in, e.g.
types.bigInt()
auto-configures it.
I agree that this is the best path forward. That also makes the transition less painful if we eventually cut a new major version that dropped support for Node < 10, but that's probably a few years out.
big-integer
and others don't seem like true polyfills to me. Even if they useBigInt
internally if available, they don't return values that act like aBigInt
.
True, but if the alternative is to use strings on Node < 10, then that's even less like BigInt
. But I suppose we can leave it up to the client to decide if they want to opt in to a polyfill.
from node-pg-types.
B.T.W. I'm dropping pre-ES2017 support right now.
from node-pg-types.
PR welcome here. Based on the factors discussed here and in brianc/node-postgres#2398 this is a reasonable default.
I'm removing this from the 4.0 milestone because I don't have time to work on it right now and don't want to continue holding up other breaking change PRs that were submitted and merged.
from node-pg-types.
Even after reschedule EOL for Node 8, it ends this year. So, a major release with native BigInt
support is reasonable for me. Also AWS Lambda supports it. That's a sign that Node 8 is really old :D
@bendrucker @brianc What do you think about drawbacks with:
import { types } from 'pg'
types.setTypeParser(20, BigInt)
?
from node-pg-types.
pg currently supports node 4. I think it’s reasonable to track LTS but I’d like to do that by project policy rather than evaluating on a per-release basis. Please open an issue about that on the main pg repo and we can discuss and document accordingly.
from node-pg-types.
I try to keep pg supporting any LTS release which is still in its support window. Looks like I could drop support for node 4 and node 6 as they've both left the LTS window.
from node-pg-types.
Node 8 reaches end of life on December 31, 2019. At that point all active Nodejs releases will have BigInt.
from node-pg-types.
At that point all active Nodejs releases will have BigInt.
Node.js v10.4.0 onward, actually.
Also, as per comments above, enabling BitInt
support in the current driver is pretty much a no-effort. With that in view, I believe nothing needs to be done here.
from node-pg-types.
Node.js v10.4.0 onward, actually.
Yes, but I believe v10 went LTS at 10.13. I would be surprised if there were still an appreciable number of clients running an older version of v10 before the LTS. If there are, I sure hope they're getting patched!
Also, as per comments above, enabling BitInt support in the current driver is pretty much a no-effort. With that in view, I believe nothing needs to be done here.
It's fine if this issue is closed as "won't fix". But I think there is an opportunity for node-pg to change its defaults. If it were written from scratch today, I see no reason why it wouldn't default to using BigInt (ignoring the LTS debate for a moment).
from node-pg-types.
I would be surprised if there were still an appreciable number of clients running an older version of v10 before the LTS. If there are, I sure hope they're getting patched!
Lots of them are old, and won't be patched.
But I think there is an opportunity for node-pg to change its defaults...
Such "opportunity" would require major version change, as it would break huge number of clients out there, while at the same time bringing in nothing new, because BigInt
can be used already as it is.
from node-pg-types.
The "something new" is that node-pg does what you expect out of the box.
You would only expect it, if you do not know how 64-bit numbers are handled in JavaScript, and nuances of switching between 53-bit number
and 64-bit BigInt
. And since number
and BitInt
are not even directly compatible with each other (needs explicit conversion), it is fair to say, that expectation is wrong, not this library.
For example, you can multiply INT
by BIGINT
in PostgreSQL directly. You cannot do the same in JavaScript, you need to convert the type first. That brings in a lot of implications, and extra complications.
from node-pg-types.
Yeah I agree w/ @vitaly-t ... it's a huge change in a lot of subtle ways. What happens to all your queries which were doing JSON.stringify(results)
and passing them to the browser? What about browsers that don't support it? reference
There are other edge cases too like BigInt
behaving differently as keys in indexeddb, math between bigint and non-bigint numbers. I don't think we'll do this conversion by default for long time, but as @vitaly-t pointed out it's very easy to enable it. I also like @bendrucker's idea of making it an even easier "on switch" in pg types...but doing it by default introduces so many edge cases and footguns I don't imagine it will be a thing for at least a year or two.
from node-pg-types.
I think the great thing about it is that the changes aren’t subtle. JSON.stringify
will throw a very specific error, doing dangerous mixed math will throw a very specific error, and so on. Whereas right now…
-
anyone doing math on
count(*)
with the defaults will get the right result most of the time, but could accidentally concatenate or produce a different type after only minor changes -
overflow is always a problem, and is equally silent
BigInt
adds a layer of type safety, can be converted to a string or number for perfect compatibility with any existing operation, and is just as easy to opt out of through type overrides.
from node-pg-types.
What about browsers that don't support it?
Not sure I follow. As @charmander mentioned, you need to decide how you want to serialize BigInts anyway. If using JSON, that means they'll still typically be represented as strings.
Or does node-pg need to support running in browsers? I assumed from the name that it only ran in Node :)
from node-pg-types.
yeah it only runs in node (as far as I know! someone might have gone crazy w/ webpack hacks at some point?) but i just mean its common in apps to take query results and serialize them to JSON and there's weirdness w/ json and bigint. But yeah...I definitely have never nor do I plan on supporting running node-postgres in the browser - if that's something someone wants to do...more power to 'em. 😄
from node-pg-types.
So although I don’t understand how IndexedDB could be a problem if BigInt
s from pg can’t make it to the browser without explicit choices, the same concept does apply to Map
s. new Map([[1n, 2]]).has(1) === false
with no error, and generally 1n !== 1
with no error.
Still, waiting doesn’t make it less painful in the future. Maybe the next major version can make it opt-in with a single function call, with a deprecation warning if you don’t opt in explicitly opt in or opt out?
from node-pg-types.
Still, waiting doesn’t make it less painful in the future. Maybe the next major version can make it opt-in with a single function call, with a deprecation warning if you don’t
opt inexplicitly opt in or opt out?
I think that's a cool idea.
from node-pg-types.
polyfill like https://www.npmjs.com/package/big-integer
FWIW I think https://github.com/GoogleChromeLabs/jsbi is the superior choice for BigInt.
types.bigInt() auto-configures it
I agree that the best thing to do is make native bigint usage customizable -- which it already is, but build-in the option to the library.
JSON.stringify
The error is a new, but default JSON.parse/JSON.stringify could never round-trip all node-pg types anyway (e.g. Date).
from node-pg-types.
@pauldraper Actually, in case of BigInt
, round-tripping is easy, which is what I did within pg-promise
. First, see my earlier post above, where I show how easy it is to activate BigInt
conversion from server to the client. And then you can provide a custom JSON converter, like I do here. See also full feature page I wrote about it.
from node-pg-types.
@bendrucker is this available yet?
from node-pg-types.
Related Issues (20)
- Parser not working for bigint values HOT 3
- Can I customize type parsing per query? HOT 3
- please throw when oid is nullish HOT 1
- Reuse postgres-bytea HOT 6
- Releasing the latest changes HOT 4
- parsing postgres box type? HOT 2
- should handle null values HOT 1
- overriding a type parser HOT 1
- Does full text search with to_tsquery work ? HOT 1
- Is the Date parsing fix in the current pg release? HOT 1
- How to import pg-types using ES6 syntax? HOT 2
- Not able to use with modules HOT 1
- +num over parseInt(num, 10) HOT 1
- Changelog with breaking changes? HOT 1
- Add support for parsing RECORD types (OIDs 2249 and 2287) HOT 3
- Can this package be used in the browser? HOT 2
- Tracking down geometry oids HOT 3
- Readme. What number represents what type. HOT 2
- types.setTypeParser(types.builtins.DATE) does not work
- Numeric values are parsed as strings but arrays of numerics are parsed as number arrays HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from node-pg-types.