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

BigInt returned as string #403

Closed
vikramkh opened this issue Dec 10, 2019 · 14 comments
Closed

BigInt returned as string #403

vikramkh opened this issue Dec 10, 2019 · 14 comments

Comments

@vikramkh
Copy link

Steps to reproduce

I have two models, Players and Team. Both ids are BigInts. Players has a foreign Key teamId which is of type BigInt in the database, and number in Postgres model properties.

Current Behavior

When I do .find() on players, teamId is returned as a string. If I do .execute, both id and teamId are returned as a string.

Expected Behavior

When I do .find(), or .execute, both id and teamId should be returned as numbers (or both as strings for consistencies sake)

Additional information

@vikramkh vikramkh added the bug label Dec 10, 2019
@QuentinLB
Copy link

I use loopback 4 and I have the same problem with numeric fields.

Here's an extract of my model :

@model()
export class MyModel extends Entity {
   @property({
        type: 'number',
        required: true,
        postgresql: {
            dataType: 'decimal'
       }
  })
  my_prop: Number;
}

I tried to trace a find() call but I could find the point where the conversion is done.

@achrinza
Copy link
Member

achrinza commented Dec 14, 2019

I suspect the following line may be the culprit:

https://github.com/strongloop/loopback-connector-postgresql/blob/e3c03e65abce4d5cb44f1e35b0af4b79c17a46d2/lib/postgresql.js#L240-L272

As per lb docs, fromColumnValue() is responsible for converting from the database format to JavaScript. However, I'm not too familiar on how the function works to be sure.

@QuentinLB
Copy link

So I did some digging, turns out :

By default the PostgreSQL backend server returns everything as strings.

To fix this issue, you need to provide a parser for the targeted type. In my case I did the following :

import {types} from 'pg'

 // 1700 : Numeric (https://github.com/brianc/node-pg-types/blob/master/index.d.ts#L42)
types.setTypeParser(1700, parseFloat)

@vikramkh For your BigInt case, the fix should be types.setTypeParser(20, BigInt). You must use node ^10.4.0, before this version Int were limited to 53 bits.

@stale
Copy link

stale bot commented Mar 13, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 13, 2020
@stale
Copy link

stale bot commented Mar 27, 2020

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Mar 27, 2020
@QuentinLB
Copy link

As this issue is being closed, maybe the use of pg setTypeParser should be documented either in this repo or more importantly on loopback documentation as it's often the entry point for people using the connector. Just a suggestion.

@emonddr
Copy link
Contributor

emonddr commented Mar 30, 2020

@QuentinLB , the solution you mention above seems to involve only node.js and the pg module. What about how it integrates with LB 3? If you could provide a minimal app that shows how you fully integrated your solution, it would help to document this properly. thx.

@QuentinLB
Copy link

Hi @emonddr

I have no idea how it integrate with LB3, I stumbled on this issue while working on a LB4 server. What I can describe was my assumption working on my server that use this connector. Once my models defined I was surprised to see my non-string models properties being returned as string (see my previous example).

As a way to avoid getting this type of issue open again I was suggesting to document the fact that neither LB nor this connector handle this behavior, allow the user facing this issue to setup their own setTypeParser.

Another way to manage this would be to setup those type parser in initializeDataSource but I'm not sure if it's the best place or the responsibility of the connector to do this.

Finally maybe it's the responsibility of node-pg-types to change this behavior since it was added at a time Node didn't have proper 64bit numeric types. There's an issue open on their repo addressing this brianc/node-pg-types#78

Hope that helps

@emonddr
Copy link
Contributor

emonddr commented Mar 30, 2020

@QuentinLB , sorry, I meant LB4.

@QuentinLB
Copy link

QuentinLB commented Mar 30, 2020

It's already documented 🤦‍♂️

My bad

@vikramkh
Copy link
Author

vikramkh commented Apr 26, 2020

@QuentinLB , where exactly does this code go?

import {types} from 'pg'

 // 1700 : Numeric (https://github.com/brianc/node-pg-types/blob/master/index.d.ts#L42)
types.setTypeParser(1700, parseFloat)

In a file in the node_modules or in each of the individual repositories? I'm really confused sorry

@QuentinLB
Copy link

@vikramkh

It doesn't matter where you setup your types parsers since it's a static interface. What matter is when you do this setup. If you use Loopback, I recommend to do it during the application initialization ; this way you ensure that your parser are setup before any connection to the database and before migrations.

@vikramkh
Copy link
Author

vikramkh commented Apr 26, 2020

@QuentinLB Thanks again for the help. After adding:

 types.setTypeParser(20, BigInt)

I get the error:

500 TypeError: Do not know how to serialize a BigInt
    at JSON.stringify (<anonymous>)
    at MySequence.writeResultToResponse [as send] (/Users/vikramkhemlani/Desktop/loopback/meal-plan-loopback/node_modules/@loopback/rest/dist/writer.js:51:31)
    at MySequence.handle (/Users/vikramkhemlani/Desktop/loopback/meal-plan-loopback/dist/sequence.js:54:18)
    at process._tickCallback (internal/process/next_tick.js:68:7)

I am using node v10.15.3

@QuentinLB
Copy link

You can't use Json.stringify to serialize BigInt (see the spec https://tc39.es/proposal-bigint/#sec-serializejsonproperty). One way to circumvent this is to provide your own toJson method :

BigInt.prototype.toJSON = function() { return this.toString()  }

Taken from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#Use_within_JSON

However by my understanding of your issue, trying to serialize your data seems convoluted. By default node-pg return already everything as string, so if you try to serialize it afterwards no need to set a type parser.

I think you should also look into the necessity to use BigInt, given the support for BigInt in js. If you use BigInt for an id, you should handle it as a string in you code since you won't use it as a number. Even better, use auto-generated uuids.

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

4 participants