sequelize / sequelize-auto

Automatically generate bare sequelize models from your database.
2.88k stars 522 forks source link

Postgres BIGINT incorrectly mapped to number type #616

Open aawalton opened 1 year ago

aawalton commented 1 year ago

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch sequelize-auto@0.8.6 for a project I'm working on.

The TypeScript types generated for the Postgres BIGINT DataType are incorrect. Since Postgres BIGINT values are too large to fit into JavaScript integers, Sequelize maps them to JavaScript strings to avoid loss of resolution. With the incorrect TypeScript types, this resulted in some tricky errors for us, where TypeScript missed incorrect math operations on the BIGINT string ('10' + 10 = '1010'), or incorrect comparisons ('5' === 5 -> false).

In Postgres, BIGINT values are often used for numbers that could grow larger than JavaScript integers, and in particular, for auto-incrementing primary keys.

Here is the diff that solved my problem:

diff --git a/node_modules/sequelize-auto/lib/auto-generator.js b/node_modules/sequelize-auto/lib/auto-generator.js
index 8adca17..5646fa8 100644
--- a/node_modules/sequelize-auto/lib/auto-generator.js
+++ b/node_modules/sequelize-auto/lib/auto-generator.js
@@ -773,7 +773,7 @@ class AutoGenerator {
         return (/^[$A-Z_][0-9A-Z_$]*$/i.test(name) ? name : "'" + name + "'");
     }
     isNumber(fieldType) {
-        return /^(smallint|mediumint|tinyint|int|bigint|float|money|smallmoney|double|decimal|numeric|real|oid)/.test(fieldType);
+        return /^(smallint|mediumint|tinyint|int|float|money|smallmoney|double|decimal|numeric|real|oid)/.test(fieldType);
     }
     isBoolean(fieldType) {
         return /^(boolean|bit)/.test(fieldType);
@@ -782,7 +782,7 @@ class AutoGenerator {
         return /^(datetime|timestamp)/.test(fieldType);
     }
     isString(fieldType) {
-        return /^(char|nchar|string|varying|varchar|nvarchar|text|longtext|mediumtext|tinytext|ntext|uuid|uniqueidentifier|date|time|inet|cidr|macaddr)/.test(fieldType);
+        return /^(char|nchar|string|varying|varchar|nvarchar|text|longtext|mediumtext|tinytext|ntext|bigint|uuid|uniqueidentifier|date|time|inet|cidr|macaddr)/.test(fieldType);
     }
     isArray(fieldType) {
         return /(^array)|(range$)/.test(fieldType);

This issue body was partially generated by patch-package.

linxux commented 6 months ago

Agree. The bigint type should be represented as String.