From b33ccc669893623cd139234800a42d553f1b6bbc Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Wed, 13 Mar 2024 13:37:20 +0200 Subject: [PATCH 01/11] :bug: streaming reads should emit the dataset number for each dataset --- lib/commands/query.js | 6 +- package.json | 1 + .../test-multi-result-streaming.cjs | 61 +++++++++++++++++++ 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 test/integration/test-multi-result-streaming.cjs diff --git a/lib/commands/query.js b/lib/commands/query.js index 67386bbfba..e8cd215bb0 100644 --- a/lib/commands/query.js +++ b/lib/commands/query.js @@ -251,7 +251,7 @@ class Query extends Command { if (this.onResult) { this._rows[this._resultIndex].push(row); } else { - this.emit('result', row); + this.emit('result', row, this._resultIndex); } return Query.prototype.row; } @@ -268,11 +268,11 @@ class Query extends Command { stream._read = () => { this._connection && this._connection.resume(); }; - this.on('result', row => { + this.on('result', (row, resultSetIndex) => { if (!stream.push(row)) { this._connection.pause(); } - stream.emit('result', row); // replicate old emitter + stream.emit('result', row, resultSetIndex); // replicate old emitter }); this.on('error', err => { stream.emit('error', err); // Pass on any errors diff --git a/package.json b/package.json index d58248a814..105a11393e 100644 --- a/package.json +++ b/package.json @@ -11,6 +11,7 @@ "lint:typings": "npx prettier --check ./typings", "lint:tests": "npx prettier --check ./test", "test": "poku --debug --include=\"test/esm,test/unit,test/integration\"", + "test-me": "poku --debug --include=test/integration/test-multi-result-streaming.cjs", "test:bun": "poku --debug --platform=\"bun\" --include=\"test/esm,test/unit,test/integration\"", "test:tsc-build": "cd \"test/tsc-build\" && npx tsc -p \"tsconfig.json\"", "coverage-test": "c8 -r cobertura -r lcov -r text npm run test", diff --git a/test/integration/test-multi-result-streaming.cjs b/test/integration/test-multi-result-streaming.cjs new file mode 100644 index 0000000000..772d82c0dc --- /dev/null +++ b/test/integration/test-multi-result-streaming.cjs @@ -0,0 +1,61 @@ +const { createConnection } = require('../common.test.cjs'); +(async function() { + 'use strict'; + + const { assert } = require('poku'); + + const + conn = createConnection({ multipleStatements: true }), + captured1 = [], + captured2 = [], + sql1 = 'select * from information_schema.columns order by table_schema, table_name, column_name limit 1;', + sql2 = 'select * from information_schema.columns order by table_schema, table_name limit 1;'; + + const compare1 = await conn.promise().query( + sql1 + ); + const compare2 = await conn.promise().query( + sql2 + ); + + if (!compare1 || compare1.length < 1) { + assert.fail('no results for comparison 1'); + } + if (!compare2 || compare2.length < 1) { + assert.fail('no results for comparison 2'); + } + + const stream = conn.query(`${ sql1 }\n${ sql2 }`).stream(); + stream.on('result', (row, datasetIndex) => { + if (datasetIndex === 0) { + captured1.push(row); + } else { + captured2.push(row); + } + }); + // note: this is very important: + // after each result set is complete, + // the stream will emit "readable" and if we don't + // read then 'end' won't be emitted and the + // test will hang. + stream.on("readable", () => { + stream.read(); + }); + + await new Promise((resolve, reject) => { + stream.on('error', e => reject(e)); + stream.on('end', () => resolve()); + }); + + try { + assert.equal(captured1.length, 1); + assert.equal(captured2.length, 1); + assert.deepEqual(captured1[0], compare1[0][0]); + assert.deepEqual(captured2[0], compare2[0][0]); + process.exit(0); + } catch (e) { + console.error(e); + process.exit(1); + } + +})(); From 39364544020430ad67053669ef360b3969d60d04 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Wed, 13 Mar 2024 16:41:42 +0200 Subject: [PATCH 02/11] :ok_hand: attend to PR commentary --- package.json | 1 - test/integration/test-multi-result-streaming.cjs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 105a11393e..d58248a814 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,6 @@ "lint:typings": "npx prettier --check ./typings", "lint:tests": "npx prettier --check ./test", "test": "poku --debug --include=\"test/esm,test/unit,test/integration\"", - "test-me": "poku --debug --include=test/integration/test-multi-result-streaming.cjs", "test:bun": "poku --debug --platform=\"bun\" --include=\"test/esm,test/unit,test/integration\"", "test:tsc-build": "cd \"test/tsc-build\" && npx tsc -p \"tsconfig.json\"", "coverage-test": "c8 -r cobertura -r lcov -r text npm run test", diff --git a/test/integration/test-multi-result-streaming.cjs b/test/integration/test-multi-result-streaming.cjs index 772d82c0dc..1f4124fd44 100644 --- a/test/integration/test-multi-result-streaming.cjs +++ b/test/integration/test-multi-result-streaming.cjs @@ -1,8 +1,8 @@ +const { assert } = require('poku'); const { createConnection } = require('../common.test.cjs'); -(async function() { - 'use strict'; - const { assert } = require('poku'); +(async () => { + 'use strict'; const conn = createConnection({ multipleStatements: true }), From 7aaee08df9eef3bb497b3b64130078c7f5e752d2 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Wed, 13 Mar 2024 18:09:07 +0200 Subject: [PATCH 03/11] :ok_hand: rename test fixture file, as per PR request --- ...-result-streaming.cjs => test-multi-result-streaming.test.cjs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/integration/{test-multi-result-streaming.cjs => test-multi-result-streaming.test.cjs} (100%) diff --git a/test/integration/test-multi-result-streaming.cjs b/test/integration/test-multi-result-streaming.test.cjs similarity index 100% rename from test/integration/test-multi-result-streaming.cjs rename to test/integration/test-multi-result-streaming.test.cjs From a20d122630d1c683e91c27a1f7685f717f9803fc Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 09:06:00 +0200 Subject: [PATCH 04/11] :rotating_light: run prettier -w on test-multi-result-streaming.test.cjs --- .../test-multi-result-streaming.test.cjs | 38 ++++++++----------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/test/integration/test-multi-result-streaming.test.cjs b/test/integration/test-multi-result-streaming.test.cjs index 1f4124fd44..1a4cf9518a 100644 --- a/test/integration/test-multi-result-streaming.test.cjs +++ b/test/integration/test-multi-result-streaming.test.cjs @@ -4,19 +4,16 @@ const { createConnection } = require('../common.test.cjs'); (async () => { 'use strict'; - const - conn = createConnection({ multipleStatements: true }), + const conn = createConnection({ multipleStatements: true }), captured1 = [], captured2 = [], - sql1 = 'select * from information_schema.columns order by table_schema, table_name, column_name limit 1;', - sql2 = 'select * from information_schema.columns order by table_schema, table_name limit 1;'; + sql1 = + 'select * from information_schema.columns order by table_schema, table_name, column_name limit 1;', + sql2 = + 'select * from information_schema.columns order by table_schema, table_name limit 1;'; - const compare1 = await conn.promise().query( - sql1 - ); - const compare2 = await conn.promise().query( - sql2 - ); + const compare1 = await conn.promise().query(sql1); + const compare2 = await conn.promise().query(sql2); if (!compare1 || compare1.length < 1) { assert.fail('no results for comparison 1'); @@ -25,7 +22,7 @@ const { createConnection } = require('../common.test.cjs'); assert.fail('no results for comparison 2'); } - const stream = conn.query(`${ sql1 }\n${ sql2 }`).stream(); + const stream = conn.query(`${sql1}\n${sql2}`).stream(); stream.on('result', (row, datasetIndex) => { if (datasetIndex === 0) { captured1.push(row); @@ -38,24 +35,19 @@ const { createConnection } = require('../common.test.cjs'); // the stream will emit "readable" and if we don't // read then 'end' won't be emitted and the // test will hang. - stream.on("readable", () => { + stream.on('readable', () => { stream.read(); }); await new Promise((resolve, reject) => { - stream.on('error', e => reject(e)); + stream.on('error', (e) => reject(e)); stream.on('end', () => resolve()); }); - try { - assert.equal(captured1.length, 1); - assert.equal(captured2.length, 1); - assert.deepEqual(captured1[0], compare1[0][0]); - assert.deepEqual(captured2[0], compare2[0][0]); - process.exit(0); - } catch (e) { - console.error(e); - process.exit(1); - } + assert.equal(captured1.length, 1); + assert.equal(captured2.length, 1); + assert.deepEqual(captured1[0], compare1[0][0]); + assert.deepEqual(captured2[0], compare2[0][0]); + process.exit(0); })(); From dfd19e8d7cf3d756c8a0c713266bdd3c9a580056 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 10:24:44 +0200 Subject: [PATCH 05/11] :alembic: try latest mysql --- .github/workflows/ci-linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index 21d72b3bab..df50ac976d 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: node-version: [14.x, 16.x, 18.x, 20.x, "21.x"] - mysql-version: ["mysql:8.0.33"] + mysql-version: ["mysql:8.0.36"] use-compression: [0, 1] use-tls: [0, 1] mysql_connection_url_key: [""] From 11c77e6f04288340dbc3fa7e0627c5f7c812eebb Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 10:29:23 +0200 Subject: [PATCH 06/11] :alembic: enable GHA for this branch, hopefully --- .github/workflows/ci-linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index df50ac976d..6220f3d600 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -3,7 +3,7 @@ name: CI - Linux on: pull_request: push: - branches: [ main ] + branches: [ main, fix/streaming-interface-should-provide-dataset-index ] workflow_dispatch: From 72e113c6d90d0ada933850f48f1fdeb1cfebfc58 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 12:10:40 +0200 Subject: [PATCH 07/11] :bug: no need to process.exit(0) in the test if the connection is properly destroyed --- test/integration/test-multi-result-streaming.test.cjs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/integration/test-multi-result-streaming.test.cjs b/test/integration/test-multi-result-streaming.test.cjs index 1a4cf9518a..a97c9d1f78 100644 --- a/test/integration/test-multi-result-streaming.test.cjs +++ b/test/integration/test-multi-result-streaming.test.cjs @@ -12,6 +12,8 @@ const { createConnection } = require('../common.test.cjs'); sql2 = 'select * from information_schema.columns order by table_schema, table_name limit 1;'; + await conn.promise().query("set global max_allowed_packet=524288000"); + const compare1 = await conn.promise().query(sql1); const compare2 = await conn.promise().query(sql2); @@ -49,5 +51,5 @@ const { createConnection } = require('../common.test.cjs'); assert.deepEqual(captured1[0], compare1[0][0]); assert.deepEqual(captured2[0], compare2[0][0]); - process.exit(0); + conn.destroy(); })(); From f6ccfbf9487130b1132e507e17be893494b702d9 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 12:14:35 +0200 Subject: [PATCH 08/11] :rewind: revert addition of this branch as a trigger --- .github/workflows/ci-linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index 6220f3d600..df50ac976d 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -3,7 +3,7 @@ name: CI - Linux on: pull_request: push: - branches: [ main, fix/streaming-interface-should-provide-dataset-index ] + branches: [ main ] workflow_dispatch: From 0bb4ae5a614065d80cf510ac743dd3dd7736d89a Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 12:40:06 +0200 Subject: [PATCH 09/11] :rewind: revert mysql version update --- .github/workflows/ci-linux.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-linux.yml b/.github/workflows/ci-linux.yml index df50ac976d..21d72b3bab 100644 --- a/.github/workflows/ci-linux.yml +++ b/.github/workflows/ci-linux.yml @@ -19,7 +19,7 @@ jobs: fail-fast: false matrix: node-version: [14.x, 16.x, 18.x, 20.x, "21.x"] - mysql-version: ["mysql:8.0.36"] + mysql-version: ["mysql:8.0.33"] use-compression: [0, 1] use-tls: [0, 1] mysql_connection_url_key: [""] From ed186f8890b1ac6172411c7b1e2c8c290bd6bf76 Mon Sep 17 00:00:00 2001 From: Davyd McColl Date: Thu, 14 Mar 2024 12:44:04 +0200 Subject: [PATCH 10/11] :rotating_light: pacify the linter --- test/integration/test-multi-result-streaming.test.cjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/test-multi-result-streaming.test.cjs b/test/integration/test-multi-result-streaming.test.cjs index a97c9d1f78..44701af726 100644 --- a/test/integration/test-multi-result-streaming.test.cjs +++ b/test/integration/test-multi-result-streaming.test.cjs @@ -12,7 +12,7 @@ const { createConnection } = require('../common.test.cjs'); sql2 = 'select * from information_schema.columns order by table_schema, table_name limit 1;'; - await conn.promise().query("set global max_allowed_packet=524288000"); + await conn.promise().query('set global max_allowed_packet=524288000'); const compare1 = await conn.promise().query(sql1); const compare2 = await conn.promise().query(sql2); From f98476d5b40068d37d3c84e7927cacdb588f228a Mon Sep 17 00:00:00 2001 From: wellwelwel <46850407+wellwelwel@users.noreply.github.com> Date: Fri, 26 Apr 2024 17:20:28 -0300 Subject: [PATCH 11/11] ci: debug multilpe stream test order --- .../test-multi-result-streaming.test.cjs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/integration/test-multi-result-streaming.test.cjs b/test/integration/test-multi-result-streaming.test.cjs index 44701af726..81fc2dcc1f 100644 --- a/test/integration/test-multi-result-streaming.test.cjs +++ b/test/integration/test-multi-result-streaming.test.cjs @@ -1,16 +1,16 @@ +'use strict'; + const { assert } = require('poku'); const { createConnection } = require('../common.test.cjs'); (async () => { - 'use strict'; - - const conn = createConnection({ multipleStatements: true }), - captured1 = [], - captured2 = [], - sql1 = - 'select * from information_schema.columns order by table_schema, table_name, column_name limit 1;', - sql2 = - 'select * from information_schema.columns order by table_schema, table_name limit 1;'; + const conn = createConnection({ multipleStatements: true }); + const captured1 = []; + const captured2 = []; + const sql1 = + 'select * from information_schema.columns order by table_schema, table_name, column_name limit 1;'; + const sql2 = + 'select * from information_schema.columns order by table_schema, table_name, ordinal_position limit 1;'; await conn.promise().query('set global max_allowed_packet=524288000'); @@ -51,5 +51,5 @@ const { createConnection } = require('../common.test.cjs'); assert.deepEqual(captured1[0], compare1[0][0]); assert.deepEqual(captured2[0], compare2[0][0]); - conn.destroy(); + conn.end(); })();