Skip to content

Commit 099beea

Browse files
authored
fix: fix precision loss for large decimal values (#4135)
* fix(parser): prevent precision loss for numbers > 17 chars Add length-based bailout to parseFloat() to fix accumulated rounding errors from repeated *10 operations. For numbers longer than 17 characters, delegate to Number.parseFloat() which handles precision correctly. This fixes two critical issues: - DECIMAL(36,18) precision loss where 50000.000...0 parsed as 49999.999 - MAX_VALUE doubles corruption where last digits were incorrect The threshold of 17 is based on IEEE 754 double precision limits (~15-17 significant digits). Testing shows this affects only ~1% of typical MySQL data while preserving the fast path for 98%+ of cases. Add comprehensive test suite with 54 test cases covering both issues, edge cases, and regression tests. Closes #3690 Closes #2928 * test: add integration tests for issues #3690 and #2928 Add tests exercising parseFloat bailout paths to improve coverage: - DECIMAL(36,18) with many fractional digits (>17 chars) - DOUBLE with scientific notation values These integration tests ensure the bailout conditions are covered in real database query scenarios.
1 parent ff8ed8a commit 099beea

File tree

3 files changed

+380
-6
lines changed

3 files changed

+380
-6
lines changed

lib/packets/packet.js

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -660,14 +660,27 @@ class Packet {
660660
if (len === null) {
661661
return null;
662662
}
663+
if (len === 0) {
664+
return 0; // TODO: assert? exception?
665+
}
666+
667+
// For numbers with many digits (>17), use built-in parseFloat to avoid
668+
// precision loss from accumulated rounding errors in repeated *10 operations.
669+
// This fixes issues #2928 (MAX_VALUE doubles) and #3690 (DECIMAL(36,18))
670+
// where very large numbers or numbers with many fractional digits lose precision.
671+
// The threshold of 17 is based on IEEE 754 double precision (~15-17 significant digits).
672+
// Testing shows minimal performance impact as most real-world numbers are shorter.
673+
if (len > 17) {
674+
const str = this.buffer.toString('utf8', this.offset, this.offset + len);
675+
this.offset += len;
676+
return Number.parseFloat(str);
677+
}
678+
663679
let result = 0;
664680
const end = this.offset + len;
665681
let factor = 1;
666682
let pastDot = false;
667683
let charCode = 0;
668-
if (len === 0) {
669-
return 0; // TODO: assert? exception?
670-
}
671684
if (this.buffer[this.offset] === minus) {
672685
this.offset++;
673686
factor = -1;
@@ -681,9 +694,13 @@ class Packet {
681694
pastDot = true;
682695
this.offset++;
683696
} else if (charCode === exponent || charCode === exponentCapital) {
684-
this.offset++;
685-
const exponentValue = this.parseInt(end - this.offset);
686-
return (result / factor) * Math.pow(10, exponentValue);
697+
// Scientific notation detected - bail out to parseFloat for exact match.
698+
// Manual calculation with Math.pow(10, exp) cannot match parseFloat()
699+
// exactly for most non-zero exponents due to accumulated rounding errors.
700+
const start = end - len;
701+
const str = this.buffer.toString('utf8', start, end);
702+
this.offset = end;
703+
return Number.parseFloat(str);
687704
} else {
688705
result *= 10;
689706
result += this.buffer[this.offset] - 48;

test/integration/connection/test-decimals-as-numbers.test.mts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,33 @@ await describe('Decimals as Numbers', async () => {
4242
strict.equal(rows2[0].d1, largeMoneyValue);
4343
});
4444

45+
await it('should parse DECIMAL(36,18) with many fractional digits correctly (issue #3690)', async () => {
46+
const rows = await new Promise<RowDataPacket[]>((resolve, reject) => {
47+
connection2.query<RowDataPacket[]>(
48+
'SELECT CAST("+50000.000000000000000000" AS DECIMAL(36,18)) as big_decimal',
49+
(err, _rows) => (err ? reject(err) : resolve(_rows))
50+
);
51+
});
52+
53+
strict.equal(rows[0].big_decimal.constructor, Number);
54+
strict.equal(rows[0].big_decimal, 50000);
55+
});
56+
57+
await it('should parse DOUBLE with scientific notation correctly (issue #2928)', async () => {
58+
const rows = await new Promise<RowDataPacket[]>((resolve, reject) => {
59+
connection2.query<RowDataPacket[]>(
60+
'SELECT +1.7976931348623157e308 as max_double, ' +
61+
'-1.7976931348623157e308 as min_double, ' +
62+
'1e100 as sci_notation',
63+
(err, _rows) => (err ? reject(err) : resolve(_rows))
64+
);
65+
});
66+
67+
strict.equal(rows[0].max_double, 1.7976931348623157e308);
68+
strict.equal(rows[0].min_double, -1.7976931348623157e308);
69+
strict.equal(rows[0].sci_notation, 1e100);
70+
});
71+
4572
connection1.end();
4673
connection2.end();
4774
});

0 commit comments

Comments
 (0)