Skip to content

Commit b793a3a

Browse files
ruiquelhasruiquelhas
committed
BUG#34016587: fix precision loss for DECIMAL values
Values stored in numeric column data types are being retrieved either as JavaScript strings or numbers depending on whether there is a chance of losing precision or not. However, this behavior is not consistent in the case of DECIMAL values where the decimal part (the digits to the right of the decimal point) cannot be accommodated by a JavaScript number. In this case, unless the integer part cannot also be accommodated, the value is effectively retrieved as a JavaScript number and it looses decimal precision. This patch introduces the changes to address this issue ensuring DECIMAL values are returned as a JavaScript string when there is a chance of losing precision. Change-Id: I9a494059d9d902b0169beda0abd1df5d20c0d080
1 parent 15ed85e commit b793a3a

File tree

4 files changed

+99
-15
lines changed
  • lib/Protocol/Wrappers/Messages/Resultset
  • test
    • functional/default
    • unit/Protocol/Wrappers/Messages/Resultset

4 files changed

+99
-15
lines changed

‎lib/Protocol/Wrappers/Messages/Resultset/Row.js‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2021, Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2022, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -306,13 +306,22 @@ function decodeUnsignedInt (decoder, options) {
306306
* @returns {number|string} A JavaScript string (when there is overflow) or number.
307307
*/
308308
function formatSafeNumber (value) {
309-
const num = parseFloat(value);
310-
311-
if (num < Number.MIN_SAFE_INTEGER || num > Number.MAX_SAFE_INTEGER) {
312-
return value;
309+
const matcher = value.match(/(\d+)\.?(\d+)?$/) || [];
310+
const int = matcher[1];
311+
const decimal = matcher[2];
312+
313+
const isUnsafe = Number.parseFloat(int) < Number.MIN_SAFE_INTEGER ||
314+
Number.parseFloat(int) > Number.MAX_SAFE_INTEGER ||
315+
Number.parseFloat(decimal) > Number.MAX_SAFE_INTEGER;
316+
317+
// If the value is unsafe, we should not convert it to a JavaScript number
318+
// because we might be loosing precision.
319+
if (isUnsafe) {
320+
// Any initial "+" sign should not be part of the string.
321+
return value.replace('+', '');
313322
}
314323

315-
return num;
324+
return Number.parseFloat(value);
316325
}
317326

318327
/**

‎test/functional/default/relational-tables/misc.js‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2021, Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2022, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -558,6 +558,34 @@ describe('relational miscellaneous tests', () => {
558558
});
559559
});
560560

561+
context('BUG#34016587 values encoded as DECIMAL with precision loss', () => {
562+
beforeEach('create table', () => {
563+
// A JavaScript number does not have enough precision to encode
564+
// more than 17 corresponding DECIMAL digits in MySQL.
565+
return session.sql(`CREATE TABLE test (
566+
decimal_1 DECIMAL(18, 1),
567+
decimal_2 DECIMAL(18, 17),
568+
decimal_3 DECIMAL(18, 17),
569+
decimal_4 DECIMAL(18, 1))`)
570+
.execute();
571+
});
572+
573+
beforeEach('add fixtures', () => {
574+
return session.sql('INSERT INTO test VALUES (-99999999999999999.9, -9.99999999999999999, 9.99999999999999999, 99999999999999999.9)')
575+
.execute();
576+
});
577+
578+
it('decodes values using a string to avoid precision loss', () => {
579+
const expected = [['-99999999999999999.9', '-9.99999999999999999', '9.99999999999999999', '99999999999999999.9']];
580+
581+
return schema.getTable('test').select()
582+
.execute()
583+
.then(res => {
584+
return expect(res.fetchAll()).to.deep.equal(expected);
585+
});
586+
});
587+
});
588+
561589
context('values encoded as SET', () => {
562590
beforeEach('create table', () => {
563591
return session.sql('CREATE TABLE test (a_set SET(\'foo\', \'bar\'))')

‎test/functional/default/sql.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,6 +1122,34 @@ describe('raw SQL', () => {
11221122
});
11231123
});
11241124

1125+
context('BUG#34016587 values encoded as DECIMAL with precision loss', () => {
1126+
beforeEach('create table', () => {
1127+
// A JavaScript number does not have enough precision to encode
1128+
// more than 17 corresponding DECIMAL digits in MySQL.
1129+
return session.sql(`CREATE TABLE test (
1130+
decimal_1 DECIMAL(18, 1),
1131+
decimal_2 DECIMAL(18, 17),
1132+
decimal_3 DECIMAL(18, 17),
1133+
decimal_4 DECIMAL(18, 1))`)
1134+
.execute();
1135+
});
1136+
1137+
beforeEach('add fixtures', () => {
1138+
return session.sql('INSERT INTO test VALUES (-99999999999999999.9, -9.99999999999999999, 9.99999999999999999, 99999999999999999.9)')
1139+
.execute();
1140+
});
1141+
1142+
it('decodes values using a string to avoid precision loss', () => {
1143+
const expected = [['-99999999999999999.9', '-9.99999999999999999', '9.99999999999999999', '99999999999999999.9']];
1144+
1145+
return session.sql('select * from test')
1146+
.execute()
1147+
.then(res => {
1148+
return expect(res.fetchAll()).to.deep.equal(expected);
1149+
});
1150+
});
1151+
});
1152+
11251153
context('warnings', () => {
11261154
beforeEach('create table', () => {
11271155
return session.sql('CREATE TABLE test (a TINYINT NOT NULL, b VARCHAR(3))')

‎test/unit/Protocol/Wrappers/Messages/Resultset/Row.js‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2020, 2021, Oracle and/or its affiliates.
2+
* Copyright (c) 2020, 2022, Oracle and/or its affiliates.
33
*
44
* This program is free software; you can redistribute it and/or modify
55
* it under the terms of the GNU General Public License, version 2.0, as
@@ -469,25 +469,44 @@ describe('Mysqlx.Resultset.Row wrapper', () => {
469469
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([(new Date('2018-02-19T15:21:26.123Z')).getTime()]);
470470
});
471471

472-
it('returns decimal values as JavaScript numbers', () => {
472+
it('returns decimal values as JavaScript numbers when there is no risk of precision loss', () => {
473473
const columnProto = new ResultsetStub.ColumnMetaData([ResultsetStub.ColumnMetaData.FieldType.DECIMAL]);
474-
475-
let decimal = Buffer.from('04123401d0', 'hex');
474+
const decimal = Buffer.from('04123401d0', 'hex'); // d0 => sign ("-")
476475

477476
const rowProto = new ResultsetStub.Row();
478477
rowProto.addField(bytes.create(decimal).valueOf());
479478

480479
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([-12.3401]);
480+
});
481481

482-
const overflow = Number.MAX_SAFE_INTEGER + 1;
483-
const scale = 10; // overflow size in hexadecimal
482+
it('returns decimal values as JavaScript strings when there is a risk of precision loss', () => {
483+
const columnProto = new ResultsetStub.ColumnMetaData([ResultsetStub.ColumnMetaData.FieldType.DECIMAL]);
484+
const overflow = Number.MAX_SAFE_INTEGER + 1; // length = 16
485+
const safeNumber = 9; // length = 1
486+
487+
let scale = '10'; // overflow size in hexadecimal (parseInt(10, 16) = 16)
488+
let decimal = Buffer.from(`${scale}${safeNumber}${overflow}c0`, 'hex'); // c0 => sign ("+")
489+
490+
const rowProto = new ResultsetStub.Row();
491+
rowProto.addField(bytes.create(decimal).valueOf());
492+
493+
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([`${safeNumber}.${overflow}`]);
494+
495+
scale = '01'; // safe number size in hexadecimal
496+
decimal = Buffer.from(`${scale}${overflow}${safeNumber}d0`, 'hex'); // d0 => sign ("-")
497+
498+
rowProto.clearFieldList();
499+
rowProto.addField(bytes.create(decimal).valueOf());
500+
501+
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([`-${overflow}.${safeNumber}`]);
484502

485-
decimal = Buffer.from(`${scale}${overflow}${overflow}c0`, 'hex');
503+
scale = '10'; // overflow size in hexadecimal (parseInt(10, 16) = 16)
504+
decimal = Buffer.from(`${scale}${overflow}${overflow}c0`, 'hex'); // c0 => sign ("+")
486505

487506
rowProto.clearFieldList();
488507
rowProto.addField(bytes.create(decimal).valueOf());
489508

490-
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([`+${overflow}.${overflow}`]);
509+
expect(row(rowProto, { metadata: [columnMetadata(columnProto)] }).toArray()).to.deep.equal([`${overflow}.${overflow}`]);
491510
});
492511

493512
it('returns set values as JavaScript arrays', () => {

0 commit comments

Comments
 (0)