From 441788192df0292e422dc07be30438f29ba4cdff Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 9 Jan 2020 10:44:05 -0600 Subject: [PATCH 1/3] Fix one unintentional possible breaking change & update readme --- CHANGELOG.md | 5 +++++ packages/pg-query-stream/index.js | 4 ++-- packages/pg-query-stream/test/config.js | 26 +++++++++++++++++++++---- 3 files changed, 29 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9d646527..1146193f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ For richer information consult the commit log on github with referenced pull req We do not include break-fix version release in this file. +### pg-query-stream@3.0.0 + +- [Rewrote stream internals](https://github.com/brianc/node-postgres/pull/2051) to better conform to node stream semantics. This should make pg-query-stream much better at respecting [highWaterMark](https://nodejs.org/api/stream.html#stream_new_stream_readable_options) and getting rid of some edge case bugs when using pg-query-stream as an async iterator. Due to the size and nature of this change (effectively a full re-write) it's safest to bump the semver major here, though almost all tests remain untouched and still passing, which brings us to a breaking change to the API.... +- Changed `stream.close` to `stream.destroy` which is the [official](https://nodejs.org/api/stream.html#stream_readable_destroy_error) way to terminate a readable stream. This is a __breaking change__ if you rely on the `stream.close` method on pg-query-stream...though should be just a find/replace type operation to upgrade as the semantics remain very similar (not exactly the same, since internals are rewritte, but more in line with how streams are "supposed" to behave). + ### pg@7.17.0 - Add support for `idle_in_transaction_session_timeout` [option](https://github.com/brianc/node-postgres/pull/2049). diff --git a/packages/pg-query-stream/index.js b/packages/pg-query-stream/index.js index 4576f5fb5..20073381d 100644 --- a/packages/pg-query-stream/index.js +++ b/packages/pg-query-stream/index.js @@ -3,9 +3,9 @@ const Cursor = require('pg-cursor') class PgQueryStream extends Readable { constructor(text, values, config = {}) { - const { batchSize = 100 } = config; + const { batchSize, highWaterMark = 100 } = config; // https://nodejs.org/api/stream.html#stream_new_stream_readable_options - super({ objectMode: true, emitClose: true, autoDestroy: true, highWaterMark: batchSize }) + super({ objectMode: true, emitClose: true, autoDestroy: true, highWaterMark: batchSize || highWaterMark }) this.cursor = new Cursor(text, values, config) // delegate Submittable callbacks to cursor diff --git a/packages/pg-query-stream/test/config.js b/packages/pg-query-stream/test/config.js index 78251c894..1634f6174 100644 --- a/packages/pg-query-stream/test/config.js +++ b/packages/pg-query-stream/test/config.js @@ -1,8 +1,26 @@ var assert = require('assert') var QueryStream = require('../') -var stream = new QueryStream('SELECT NOW()', [], { - batchSize: 88 -}) +describe('stream config options', () => { + // this is mostly for backwards compatability. + it('sets readable.highWaterMark based on batch size', () => { + var stream = new QueryStream('SELECT NOW()', [], { + batchSize: 88 + }) + assert.equal(stream._readableState.highWaterMark, 88) + }) + + it('sets readable.highWaterMark based on highWaterMark config', () => { + var stream = new QueryStream('SELECT NOW()', [], { + highWaterMark: 88 + }) + + assert.equal(stream._readableState.highWaterMark, 88) + }) -assert.equal(stream._readableState.highWaterMark, 88) + it('defaults to 100 for highWaterMark', () => { + var stream = new QueryStream('SELECT NOW()', []) + + assert.equal(stream._readableState.highWaterMark, 100) + }) +}) From 93800ccf109ec4cea81b676bdb8b48c7b629d24c Mon Sep 17 00:00:00 2001 From: "Brian M. Carlson" Date: Thu, 9 Jan 2020 10:46:52 -0600 Subject: [PATCH 2/3] Update changelog more --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1146193f0..4e6de1430 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ We do not include break-fix version release in this file. - [Rewrote stream internals](https://github.com/brianc/node-postgres/pull/2051) to better conform to node stream semantics. This should make pg-query-stream much better at respecting [highWaterMark](https://nodejs.org/api/stream.html#stream_new_stream_readable_options) and getting rid of some edge case bugs when using pg-query-stream as an async iterator. Due to the size and nature of this change (effectively a full re-write) it's safest to bump the semver major here, though almost all tests remain untouched and still passing, which brings us to a breaking change to the API.... - Changed `stream.close` to `stream.destroy` which is the [official](https://nodejs.org/api/stream.html#stream_readable_destroy_error) way to terminate a readable stream. This is a __breaking change__ if you rely on the `stream.close` method on pg-query-stream...though should be just a find/replace type operation to upgrade as the semantics remain very similar (not exactly the same, since internals are rewritte, but more in line with how streams are "supposed" to behave). +- Unified the `config.batchSize` and `config.highWaterMark` to both do the same thing: control how many rows are buffered in memory. The `ReadableStream` will manage exactly how many rows are requested from the cursor at a time. This should give better out of the box performance and help with efficient async interation. ### pg@7.17.0 From 4f07e1ee02afdc01bc57c1d5b2966c8aea3b7a30 Mon Sep 17 00:00:00 2001 From: Brian C Date: Thu, 9 Jan 2020 21:31:12 -0600 Subject: [PATCH 3/3] Update CHANGELOG.md Co-Authored-By: Charmander <~@charmander.me> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e6de1430..7d7f86170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,7 +7,7 @@ We do not include break-fix version release in this file. ### pg-query-stream@3.0.0 - [Rewrote stream internals](https://github.com/brianc/node-postgres/pull/2051) to better conform to node stream semantics. This should make pg-query-stream much better at respecting [highWaterMark](https://nodejs.org/api/stream.html#stream_new_stream_readable_options) and getting rid of some edge case bugs when using pg-query-stream as an async iterator. Due to the size and nature of this change (effectively a full re-write) it's safest to bump the semver major here, though almost all tests remain untouched and still passing, which brings us to a breaking change to the API.... -- Changed `stream.close` to `stream.destroy` which is the [official](https://nodejs.org/api/stream.html#stream_readable_destroy_error) way to terminate a readable stream. This is a __breaking change__ if you rely on the `stream.close` method on pg-query-stream...though should be just a find/replace type operation to upgrade as the semantics remain very similar (not exactly the same, since internals are rewritte, but more in line with how streams are "supposed" to behave). +- Changed `stream.close` to `stream.destroy` which is the [official](https://nodejs.org/api/stream.html#stream_readable_destroy_error) way to terminate a readable stream. This is a __breaking change__ if you rely on the `stream.close` method on pg-query-stream...though should be just a find/replace type operation to upgrade as the semantics remain very similar (not exactly the same, since internals are rewritten, but more in line with how streams are "supposed" to behave). - Unified the `config.batchSize` and `config.highWaterMark` to both do the same thing: control how many rows are buffered in memory. The `ReadableStream` will manage exactly how many rows are requested from the cursor at a time. This should give better out of the box performance and help with efficient async interation. ### pg@7.17.0