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

Best effort support for JS BigInt in queries #14485

Merged
merged 17 commits into from
May 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/data-types.js
Expand Up @@ -240,7 +240,7 @@ class NUMBER extends ABSTRACT {
}

_stringify(number) {
if (typeof number === 'number' || typeof number === 'boolean' || number === null || number === undefined) {
if (typeof number === 'number' || typeof number === 'bigint' || typeof number === 'boolean' || number === null || number === undefined) {
return number;
}

Expand Down
2 changes: 1 addition & 1 deletion src/dialects/abstract/query-generator.js
Expand Up @@ -2832,7 +2832,7 @@ Only named replacements (:name) are allowed in literal() because we cannot guara
});
}

if (typeof smth === 'number') {
if (typeof smth === 'number' || typeof smth === 'bigint') {
let primaryKeys = factory ? Object.keys(factory.primaryKeys) : [];

if (primaryKeys.length > 0) {
Expand Down
13 changes: 8 additions & 5 deletions src/dialects/db2/query.js
@@ -1,6 +1,7 @@
'use strict';

import assert from 'node:assert';
import util from 'node:util';

const { AbstractQuery } = require('../abstract/query');
const sequelizeErrors = require('../../errors');
Expand All @@ -17,11 +18,13 @@ export class Db2Query extends AbstractQuery {
}

getSQLTypeFromJsType(value) {
const param = { ParamType: 'INPUT', Data: value };
if (Buffer.isBuffer(value)) {
param.DataType = 'BLOB';
return { ParamType: 'INPUT', DataType: 'BLOB', Data: value };
}

return param;
if (typeof value === 'bigint') {
// The ibm_db module does not handle bigint, send as a string instead:
return value.toString();
}

return value;
Expand Down Expand Up @@ -107,10 +110,10 @@ export class Db2Query extends AbstractQuery {
}

stmt.execute(params, (err, result, outparams) => {
debug(`executed(${this.connection.uuid || 'default'}):${newSql} ${parameters ? JSON.stringify(parameters) : ''}`);
debug(`executed(${this.connection.uuid || 'default'}):${newSql} ${parameters ? util.inspect(parameters, { compact: true, breakLength: Infinity }) : ''}`);

if (benchmark) {
this.sequelize.log(`Executed (${this.connection.uuid || 'default'}): ${newSql} ${parameters ? JSON.stringify(parameters) : ''}`, Date.now() - queryBegin, this.options);
this.sequelize.log(`Executed (${this.connection.uuid || 'default'}): ${newSql} ${parameters ? util.inspect(parameters, { compact: true, breakLength: Infinity }) : ''}`, Date.now() - queryBegin, this.options);
}

if (err && err.message) {
Expand Down
21 changes: 14 additions & 7 deletions src/dialects/mssql/query.js
Expand Up @@ -8,6 +8,9 @@ const { logger } = require('../../utils/logger');

const debug = logger.debugContext('sql:mssql');

const minSafeIntegerAsBigInt = BigInt(Number.MIN_SAFE_INTEGER);
const maxSafeIntegerAsBigInt = BigInt(Number.MAX_SAFE_INTEGER);

function getScale(aNum) {
if (!Number.isFinite(aNum)) {
return 0;
Expand All @@ -27,8 +30,7 @@ export class MsSqlQuery extends AbstractQuery {
}

getSQLTypeFromJsType(value, TYPES) {
const paramType = { type: TYPES.VarChar, typeOptions: {} };
paramType.type = TYPES.NVarChar;
const paramType = { type: TYPES.NVarChar, typeOptions: {}, value };
if (typeof value === 'number') {
if (Number.isInteger(value)) {
if (value >= -2_147_483_648 && value <= 2_147_483_647) {
Expand All @@ -41,6 +43,13 @@ export class MsSqlQuery extends AbstractQuery {
// Default to a reasonable numeric precision/scale pending more sophisticated logic
paramType.typeOptions = { precision: 30, scale: getScale(value) };
}
} else if (typeof value === 'bigint') {
if (value < minSafeIntegerAsBigInt || value > maxSafeIntegerAsBigInt) {
paramType.type = TYPES.VarChar;
paramType.value = value.toString();
} else {
return this.getSQLTypeFromJsType(Number(value), TYPES);
}
} else if (typeof value === 'boolean') {
paramType.type = TYPES.Bit;
}
Expand Down Expand Up @@ -91,15 +100,13 @@ export class MsSqlQuery extends AbstractQuery {
if (Array.isArray(parameters)) {
// eslint-disable-next-line unicorn/no-for-loop
for (let i = 0; i < parameters.length; i++) {
const parameter = parameters[i];

const paramType = this.getSQLTypeFromJsType(parameter, connection.lib.TYPES);
request.addParameter(String(i + 1), paramType.type, parameter, paramType.typeOptions);
const paramType = this.getSQLTypeFromJsType(parameters[i], connection.lib.TYPES);
request.addParameter(String(i + 1), paramType.type, paramType.value, paramType.typeOptions);
}
} else {
_.forOwn(parameters, (parameter, parameterName) => {
const paramType = this.getSQLTypeFromJsType(parameter, connection.lib.TYPES);
request.addParameter(parameterName, paramType.type, parameter, paramType.typeOptions);
request.addParameter(parameterName, paramType.type, paramType.value, paramType.typeOptions);
});
}

Expand Down
14 changes: 13 additions & 1 deletion src/dialects/sqlite/query.js
Expand Up @@ -12,6 +12,16 @@ const { logger } = require('../../utils/logger');

const debug = logger.debugContext('sql:sqlite');

// sqlite3 currently ignores bigint values, so we have to translate to string for now
// There's a WIP here: https://github.com/TryGhost/node-sqlite3/pull/1501
function stringifyIfBigint(value) {
if (typeof value === 'bigint') {
return value.toString();
}

return value;
}

export class SqliteQuery extends AbstractQuery {
getInsertIdField() {
return 'lastID';
Expand Down Expand Up @@ -249,10 +259,12 @@ export class SqliteQuery extends AbstractQuery {
const newParameters = Object.create(null);

for (const key of Object.keys(parameters)) {
newParameters[`$${key}`] = parameters[key];
newParameters[`$${key}`] = stringifyIfBigint(parameters[key]);
}

parameters = newParameters;
} else {
parameters = parameters.map(stringifyIfBigint);
}

conn[method](sql, parameters, afterExecute);
Expand Down
2 changes: 1 addition & 1 deletion src/model.d.ts
Expand Up @@ -1774,7 +1774,7 @@ export type ModelAttributes<M extends Model = Model, TAttributes = any> = {
/**
* Possible types for primary keys
*/
export type Identifier = number | string | Buffer;
export type Identifier = number | bigint | string | Buffer;

/**
* Options for model definition.
Expand Down
6 changes: 3 additions & 3 deletions src/model.js
Expand Up @@ -1924,8 +1924,8 @@ export class Model {
* Returns the model with the matching primary key.
* If not found, returns null or throws an error if {@link FindOptions.rejectOnEmpty} is set.
*
* @param {number|string|Buffer} param The value of the desired instance's primary key.
* @param {object} [options] find options
* @param {number|bigint|string|Buffer} param The value of the desired instance's primary key.
* @param {object} [options] find options
* @returns {Promise<Model|null>}
*/
static async findByPk(param, options) {
Expand All @@ -1936,7 +1936,7 @@ export class Model {

options = Utils.cloneDeep(options) || {};

if (typeof param === 'number' || typeof param === 'string' || Buffer.isBuffer(param)) {
if (typeof param === 'number' || typeof param === 'bigint' || typeof param === 'string' || Buffer.isBuffer(param)) {
options.where = {
[this.primaryKeyAttribute]: param,
};
Expand Down
1 change: 1 addition & 0 deletions src/sql-string.js
Expand Up @@ -42,6 +42,7 @@ export function escape(val, timeZone, dialect, format) {

return (Boolean(val)).toString();
case 'number':
case 'bigint':
return val.toString();
case 'string':
// In mssql, prepend N to all quoted vals which are originally a string (for
Expand Down
43 changes: 43 additions & 0 deletions test/integration/model/findOne.test.js
Expand Up @@ -225,6 +225,49 @@ describe(Support.getTestDialectTeaser('Model'), () => {
expect(u2.name).to.equal('Johnno');
});

it('finds entries via a bigint primary key called id', async function () {
const UserPrimary = this.sequelize.define('UserWithPrimaryKey', {
id: { type: DataTypes.BIGINT, primaryKey: true },
name: DataTypes.STRING,
});

await UserPrimary.sync({ force: true });

await UserPrimary.create({
id: 9_007_199_254_740_993n, // Number.MAX_SAFE_INTEGER + 2 (cannot be represented exactly as a number in JS)
name: 'Johnno',
});

const u2 = await UserPrimary.findByPk(9_007_199_254_740_993n);
expect(u2.name).to.equal('Johnno');

// Getting the value back as bigint is not supported yet: https://github.com/sequelize/sequelize/issues/14296
// With most dialects we'll receive a string, but in some cases we have to be a bit creative to prove that we did get hold of the right record:
if (dialect === 'db2') {
// ibm_db 2.7.4+ returns BIGINT values as JS numbers, which leads to a loss of precision:
// https://github.com/ibmdb/node-ibm_db/issues/816
// It means that u2.id comes back as 9_007_199_254_740_992 here :(
// Hopefully this will be fixed soon.
// For now we can do a separate query where we stringify the value to prove that it did get stored correctly:
const [[{ stringifiedId }]] = await this.sequelize.query(`select "id"::varchar as "stringifiedId" from "${UserPrimary.tableName}" where "id" = 9007199254740993`);
expect(stringifiedId).to.equal('9007199254740993');
} else if (dialect === 'mariadb') {
// With our current default config, the mariadb driver sends back a Long instance.
// Updating the mariadb dev dep and passing "supportBigInt: true" would get it back as a bigint,
// but that's potentially a big change.
// For now, we'll just stringify the Long and make the comparison:
expect(u2.id.toString()).to.equal('9007199254740993');
} else if (dialect === 'sqlite') {
// sqlite3 returns a number, so u2.id comes back as 9_007_199_254_740_992 here:
// https://github.com/TryGhost/node-sqlite3/issues/922
// For now we can do a separate query where we stringify the value to prove that it did get stored correctly:
const [[{ stringifiedId }]] = await this.sequelize.query(`select cast("id" as text) as "stringifiedId" from "${UserPrimary.tableName}" where "id" = 9007199254740993`);
expect(stringifiedId).to.equal('9007199254740993');
} else {
expect(u2.id).to.equal('9007199254740993');
}
});

it('always honors ZERO as primary key', async function () {
const permutations = [
0,
Expand Down
6 changes: 3 additions & 3 deletions test/integration/sequelize/query.test.js
Expand Up @@ -182,8 +182,8 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {
expect(updateSql).to.match(/; "li", 1$/);
} else if (dialect === 'db2') {
// TODO: db2 should be unified with the other positional parameter dialects
expect(createSql).to.match(/; \["john","john@gmail.com"]$/);
expect(updateSql).to.match(/; \["li",1]$/);
expect(createSql).to.match(/; \[ 'john', 'john@gmail.com' ]$/);
expect(updateSql).to.match(/; \[ 'li', 1 ]$/);
} else {
expect(createSql).to.match(/; \{"sequelize_1":"john","sequelize_2":"john@gmail.com"}$/);
expect(updateSql).to.match(/; \{"sequelize_1":"li","sequelize_2":1}$/);
Expand All @@ -207,7 +207,7 @@ describe(Support.getTestDialectTeaser('Sequelize'), () => {

if (dialect === 'db2') {
// TODO: db2 should be unified with the other positional parameter dialects
expect(logSql).to.match(/; \["foo","bar"]$/);
expect(logSql).to.match(/; \[ 'foo', 'bar' ]$/);
} else {
expect(logSql).to.match(/; ("foo", "bar"|{"(\$1|0)":"foo","(\$2|1)":"bar"})/);
}
Expand Down
2 changes: 1 addition & 1 deletion test/registerEsbuild.js
Expand Up @@ -29,7 +29,7 @@ function compileFor(loader) {
return (source, sourcefile) => {
const { code, map } = esbuild.transformSync(source, {
sourcemap: true,
target: 'node10',
target: 'node14',
format: 'cjs',
sourcefile,
loader,
Expand Down
28 changes: 20 additions & 8 deletions test/unit/dialects/mssql/query.test.js
Expand Up @@ -44,19 +44,31 @@ if (dialect === 'mssql') {
describe('getSQLTypeFromJsType', () => {
const TYPES = tedious.TYPES;
it('should return correct parameter type', () => {
expect(query.getSQLTypeFromJsType(2_147_483_647, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {} });
expect(query.getSQLTypeFromJsType(-2_147_483_648, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2_147_483_647, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: 2_147_483_647 });
expect(query.getSQLTypeFromJsType(-2_147_483_648, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: -2_147_483_648 });

expect(query.getSQLTypeFromJsType(2_147_483_648, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {} });
expect(query.getSQLTypeFromJsType(-2_147_483_649, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2_147_483_648, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: 2_147_483_648 });
expect(query.getSQLTypeFromJsType(-2_147_483_649, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: -2_147_483_649 });

expect(query.getSQLTypeFromJsType(Buffer.from('abc'), TYPES)).to.eql({ type: TYPES.VarBinary, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2_147_483_647n, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: 2_147_483_647 });
expect(query.getSQLTypeFromJsType(-2_147_483_648n, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: -2_147_483_648 });

expect(query.getSQLTypeFromJsType(BigInt(Number.MAX_SAFE_INTEGER), TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: Number.MAX_SAFE_INTEGER });
expect(query.getSQLTypeFromJsType(BigInt(Number.MIN_SAFE_INTEGER), TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: Number.MIN_SAFE_INTEGER });

const overMaxSafe = BigInt(Number.MAX_SAFE_INTEGER) + 1n;
expect(query.getSQLTypeFromJsType(overMaxSafe, TYPES)).to.eql({ type: TYPES.VarChar, typeOptions: {}, value: overMaxSafe.toString() });
const underMinSafe = BigInt(Number.MIN_SAFE_INTEGER) - 1n;
expect(query.getSQLTypeFromJsType(underMinSafe, TYPES)).to.eql({ type: TYPES.VarChar, typeOptions: {}, value: underMinSafe.toString() });

const buffer = Buffer.from('abc');
expect(query.getSQLTypeFromJsType(buffer, TYPES)).to.eql({ type: TYPES.VarBinary, typeOptions: {}, value: buffer });
});

it('should return parameter type correct scale for float', () => {
expect(query.getSQLTypeFromJsType(1.23, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 2 } });
expect(query.getSQLTypeFromJsType(0.300_000_000_000_000_04, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 17 } });
expect(query.getSQLTypeFromJsType(2.5e-15, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 16 } });
expect(query.getSQLTypeFromJsType(1.23, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 2 }, value: 1.23 });
expect(query.getSQLTypeFromJsType(0.300_000_000_000_000_04, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 17 }, value: 0.300_000_000_000_000_04 });
expect(query.getSQLTypeFromJsType(2.5e-15, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 16 }, value: 2.5e-15 });
});
});

Expand Down
29 changes: 28 additions & 1 deletion test/unit/query-generator/select-query.test.ts
Expand Up @@ -14,7 +14,13 @@ describe('QueryGenerator#selectQuery', () => {
username: DataTypes.STRING,
}, { timestamps: false });

const Project = sequelize.define('Project', {}, { timestamps: false });
interface TProject extends Model<InferAttributes<TProject>> {
duration: bigint;
}

const Project = sequelize.define<TProject>('Project', {
duration: DataTypes.BIGINT,
}, { timestamps: false });

const ProjectContributor = sequelize.define('ProjectContributor', {}, { timestamps: false });

Expand Down Expand Up @@ -47,6 +53,27 @@ describe('QueryGenerator#selectQuery', () => {
});
});

it('supports querying for bigint values', () => {
const sql = queryGenerator.selectQuery(Project.tableName, {
model: Project,
attributes: ['id'],
where: {
duration: { [Op.eq]: 9_007_199_254_740_993n },
},
}, Project);

expectsql(sql, {
postgres: `SELECT "id" FROM "Projects" AS "Project" WHERE "Project"."duration" = 9007199254740993;`,
mysql: 'SELECT `id` FROM `Projects` AS `Project` WHERE `Project`.`duration` = 9007199254740993;',
mariadb: 'SELECT `id` FROM `Projects` AS `Project` WHERE `Project`.`duration` = 9007199254740993;',
sqlite: 'SELECT `id` FROM `Projects` AS `Project` WHERE `Project`.`duration` = 9007199254740993;',
snowflake: 'SELECT "id" FROM "Projects" AS "Project" WHERE "Project"."duration" = 9007199254740993;',
db2: `SELECT "id" FROM "Projects" AS "Project" WHERE "Project"."duration" = 9007199254740993;`,
ibmi: `SELECT "id" FROM "Projects" AS "Project" WHERE "Project"."duration" = '9007199254740993'`,
mssql: `SELECT [id] FROM [Projects] AS [Project] WHERE [Project].[duration] = 9007199254740993;`,
});
});

describe('replacements', () => {
it('parses named replacements in literals', async () => {
// The goal of this test is to test that :replacements are parsed in literals in as many places as possible
Expand Down