wwwouter / typed-knex

A TypeScript wrapper for Knex.js
MIT License
112 stars 13 forks source link

getCount with postgresql returns string, not number #56

Closed jellelicht closed 2 years ago

jellelicht commented 2 years ago

Issue type:

[ ] Question [x] Bug report [ ] Feature request [ ] Documentation issue

Database system/driver:

[x] Postgres [ ] MSSQL [ ] MySQL [ ] MariaDB [ ] SQLite3 [ ] Oracle [ ] Amazon Redshift

typed-knex version:

[x] latest [ ] @next [ ] 0.x.x (or put your version here)

Knex.js version: 1.0.4

Steps to reproduce or a small repository showing the problem:

Calling getCount with latest typed-knex returns a Promise<string>, instead of the advertised Promise<number>. This seems to be a deliberate design choice by the folks who made node-postgres, as count can be (is?) a bigint.

I'm not sure what currently happens for other implementations, but it would be nice to have a workarounds if you can think of any. Thanks in advance!

wwwouter commented 2 years ago

node-postgres does this, because COUNT can return a number larger than a 32-bits integer. The projects I worked on, this has never been the case, so I'm using this in server.ts to automatically map the string to an integer:

pg.types.setTypeParser(20, 'text', parseInt);

Would this work for you as well, or do you need something else?

jellelicht commented 2 years ago

I can see that working in a subset of situations but wouldn't pg.types.setTypeParser(20, BigInt); be a more correct default mapping/assumption/recommendation for typed-knex to make instead?

Either way, it would be nice to document a short list of assumption that typed-knex makes about your configuration in order to be correct. Are there any more of these, or can I just open a PR to add this note to the README?

wwwouter commented 2 years ago

When I started this library, BigInt wasn't a thing yet 😄

I think it's a good idea to follow Knex.js: type it as number|string and give the possibility to override it.

I created a PR to explain this, can you check if this makes sense? https://github.com/wwwouter/typed-knex/pull/57

wwwouter commented 2 years ago

Landed in v4.10.0