From 14eeeb868cdcb287b7c89f94dca7e42e5ba9eecd Mon Sep 17 00:00:00 2001 From: GermanBluefox Date: Sat, 3 Oct 2026 00:17:13 +0300 Subject: [PATCH] Guard against insert() concatenating statements again (#294) The bug reported in #294 is gone. The adapter used to emit INSERT INTO `iobroker`.ts_counter ...;INSERT INTO `iobroker`.ts_number ...; as a single query, which MariaDB rejects: no driver here is configured for multi-statement batches. Reproducing the reported batch against the current builders gives two separate statements on all four dialects, and _insertValuesIntoDB() sends them one at a time over one borrowed connection. What was missing is anything that keeps it that way. Joining the statements back together is the obvious "optimization" for someone looking at a list of queries where one would do, and the only thing standing in the way was a comment. test/testInsertStatements.js now asserts, per dialect, that a batch holding both counter and value rows produces one statement per table, that no statement carries a second one behind a semicolon, and that a 1200 row batch chunks into 500/500/200 without the chunks being glued back together. Verified by restoring the old behaviour in the compiled mysql builder: the three mysql cases fail, and pass again once it is reverted. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/test-and-release.yml | 2 +- test/testInsertStatements.js | 83 ++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) create mode 100644 test/testInsertStatements.js diff --git a/.github/workflows/test-and-release.yml b/.github/workflows/test-and-release.yml index 25befe3..de78ff8 100644 --- a/.github/workflows/test-and-release.yml +++ b/.github/workflows/test-and-release.yml @@ -200,7 +200,7 @@ jobs: # @iobroker/adapter-core, which calls process.exit(10) when js-controller cannot be # resolved, and js-controller is not a dependency of this repository. - name: Run unit tests - run: node node_modules/mocha/bin/mocha test/testCommons.js test/testIntegral.js test/testDockerCompose.js test/testErrors.js test/testSqlClient.js test/testConnectionOptions.js test/testMessageGuard.js test/testStatistics.js test/testPackageFiles.js --exit + run: node node_modules/mocha/bin/mocha test/testCommons.js test/testIntegral.js test/testDockerCompose.js test/testErrors.js test/testSqlClient.js test/testConnectionOptions.js test/testMessageGuard.js test/testStatistics.js test/testInsertStatements.js test/testPackageFiles.js --exit - name: Run SQLite tests run: node node_modules/mocha/bin/mocha test/testSQLite.js --exit diff --git a/test/testInsertStatements.js b/test/testInsertStatements.js new file mode 100644 index 0000000..b68e7e6 --- /dev/null +++ b/test/testInsertStatements.js @@ -0,0 +1,83 @@ +const assert = require('node:assert'); + +// Imported per dialect, never from build/main: that would pull in @iobroker/adapter-core, which +// calls process.exit(10) when js-controller cannot be resolved. +const DIALECTS = { + mysql: require('../build/lib/mysql'), + postgresql: require('../build/lib/postgresql'), + mssql: require('../build/lib/mssql'), + sqlite: require('../build/lib/sqlite'), +}; + +// A statement that carries a second one after a semicolon. The trailing semicolon of the statement +// itself is not a match - only a semicolon with another statement behind it. +const CONCATENATED = /;\s*(INSERT|SELECT|UPDATE|DELETE)/i; + +function rows() { + // The batch from issue #294: a counter datapoint produces rows in ts_counter *and* ts_number + // within the same flush, which is what used to end up in one query. + return [ + { table: 'ts_counter', state: { val: 53.32, ts: 1676135872618 }, from: 2 }, + { table: 'ts_counter', state: { val: 53, ts: 1676138387812 }, from: 2 }, + { table: 'ts_number', state: { val: 53, ts: 1676138387812, ack: true }, from: 2 }, + { table: 'ts_counter', state: { val: 52.95, ts: 1676138670902 }, from: 2 }, + { table: 'ts_number', state: { val: 52.95, ts: 1676138670902, ack: true }, from: 2 }, + ]; +} + +describe('Test insert() never concatenates statements', function () { + for (const [dialect, sql] of Object.entries(DIALECTS)) { + it(`${dialect}: emits one statement per table instead of one joined query`, function () { + const queries = sql.insert('iobroker', 5, rows()); + + assert.ok(Array.isArray(queries), 'insert() returns a list of statements'); + assert.strictEqual(queries.length, 2, `ts_counter and ts_number, got ${queries.length}`); + + // This is issue #294: the adapter used to send + // INSERT INTO ...ts_counter ...;INSERT INTO ...ts_number ...; + // as a single query, which MariaDB rejects with a syntax error because no driver here + // is configured for multi-statement batches. + for (const query of queries) { + assert.ok(!CONCATENATED.test(query), `carries a second statement: ${query}`); + assert.strictEqual((query.match(/INSERT/gi) || []).length, 1, `more than one INSERT in: ${query}`); + } + + const tables = queries.map(q => (q.match(/ts_\w+/) || [])[0]); + assert.deepStrictEqual([...tables].sort(), ['ts_counter', 'ts_number']); + }); + + it(`${dialect}: keeps the statements apart for every table`, function () { + const queries = sql.insert('iobroker', 5, [ + { table: 'ts_number', state: { val: 1, ts: 1, ack: true }, from: 2 }, + { table: 'ts_string', state: { val: 'x', ts: 2, ack: true }, from: 2 }, + { table: 'ts_bool', state: { val: true, ts: 3, ack: true }, from: 2 }, + { table: 'ts_counter', state: { val: 4, ts: 4 }, from: 2 }, + ]); + + assert.strictEqual(queries.length, 4, 'one per table'); + for (const query of queries) { + assert.ok(!CONCATENATED.test(query), `carries a second statement: ${query}`); + } + }); + + it(`${dialect}: chunks a large batch without joining the chunks`, function () { + const many = []; + for (let i = 0; i < 1200; i++) { + many.push({ table: 'ts_number', state: { val: i, ts: 1700000000000 + i, ack: true }, from: 2 }); + } + + const queries = sql.insert('iobroker', 5, many); + + // 500 rows per statement, so the chunks must stay separate queries rather than being + // glued back together with semicolons. + assert.strictEqual(queries.length, 3, `expected 500/500/200, got ${queries.length} chunks`); + for (const query of queries) { + assert.ok(!CONCATENATED.test(query), `carries a second statement: ${query}`); + assert.strictEqual((query.match(/INSERT/gi) || []).length, 1); + } + + const tuples = queries.map(q => (q.match(/\),\(/g) || []).length + 1); + assert.deepStrictEqual(tuples, [500, 500, 200]); + }); + } +});