Skip to content

Commit

Permalink
fix: backport sequelize#14485 in v6 branch to resolve sequelize#15411
Browse files Browse the repository at this point in the history
  • Loading branch information
Riccardo Marraghini committed Dec 7, 2022
1 parent 94beace commit 90927ff
Show file tree
Hide file tree
Showing 10 changed files with 118 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/data-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ class NUMBER extends ABSTRACT {
return true;
}
_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;
}
if (typeof number.toString === 'function') {
Expand Down
2 changes: 1 addition & 1 deletion src/dialects/abstract/query-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -2792,7 +2792,7 @@ class QueryGenerator {
type: options.type
});
}
if (typeof smth === 'number') {
if (typeof smth === 'number' || typeof smth === 'bigint') {
let primaryKeys = factory ? Object.keys(factory.primaryKeys) : [];

if (primaryKeys.length > 0) {
Expand Down
24 changes: 15 additions & 9 deletions src/dialects/db2/query.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const util = require('node:util');

const AbstractQuery = require('../abstract/query');
const sequelizeErrors = require('../../errors');
const parserStore = require('../parserStore')('db2');
Expand All @@ -14,15 +16,19 @@ class Query extends AbstractQuery {
}

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

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

return value;
}

async _run(connection, sql, parameters) {
async _run(connection, sql, parameters) {
this.sql = sql;
const benchmark = this.sequelize.options.benchmark || this.options.benchmark;
let queryBegin;
Expand Down Expand Up @@ -98,10 +104,10 @@ class Query 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 Expand Up @@ -454,7 +460,7 @@ class Query extends AbstractQuery {
result = result || this.sql.startsWith('SELECT NAME AS "name", TBNAME AS "tableName", UNIQUERULE AS "keyType", COLNAMES, INDEXTYPE AS "type" FROM SYSIBM.SYSINDEXES');
return result;
}

handleShowIndexesQuery(data) {
let currItem;
const result = [];
Expand All @@ -468,7 +474,7 @@ class Query extends AbstractQuery {
unique: item.keyType === 'U',
type: item.type
};

_.forEach(item.COLNAMES.replace(/\+|-/g, x => { return ` ${ x}`; }).split(' '), column => {
let columnName = column.trim();
if ( columnName ) {
Expand All @@ -484,7 +490,7 @@ class Query extends AbstractQuery {
result.push(currItem);
}
});
return result;
return result;
}

handleInsertQuery(results, metaData) {
Expand Down
13 changes: 11 additions & 2 deletions src/dialects/mssql/query.js
Original file line number Diff line number Diff line change
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;
let e = 1;
Expand All @@ -21,8 +24,7 @@ class Query 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 >= -2147483648 && value <= 2147483647) {
Expand All @@ -35,6 +37,13 @@ class Query 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
21 changes: 21 additions & 0 deletions src/dialects/sqlite/query.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ 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;
}

class Query extends AbstractQuery {
getInsertIdField() {
Expand Down Expand Up @@ -244,6 +253,18 @@ class Query extends AbstractQuery {
}

if (!parameters) parameters = [];

if (_.isPlainObject(parameters)) {
const newParameters = Object.create(null);

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

conn[method](sql, parameters, afterExecute);

return null;
Expand Down
2 changes: 1 addition & 1 deletion src/model.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1663,7 +1663,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
10 changes: 5 additions & 5 deletions src/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -1916,10 +1916,10 @@ class Model {
/**
* Search for a single instance by its primary key._
*
* @param {number|string|Buffer} param The value of the desired instance's primary key.
* @param {object} [options] find options
* @param {Transaction} [options.transaction] Transaction to run query under
* @param {string} [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only)
* @param {number|bigint|string|Buffer} param The value of the desired instance's primary key.
* @param {object} [options] find options
* @param {Transaction} [options.transaction] Transaction to run query under
* @param {string} [options.searchPath=DEFAULT] An optional parameter to specify the schema search_path (Postgres only)
*
* @see
* {@link Model.findAll} for a full explanation of options, Note that options.where is not supported.
Expand All @@ -1934,7 +1934,7 @@ 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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ function escape(val, timeZone, dialect, format) {
}
return (!!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
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,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
27 changes: 19 additions & 8 deletions test/unit/dialects/mssql/query.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,30 @@ if (dialect === 'mssql') {
describe('getSQLTypeFromJsType', () => {
const TYPES = tedious.TYPES;
it('should return correct parameter type', () => {
expect(query.getSQLTypeFromJsType(2147483647, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {} });
expect(query.getSQLTypeFromJsType(-2147483648, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2147483647, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: 2147483647 });
expect(query.getSQLTypeFromJsType(-2147483648, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: -2147483648 });

expect(query.getSQLTypeFromJsType(2147483648, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {} });
expect(query.getSQLTypeFromJsType(-2147483649, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2147483648, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: 2147483648 });
expect(query.getSQLTypeFromJsType(-2147483649, TYPES)).to.eql({ type: TYPES.BigInt, typeOptions: {}, value: -2147483649 });

expect(query.getSQLTypeFromJsType(Buffer.from('abc'), TYPES)).to.eql({ type: TYPES.VarBinary, typeOptions: {} });
expect(query.getSQLTypeFromJsType(2147483647n, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: 2147483647 });
expect(query.getSQLTypeFromJsType(-2147483648n, TYPES)).to.eql({ type: TYPES.Int, typeOptions: {}, value: -2147483648 });

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() });

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

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.30000000000000004, 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.30000000000000004, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 17 }, value: 0.30000000000000004 });
expect(query.getSQLTypeFromJsType(2.5e-15, TYPES)).to.eql({ type: TYPES.Numeric, typeOptions: { precision: 30, scale: 16 }, value: 2.5e-15 });
});
});

Expand Down

0 comments on commit 90927ff

Please sign in to comment.