Skip to content

BigInt errors inside prepared statements #2395

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

Closed
vitaly-t opened this issue Oct 28, 2020 · 8 comments
Closed

BigInt errors inside prepared statements #2395

vitaly-t opened this issue Oct 28, 2020 · 8 comments

Comments

@vitaly-t
Copy link
Contributor

vitaly-t commented Oct 28, 2020

Why use of BigInt in Prepared Statements throws an error inside this library's formatter?

const a = {d: 1n};
await pool.query({text: 'SELECT $1 as value', values: [a]});

throws library-level error (not server-level):

(node:65560) UnhandledPromiseRejectionWarning: TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at prepareObject (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/utils.js:80:15)
    at prepareValue (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/utils.js:65:12)
    at prepareValueWrapper (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/utils.js:181:12)
    at Array.map (<anonymous>)
    at Query.prepare (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/query.js:202:35)
    at Query.submit (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/query.js:155:12)
    at Client._pulseQueryQueue (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/client.js:472:45)
    at Client.query (/Users/~/Documents/Git/Personal/pg-queue/node_modules/pg/lib/client.js:567:10)

Note that the error is thrown while preparing the query for execution, and not on the returned result.

@sehrope
Copy link
Contributor

sehrope commented Oct 28, 2020

That's not specific to this driver. It's the default handling for JSON.stringify(...) for BigInt datatypes:

> JSON.stringify({ a: 1n })
Uncaught TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)

You'll need to stringify it yourself or add system support for it it via a polyfill.

I think it's not part of the JSON.stringify(...) by default because JSON.parse(...) would not know if it should use a BigInt or a regular Number. So rather than have it be ambiguous, the defaults will remain and it's up to you to handle it locally if you want a different approach.

@vitaly-t
Copy link
Contributor Author

vitaly-t commented Oct 28, 2020

I think this library should offer a default serialization for BigInt, or there will be too many people running into this issue.

example:

if(typeof BigInt.prototype.toJSON !== 'function') {
    // no global BigInt serialization available,
    // apply default serialization to BigInt-s,
    // to avoid throwing errors

    // use custom JSON.stringify
}

@sehrope
Copy link
Contributor

sehrope commented Oct 28, 2020

I think that's a bad idea as if you round trip an object (i.e. INSERT then SELECT it back), you'll won't get a BigInt. The JSON.parse(...) would give you a regular Number for the property.

@vitaly-t
Copy link
Contributor Author

@sehrope De-serialization is a separate issue altogether, which works via custom parsers. This issue is strictly about serialization.

@vitaly-t
Copy link
Contributor Author

I have been using custom serializer inside pg-promise for a while, and no issues. But when someone uses a prepared statement, the values are passed on, and then this library fails.

@sehrope
Copy link
Contributor

sehrope commented Oct 28, 2020

You have to explicitly add support for deserialization, so it makes sense to require explicit support for serialization. Having it work automatically in only one direction would lead to confusion and any errors may well be hidden in edge cases.

Again, I don't see it as a failure of this library as nothing that handles JSON stringification works out of the box with BigInts. For example if you use express and return an object via req.send({ myBigInt: 123n }) then you'll get the same error.

@vitaly-t
Copy link
Contributor Author

You have to explicitly add support for deserialization, so it makes sense to require explicit support for serialization

No, it doesn't, because serialization doesn't bring any ambiguity, it produces just one format which Postgres understands, and that's it, while de-serialization allows for multiple interpretations of the incoming data, depending on precision requirements, etc. This is why de-serialization is where it belongs, so you can control BigInt precision by yourself. For serialization you do not need that, things are way simpler there. That's why I think this library should just implement that.

@charmander
Copy link
Collaborator

Custom serializers are slow. People putting BigInts in JSON parameters can handle this themselves, I think.

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

No branches or pull requests

3 participants