Skip to content

Commit ce2ad75

Browse files
authored
fix: resolve parser cache collision with dual typeCast connections (#3644)
* fix: resolve parser cache collision with dual typeCast connections The parser cache system had a bug where connections with different typeCast boolean values (true vs false) would share cached parsers, causing incorrect data type conversion. Update `parser_cache.js` with following changes : - Enhanced cache key generation to create unique keys for each typeCast value - Now typeCast: true and typeCast: false generate distinct cache keys - Maintains function-based typeCast compatibility - Ensures proper cache isolation and eliminates ambiguous cache hits Add test to validate changes in `test-typecast-dual-connections.test.cjs` with following changes : - Added test-typecast-dual-connections.test.cjs to validate the fix - Tests concurrent connections with different typeCast boolean settings - Prevents future regressions in parser cache logic Signed-off-by: Mohit Malhotra <[email protected]> * enh: update parser key generation condition for typecast and tests to match parser cache behavior Update test expectations for `typeCast` field to reflect actual implementation where `typeCast` returns the value directly instead of the type. This aligns tests with the `lib/parsers/parser_cache.js` key generation logic. Add more robust check for generating key for parser caching for the field `tyepCast` to ensure value is used for only boolean input. Also removed `test-typecast-dual-connections.test.cjs` as existing testcases are sufficient. Signed-off-by: Mohit Malhotra <[email protected]> --------- Signed-off-by: Mohit Malhotra <[email protected]>
1 parent 4e395ea commit ce2ad75

File tree

2 files changed

+67
-9
lines changed

2 files changed

+67
-9
lines changed

lib/parsers/parser_cache.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ function keyFromFields(type, fields, options, config) {
1414
Boolean(options.rowsAsArray),
1515
Boolean(options.supportBigNumbers || config.supportBigNumbers),
1616
Boolean(options.bigNumberStrings || config.bigNumberStrings),
17-
typeof options.typeCast,
17+
typeof options.typeCast === 'boolean'
18+
? options.typeCast
19+
: typeof options.typeCast,
1820
options.timezone || config.timezone,
1921
Boolean(options.decimalNumbers),
2022
options.dateStrings,

test/esm/unit/parsers/cache-key-serialization.test.mjs

Lines changed: 64 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,46 @@ const test9 = {
326326
},
327327
};
328328

329+
// Similar to test4 but with typeCast: false
330+
const test10 = {
331+
type: 'binary',
332+
fields: [
333+
{
334+
name: 'id',
335+
columnType: '3',
336+
length: undefined,
337+
schema: 'test',
338+
table: 'test',
339+
flags: '16899',
340+
characterSet: '63',
341+
},
342+
{
343+
name: 'value',
344+
columnType: '246',
345+
length: undefined,
346+
schema: 'test',
347+
table: 'test',
348+
flags: '0',
349+
characterSet: '63',
350+
},
351+
],
352+
options: {
353+
nestTables: false,
354+
rowsAsArray: false,
355+
supportBigNumbers: false,
356+
bigNumberStrings: false,
357+
typeCast: false,
358+
timezone: 'local',
359+
decimalNumbers: false,
360+
dateStrings: 'DATETIME',
361+
},
362+
config: {
363+
supportBigNumbers: undefined,
364+
bigNumberStrings: undefined,
365+
timezone: undefined,
366+
},
367+
};
368+
329369
const result1 = _keyFromFields(
330370
test1.type,
331371
test1.fields,
@@ -380,6 +420,12 @@ const result9 = _keyFromFields(
380420
test9.options,
381421
test9.config
382422
);
423+
const result10 = _keyFromFields(
424+
test10.type,
425+
test10.fields,
426+
test10.options,
427+
test10.config
428+
);
383429

384430
assert.deepStrictEqual(
385431
result1,
@@ -395,13 +441,13 @@ assert(JSON.parse(result2));
395441

396442
assert.deepStrictEqual(
397443
result3,
398-
'[null,"string","",false,false,false,"boolean","local",false,false,[null,null,null,null,null,null,null]]'
444+
'[null,"string","",false,false,false,true,"local",false,false,[null,null,null,null,null,null,null]]'
399445
);
400446
assert(JSON.parse(result3));
401447

402448
assert.deepStrictEqual(
403449
result4,
404-
'["binary","boolean",false,false,false,false,"boolean","local",false,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]'
450+
'["binary","boolean",false,false,false,false,true,"local",false,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]'
405451
);
406452
assert(JSON.parse(result4));
407453

@@ -410,14 +456,14 @@ assert(JSON.parse(result5));
410456

411457
assert.deepStrictEqual(
412458
result6,
413-
'["binary","boolean",false,true,true,true,"boolean","\\"\\"`\'",true,"#",[":","©",null,"/",",","_","❌"]]'
459+
'["binary","boolean",false,true,true,true,true,"\\"\\"`\'",true,"#",[":","©",null,"/",",","_","❌"]]'
414460
);
415461
// Ensuring that JSON is valid with invalid delimiters
416462
assert(JSON.parse(result6));
417463

418464
assert.deepStrictEqual(
419465
result7,
420-
'["binary","boolean",true,true,true,true,"boolean","local",true,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]'
466+
'["binary","boolean",true,true,true,true,true,"local",true,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]'
421467
);
422468
assert(JSON.parse(result7));
423469

@@ -433,7 +479,13 @@ assert(JSON.parse(result9)[5] === true);
433479
assert(JSON.parse(result9)[6] === 'function');
434480
assert(JSON.parse(result9)[9] === null);
435481

436-
// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same
482+
assert.deepStrictEqual(
483+
result10,
484+
'["binary","boolean",false,false,false,false,false,"local",false,"DATETIME",["id","3",null,"test","test","16899","63"],["value","246",null,"test","test","0","63"]]'
485+
);
486+
assert(JSON.parse(result10));
487+
488+
// Testing twice all existent tests needs to return 8 keys, since two of them expects to be the same
437489
assert(
438490
Array.from(
439491
new Set([
@@ -455,8 +507,10 @@ assert(
455507
_keyFromFields(test8.type, test8.fields, test8.options, test8.config),
456508
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
457509
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
510+
_keyFromFields(test10.type, test10.fields, test10.options, test10.config),
511+
_keyFromFields(test10.type, test10.fields, test10.options, test10.config),
458512
])
459-
).length === 7
513+
).length === 8
460514
);
461515

462516
const stringify = JSON.stringify;
@@ -465,7 +519,7 @@ const stringify = JSON.stringify;
465519
JSON.stringify = (value, replacer, space = 8) =>
466520
stringify(value, replacer, space);
467521

468-
// Testing twice all existent tests needs to return 7 keys, since two of them expects to be the same
522+
// Testing twice all existent tests needs to return 8 keys, since two of them expects to be the same
469523
assert(
470524
Array.from(
471525
new Set([
@@ -487,6 +541,8 @@ assert(
487541
_keyFromFields(test8.type, test8.fields, test8.options, test8.config),
488542
result9,
489543
_keyFromFields(test9.type, test9.fields, test9.options, test9.config),
544+
result10,
545+
_keyFromFields(test10.type, test10.fields, test10.options, test10.config),
490546
])
491-
).length === 7
547+
).length === 8
492548
);

0 commit comments

Comments
 (0)