Merge pull request #1445 from sandorfr/bugfix/mssql-bigint

add bigint detection and sets mssql req.input when appropriate
This commit is contained in:
Mikael Lepistö 2016-05-30 00:17:38 +03:00
commit 05a95c2b8b
3 changed files with 93 additions and 2 deletions

View File

@ -17,6 +17,9 @@ import ColumnCompiler from './schema/columncompiler';
const { isArray } = Array;
const SQL_INT4 = { MIN : -2147483648, MAX: 2147483647}
const SQL_BIGINT_SAFE = { MIN : -9007199254740991, MAX: 9007199254740991}
// Always initialize with the "QueryBuilder" and "QueryCompiler" objects, which
// extend the base 'lib/query/builder' and 'lib/query/compiler', respectively.
function Client_MSSQL(config) {
@ -88,6 +91,7 @@ assign(Client_MSSQL.prototype, {
// Grab a connection, run the query via the MSSQL streaming interface,
// and pass that through to the stream we've sent back to the client.
_stream(connection, obj, stream, options) {
const client = this;
options = options || {}
if (!obj || typeof obj === 'string') obj = {sql: obj}
// convert ? params into positional bindings (@p1)
@ -104,7 +108,7 @@ assign(Client_MSSQL.prototype, {
req.stream = true;
if (obj.bindings) {
for (let i = 0; i < obj.bindings.length; i++) {
req.input(`p${i}`, obj.bindings[i])
client._setReqInput(req, i, obj.bindings[i])
}
}
req.pipe(stream)
@ -115,6 +119,7 @@ assign(Client_MSSQL.prototype, {
// Runs the query on the specified connection, providing the bindings
// and any other necessary prep work.
_query(connection, obj) {
const client = this;
if (!obj || typeof obj === 'string') obj = {sql: obj}
// convert ? params into positional bindings (@p1)
obj.sql = this.positionBindings(obj.sql);
@ -127,7 +132,7 @@ assign(Client_MSSQL.prototype, {
req.multiple = true;
if (obj.bindings) {
for (let i = 0; i < obj.bindings.length; i++) {
req.input(`p${i}`, obj.bindings[i])
client._setReqInput(req, i, obj.bindings[i])
}
}
req.query(sql, function(err, recordset) {
@ -138,6 +143,18 @@ assign(Client_MSSQL.prototype, {
})
},
// sets a request input parameter. Detects bigints and sets type appropriately.
_setReqInput(req, i, binding) {
if (typeof binding == 'number' && (binding < SQL_INT4.MIN || binding > SQL_INT4.MAX)) {
if (binding < SQL_BIGINT_SAFE.MIN || binding > SQL_BIGINT_SAFE.MAX) {
throw new Error(`Bigint must be safe integer or must be passed as string, saw ${binding}`)
}
req.input(`p${i}`, this.driver.BigInt, binding)
} else {
req.input(`p${i}`, binding)
}
},
// Process the response as returned from the query.
processResponse(obj, runner) {
if (obj == null) return;

View File

@ -0,0 +1,73 @@
/*global describe, it, expect, testPromise*/
'use strict';
var Promise = testPromise;
module.exports = function (knex) {
var bigintTimestamp = 1464294366973;
var negativeBigintTimestamp = -1464294366973;
var unsafeBigint = 99071992547409911;
var negativeUnsafeBigint = -99071992547409911;
it('#test number mssql should not allow unsafe bigint', function () {
if (!/mssql/i.test(knex.client.dialect)) {
return Promise.resolve();
}
var constraintName = 'pk_id';
var tableName = 'bigint_test';
return knex.transaction(function (tr) {
return tr.schema.dropTableIfExists(tableName)
.then(function () {
return tr.schema.createTable(tableName, function (table) {
table.string('id').primary(constraintName);
table.bigInteger('expiry');
});
});
}).then(function () {
return knex(tableName).where('expiry', unsafeBigint).select("*");
}).map(function (row) {
// triggers request execution
}).then(function () {
return knex(tableName).where('expiry', negativeUnsafeBigint).select("*");
}).map(function (row) {
// triggers request execution
}).catch(function (err) {
expect(err).to.be.an.instanceof(Error);
expect(err.message).to.contain('Bigint must be safe integer or must be passed as string');
});
});
it('#test number mssql should allow safe bigint', function () {
if (!/mssql/i.test(knex.client.dialect)) {
return Promise.resolve();
}
var constraintName = 'pk_id';
var tableName = 'bigint_test';
return knex.transaction(function (tr) {
return tr.schema.dropTableIfExists(tableName)
.then(function () {
return tr.schema.createTable(tableName, function (table) {
table.string('id').primary(constraintName);
table.bigInteger('expiry');
});
});
}).then(function () {
return knex(tableName).insert({id : "positive", expiry: bigintTimestamp});
}).then(function () {
return knex(tableName).insert({id : "negative", expiry: negativeBigintTimestamp});
}).then(function () {
return knex(tableName).where('expiry', bigintTimestamp).select("*");
}).map(function (row) {
console.log(row);
expect(row.id).to.equal('positive');
}).then(function () {
return knex(tableName).where('expiry', negativeBigintTimestamp).select("*");
}).map(function (row) {
console.log(row);
expect(row.id).to.equal('negative');
}).catch(function (err) {
expect(err).to.be.undefined;
});
});
};

View File

@ -22,6 +22,7 @@ module.exports = function(knex) {
require('./builder/transaction')(knex);
require('./builder/deletes')(knex);
require('./builder/additional')(knex);
require('./datatype/bigint')(knex);
describe('knex.destroy', function() {
it('should allow destroying the pool with knex.destroy', function() {