From 11955a0ad2b2f2687988248c4c995be4cc2bb622 Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Fri, 11 Aug 2023 17:50:07 +0200 Subject: [PATCH 1/6] Add property usePrebuiltEmptyResultObjects to Query constructor which generates pre-shaped result rows --- packages/pg/lib/query.js | 21 ++++++-- packages/pg/lib/result.js | 3 +- .../test/integration/gh-issues/3042-tests.js | 51 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 packages/pg/test/integration/gh-issues/3042-tests.js diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 6655a0e69..129207241 100644 --- a/packages/pg/lib/query.js +++ b/packages/pg/lib/query.js @@ -24,13 +24,14 @@ class Query extends EventEmitter { if (process.domain && config.callback) { this.callback = process.domain.bind(config.callback) } + this.usePrebuiltEmptyResultObjects = config.usePrebuiltEmptyResultObjects == true this._result = new Result(this._rowMode, this.types) - // potential for multiple results this._results = this._result this.isPreparedStatement = false this._canceledDueToError = false this._promise = null + this._prebuiltEmptyResultObject = null } requiresPreparation() { @@ -64,6 +65,7 @@ class Query extends EventEmitter { } this._result = new Result(this._rowMode, this.types) this._results.push(this._result) + this._prebuiltEmptyResultObject = null } } @@ -83,8 +85,12 @@ class Query extends EventEmitter { return } + if (this.usePrebuiltEmptyResultObjects && !this._prebuiltEmptyResultObject) { + this._createPrebuiltEmptyResultObject() + } + try { - row = this._result.parseRow(msg.fields) + row = this._result.parseRow(msg.fields, this.usePrebuiltEmptyResultObjects ? { ... this._prebuiltEmptyResultObject } : {}) } catch (err) { this._canceledDueToError = err return @@ -138,7 +144,7 @@ class Query extends EventEmitter { try { this.callback(null, this._results) } - catch(err) { + catch (err) { process.nextTick(() => { throw err }) @@ -236,6 +242,13 @@ class Query extends EventEmitter { handleCopyData(msg, connection) { // noop } + _createPrebuiltEmptyResultObject() { + var row = {}; + for (var i = 0; i < this._result.fields.length; i++) { + row[this._result.fields[i].name] = null + } + this._prebuiltEmptyResultObject = row + } } -module.exports = Query +module.exports = Query \ No newline at end of file diff --git a/packages/pg/lib/result.js b/packages/pg/lib/result.js index 350609743..259ee4aae 100644 --- a/packages/pg/lib/result.js +++ b/packages/pg/lib/result.js @@ -59,8 +59,7 @@ class Result { return row } - parseRow(rowData) { - var row = {} + parseRow(rowData, row = {}) { for (var i = 0, len = rowData.length; i < len; i++) { var rawValue = rowData[i] var field = this.fields[i].name diff --git a/packages/pg/test/integration/gh-issues/3042-tests.js b/packages/pg/test/integration/gh-issues/3042-tests.js new file mode 100644 index 000000000..b7ca4bacc --- /dev/null +++ b/packages/pg/test/integration/gh-issues/3042-tests.js @@ -0,0 +1,51 @@ +'use strict' +const helper = require('../test-helper') + +const suite = new helper.Suite() + +// https://github.com/brianc/node-postgres/issues/2716 +suite.testAsync('rows returned from pg have a clean shape which allows faster access and copying', async () => { + const amountOfCols = 100 + const testCopies = 10000 + const expectedFasterFactor = 50 + + const client = new helper.pg.Client() + await client.connect() + + + const queryFields = [] + for (let i = 1; i <= amountOfCols; i++) { + queryFields.push(`${i} as col${i}`) + } + const query = `SELECT ${queryFields.join(', ')}` + const resultWithoutShape = await client.query({ + text: query, + usePrebuiltEmptyResultObjects: false + }) + const rowFromPgWithoutShape = resultWithoutShape.rows[0] + const resultWithShape = await client.query({ + text: query, + usePrebuiltEmptyResultObjects: true + }) + const rowFromPgWithShape = resultWithShape.rows[0] + + + const beforeWithout = process.hrtime.bigint() + for (let i = 0; i < testCopies; i++) { + let copy = { ...rowFromPgWithoutShape } + } + const afterWithout = process.hrtime.bigint() + const withoutResult = afterWithout - beforeWithout + + const beforeWith = process.hrtime.bigint() + for (let i = 0; i < testCopies; i++) { + let copy = { ...rowFromPgWithShape } + } + const afterWith = process.hrtime.bigint() + const withResult = afterWith - beforeWith + + const factorFaster = withoutResult / withResult + assert(factorFaster > expectedFasterFactor) + + await client.end() +}) From c8abc18eb35ec6cdd8286b31a13c67e803f37d72 Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Fri, 11 Aug 2023 22:15:26 +0200 Subject: [PATCH 2/6] Remove option and test for prebuiltEmptyResultObject --- packages/pg/lib/query.js | 9 ++-- .../test/integration/gh-issues/3042-tests.js | 51 ------------------- 2 files changed, 4 insertions(+), 56 deletions(-) delete mode 100644 packages/pg/test/integration/gh-issues/3042-tests.js diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 129207241..87dda214b 100644 --- a/packages/pg/lib/query.js +++ b/packages/pg/lib/query.js @@ -24,7 +24,6 @@ class Query extends EventEmitter { if (process.domain && config.callback) { this.callback = process.domain.bind(config.callback) } - this.usePrebuiltEmptyResultObjects = config.usePrebuiltEmptyResultObjects == true this._result = new Result(this._rowMode, this.types) // potential for multiple results this._results = this._result @@ -85,12 +84,12 @@ class Query extends EventEmitter { return } - if (this.usePrebuiltEmptyResultObjects && !this._prebuiltEmptyResultObject) { + if (!this._prebuiltEmptyResultObject) { this._createPrebuiltEmptyResultObject() } try { - row = this._result.parseRow(msg.fields, this.usePrebuiltEmptyResultObjects ? { ... this._prebuiltEmptyResultObject } : {}) + row = this._result.parseRow(msg.fields, { ... this._prebuiltEmptyResultObject }) } catch (err) { this._canceledDueToError = err return @@ -144,7 +143,7 @@ class Query extends EventEmitter { try { this.callback(null, this._results) } - catch (err) { + catch(err) { process.nextTick(() => { throw err }) @@ -251,4 +250,4 @@ class Query extends EventEmitter { } } -module.exports = Query \ No newline at end of file +module.exports = Query diff --git a/packages/pg/test/integration/gh-issues/3042-tests.js b/packages/pg/test/integration/gh-issues/3042-tests.js deleted file mode 100644 index b7ca4bacc..000000000 --- a/packages/pg/test/integration/gh-issues/3042-tests.js +++ /dev/null @@ -1,51 +0,0 @@ -'use strict' -const helper = require('../test-helper') - -const suite = new helper.Suite() - -// https://github.com/brianc/node-postgres/issues/2716 -suite.testAsync('rows returned from pg have a clean shape which allows faster access and copying', async () => { - const amountOfCols = 100 - const testCopies = 10000 - const expectedFasterFactor = 50 - - const client = new helper.pg.Client() - await client.connect() - - - const queryFields = [] - for (let i = 1; i <= amountOfCols; i++) { - queryFields.push(`${i} as col${i}`) - } - const query = `SELECT ${queryFields.join(', ')}` - const resultWithoutShape = await client.query({ - text: query, - usePrebuiltEmptyResultObjects: false - }) - const rowFromPgWithoutShape = resultWithoutShape.rows[0] - const resultWithShape = await client.query({ - text: query, - usePrebuiltEmptyResultObjects: true - }) - const rowFromPgWithShape = resultWithShape.rows[0] - - - const beforeWithout = process.hrtime.bigint() - for (let i = 0; i < testCopies; i++) { - let copy = { ...rowFromPgWithoutShape } - } - const afterWithout = process.hrtime.bigint() - const withoutResult = afterWithout - beforeWithout - - const beforeWith = process.hrtime.bigint() - for (let i = 0; i < testCopies; i++) { - let copy = { ...rowFromPgWithShape } - } - const afterWith = process.hrtime.bigint() - const withResult = afterWith - beforeWith - - const factorFaster = withoutResult / withResult - assert(factorFaster > expectedFasterFactor) - - await client.end() -}) From 2172d4e19d5cd73f6b0dab05c447c34daad0d308 Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Fri, 11 Aug 2023 22:17:31 +0200 Subject: [PATCH 3/6] Remove errorneously added newline --- packages/pg/lib/query.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 87dda214b..1974c9ac9 100644 --- a/packages/pg/lib/query.js +++ b/packages/pg/lib/query.js @@ -25,6 +25,7 @@ class Query extends EventEmitter { this.callback = process.domain.bind(config.callback) } this._result = new Result(this._rowMode, this.types) + // potential for multiple results this._results = this._result this.isPreparedStatement = false From cfff0b9a1e8ffe6fa2cd488707d2ccdbdf1bf3ec Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Fri, 11 Aug 2023 22:43:07 +0200 Subject: [PATCH 4/6] Move all logic for prebuilding objects to Result --- packages/pg/lib/query.js | 15 +-------------- packages/pg/lib/result.js | 16 +++++++++++++--- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/packages/pg/lib/query.js b/packages/pg/lib/query.js index 1974c9ac9..6655a0e69 100644 --- a/packages/pg/lib/query.js +++ b/packages/pg/lib/query.js @@ -31,7 +31,6 @@ class Query extends EventEmitter { this.isPreparedStatement = false this._canceledDueToError = false this._promise = null - this._prebuiltEmptyResultObject = null } requiresPreparation() { @@ -65,7 +64,6 @@ class Query extends EventEmitter { } this._result = new Result(this._rowMode, this.types) this._results.push(this._result) - this._prebuiltEmptyResultObject = null } } @@ -85,12 +83,8 @@ class Query extends EventEmitter { return } - if (!this._prebuiltEmptyResultObject) { - this._createPrebuiltEmptyResultObject() - } - try { - row = this._result.parseRow(msg.fields, { ... this._prebuiltEmptyResultObject }) + row = this._result.parseRow(msg.fields) } catch (err) { this._canceledDueToError = err return @@ -242,13 +236,6 @@ class Query extends EventEmitter { handleCopyData(msg, connection) { // noop } - _createPrebuiltEmptyResultObject() { - var row = {}; - for (var i = 0; i < this._result.fields.length; i++) { - row[this._result.fields[i].name] = null - } - this._prebuiltEmptyResultObject = row - } } module.exports = Query diff --git a/packages/pg/lib/result.js b/packages/pg/lib/result.js index 259ee4aae..5a5de6d0d 100644 --- a/packages/pg/lib/result.js +++ b/packages/pg/lib/result.js @@ -21,6 +21,7 @@ class Result { if (this.rowAsArray) { this.parseRow = this._parseRowAsArray } + this._prebuiltEmptyResultObject = null } // adds a command complete message @@ -59,14 +60,16 @@ class Result { return row } - parseRow(rowData, row = {}) { + parseRow(rowData) { + if (!this._prebuiltEmptyResultObject) { + this._createPrebuiltEmptyResultObject() + } + var row = { ... this._prebuiltEmptyResultObject } for (var i = 0, len = rowData.length; i < len; i++) { var rawValue = rowData[i] var field = this.fields[i].name if (rawValue !== null) { row[field] = this._parsers[i](rawValue) - } else { - row[field] = null } } return row @@ -94,6 +97,13 @@ class Result { } } } + _createPrebuiltEmptyResultObject() { + var row = {}; + for (var i = 0; i < this.fields.length; i++) { + row[this.fields[i].name] = null + } + this._prebuiltEmptyResultObject = row + } } module.exports = Result From 2de4a4ed0da81f0c2424cbcdd370a0604ec517fa Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Fri, 11 Aug 2023 22:51:24 +0200 Subject: [PATCH 5/6] Move prebuilding to addFields --- packages/pg/lib/result.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/pg/lib/result.js b/packages/pg/lib/result.js index 5a5de6d0d..374e4ad9d 100644 --- a/packages/pg/lib/result.js +++ b/packages/pg/lib/result.js @@ -61,9 +61,6 @@ class Result { } parseRow(rowData) { - if (!this._prebuiltEmptyResultObject) { - this._createPrebuiltEmptyResultObject() - } var row = { ... this._prebuiltEmptyResultObject } for (var i = 0, len = rowData.length; i < len; i++) { var rawValue = rowData[i] @@ -96,6 +93,7 @@ class Result { this._parsers[i] = types.getTypeParser(desc.dataTypeID, desc.format || 'text') } } + this._createPrebuiltEmptyResultObject() } _createPrebuiltEmptyResultObject() { var row = {}; From 41533fdaf14a92e54caf6aa0a8b33498e0a92c0a Mon Sep 17 00:00:00 2001 From: HZ111 / Dev2 Date: Sun, 13 Aug 2023 11:37:19 +0200 Subject: [PATCH 6/6] Use a clone as clone-base --- packages/pg/lib/result.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pg/lib/result.js b/packages/pg/lib/result.js index 374e4ad9d..187c0d016 100644 --- a/packages/pg/lib/result.js +++ b/packages/pg/lib/result.js @@ -96,11 +96,11 @@ class Result { this._createPrebuiltEmptyResultObject() } _createPrebuiltEmptyResultObject() { - var row = {}; + var row = {} for (var i = 0; i < this.fields.length; i++) { row[this.fields[i].name] = null } - this._prebuiltEmptyResultObject = row + this._prebuiltEmptyResultObject = { ... row } } }