Off-by-one error with supportBigNumbers

[joanniclaborde]

Hello,

I think I found an issue with the supportBigNumbers connection option. Running this quick test:

const mysql = require('mysql2/promise');

async function test() {
  const connection = await mysql.createConnection({uri: 'mysql://...', supportBigNumbers: true});
  const [results] = await connection.query(`
    SELECT
      -9007199254740993 AS 'MIN_SAFE_INTEGER - 2',
      -9007199254740992 AS 'MIN_SAFE_INTEGER - 1',
      -9007199254740991 AS 'MIN_SAFE_INTEGER',
      -9007199254740990 AS 'MIN_SAFE_INTEGER + 1',
      -9007199254740989 AS 'MIN_SAFE_INTEGER + 2',
       9007199254740989 AS 'MAX_SAFE_INTEGER - 2',
       9007199254740990 AS 'MAX_SAFE_INTEGER - 1',
       9007199254740991 AS 'MAX_SAFE_INTEGER',
       9007199254740992 AS 'MAX_SAFE_INTEGER + 1',
       9007199254740993 AS 'MAX_SAFE_INTEGER + 2'
  `);
  console.log(results[0]);
  connection.end();
}

test();

Results in the following:

{
  'MIN_SAFE_INTEGER - 2': '-9007199254740993',
  'MIN_SAFE_INTEGER - 1':  -9007199254740992, // Expecting this to be a string
  'MIN_SAFE_INTEGER':      -9007199254740991,
  'MIN_SAFE_INTEGER + 1':  -9007199254740990,
  'MIN_SAFE_INTEGER + 2':  -9007199254740989,
  'MAX_SAFE_INTEGER - 2':   9007199254740989,
  'MAX_SAFE_INTEGER - 1':   9007199254740990,
  'MAX_SAFE_INTEGER':       9007199254740991,
  'MAX_SAFE_INTEGER + 1':   9007199254740992, // Expecting this to be a string
  'MAX_SAFE_INTEGER + 2':  '9007199254740993'
}

Given the values of Number.MIN_SAFE_INTEGER and Number.MAX_SAFE_INTEGER, shouldn't the values highlighted above be strings?

Thanks!

[joanniclaborde]

Here is another test:

SELECT
  9007199254740991, # Number.MAX_SAFE_INTEGER
  9007199254740992,
  9007199254740993,
  9007199254740994,
  9007199254740995,
  9007199254740996,
  9007199254740997,
  9007199254740998,
  9007199254740999,
  9007199254740100,
  9007199254740101

{
  '9007199254740991': 9007199254740991,
  '9007199254740992': 9007199254740992,
  '9007199254740993': '9007199254740993',
  '9007199254740994': 9007199254740994,
  '9007199254740995': '9007199254740995',
  '9007199254740996': 9007199254740996,
  '9007199254740997': '9007199254740997',
  '9007199254740998': 9007199254740998,
  '9007199254740999': '9007199254740999',
  '9007199254740100': 9007199254740100,
  '9007199254740101': 9007199254740101
}

And while I'm at it, I tested the upper limit of UNSIGNED BIGINT. The result is correct (all strings), unless I add the decimalNumbers option:

const connection = await mysql.createConnection({uri: '...', supportBigNumbers: true, decimalNumbers: true});
const results = (await connection.query(`
  SELECT
    18446744073709551615, # UNSIGNED BIGINT
    18446744073709551616,
    18446744073709551617,
    18446744073709551618,
    18446744073709551619,
    18446744073709551620,
    18446744073709551621,
    18446744073709551622,
    18446744073709551623,
    18446744073709551624,
    18446744073709551625
`))[0];
console.log(results[0]);

{
  '18446744073709551615': '18446744073709551615',
  '18446744073709551616': 18446744073709552000,
  '18446744073709551617': 18446744073709552000,
  '18446744073709551618': 18446744073709552000,
  '18446744073709551619': 18446744073709552000,
  '18446744073709551620': 18446744073709552000,
  '18446744073709551621': 18446744073709552000,
  '18446744073709551622': 18446744073709552000,
  '18446744073709551623': 18446744073709552000,
  '18446744073709551624': 18446744073709552000,
  '18446744073709551625': 18446744073709552000
}

[mattyp]

I think it depends on what is trying to be achieved. We encountered the same anomaly with a BigInt value of 36001000000201450 being returned as a number rather than a string. On one hand, this can be considered correct behaviour as 36001000000201450 can be precisely represented by a number type, so does not need to be a string. On the other hand, it could be considered incorrect behaviour as anything beyond the safe integer range should be represented as strings to ensure appropriate precision is retained.

The bit of logic in the driver that determines this is in packet.js, based on the digit count:

      if (numDigits >= 15) {
        str = this.readString(end - this.offset, 'binary');
        result = parseInt(str, 10);
        if (result.toString() === str) {
          return sign * result;
        }
        return sign === -1 ? `-${str}` : str;
      }
      if (numDigits > 16) {
        str = this.readString(end - this.offset);
        return sign === -1 ? `-${str}` : str;
      }

It might make more sense for the driver to utilise the isSafeInteger function on the parsed result to decide if a string or number is returned. This would make anything outside the safe integer range be returned as string, even if it can be precisely represented as a number.

Using supportBigNumbers can be interesting with aggregate results from a SQL query, too, as the return type of aggregate functions is BigInt (e.g. COUNT(*)). Depending on the value being returned from the aggregate function, it could be returned as a number or as a string. Typically, we all presume those results will always be numbers.