node-postgres icon indicating copy to clipboard operation
node-postgres copied to clipboard

pool.query(new QueryStream(...)) unexpected behaviour

Open m-ronchi opened this issue 6 years ago • 6 comments

Hi, if I use the query method on pg.Pool with a query object that implements sumbit (i.e. pg-query-stream or pg-cursor) like this:

import { Pool } from "pg";
import QueryStream = require("pg-query-stream");
const pool = new Pool();
// ...
const query = new QueryStream("SELECT * FROM ...");
const q = pool.query(query);
console.log(q); // Promise { <pending>, ...
await q; //!

the returned promise never resolves and the pool never releases the used client

I could ignore the return value and continue with the result, e.g.

for await (const row in query) { ...process row...}

but the client leak is a problem

furthermore, the typescript definition is plain wrong:

export class Pool extends events.EventEmitter {
// ...
    query<T extends Submittable>(queryStream: T): T;
// ...
}

m-ronchi avatar Nov 27 '19 16:11 m-ronchi

yeah that sounds like a problem - I'll definitely look at this. For now I think a workaround you can do is

const client = await pool.connect()
const query = new QueryStream('SELECT *')
const stream = client.query(query)
for await (const row of stream) {

}
client.release()

Sorry for the hassle - i'll get this fixed

brianc avatar Nov 27 '19 22:11 brianc

that's what I did (wrapped in try-finally)

m-ronchi avatar Nov 28 '19 08:11 m-ronchi

Just running into this now with [email protected]. and [email protected], is the workaround above still the best option?

timothyallan avatar Nov 17 '20 00:11 timothyallan

I am wondering if this is the cause of the issue that I am seeing on [email protected]. I have a codebase that appears to run OK but the issue appears in functional testing. Our unit tests actually run against a database and we use pool.query to run queries. We have a lot of unit tests and about 3/4 of the way through we see :

(node:4274) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 end listeners added to [Connection]. Use emitter.setMaxListeners() to increase limit at _addListener (events.js:281:17) at Connection.addListener (events.js:297:10) at Connection.once (events.js:328:8) at /Users/username/GitHub/my-code-base/node_modules/pg/lib/client.js:604:25 at new WrappedPromise (/Users/username/GitHub/my-code-base/node_modules/async-listener/es6-wrapped-promise.js:13:18) at Client.end (/Users/username/GitHub/my-code-base/node_modules/pg/lib/client.js:603:14) at BoundPool._remove (/Users/username/GitHub/my-code-base/node_modules/pg-pool/index.js:157:12) at Timeout.<anonymous> (/Users/username/GitHub/my-code-base/node_modules/pg-pool/index.js:324:14) at Timeout._onTimeout (/Users/username/GitHub/my-code-base/node_modules/async-listener/glue.js:188:31) at listOnTimeout (internal/timers.js:531:17) at processTimers (internal/timers.js:475:7) Our pg.query is like this:

async query(text, values){ const res = await this.pool.query(text, values); return res; }

Is there anything that I can add to our function to ensure it is releasing things correctly (if this is in fact the issue)? Many thanks.

I have tried replacing pool.query with getting a client and then releasing it but this doesn't resolve the issue

MelissaSnell avatar Mar 29 '21 10:03 MelissaSnell

I've submitted a PR that fixes this 👀

#2810

aleclarson avatar Sep 07 '22 17:09 aleclarson

Run into this today as well, it tooks me several hours to figure out why (and yet I know well this library !)

It would be great to finalize the PR

abenhamdine avatar Feb 18 '24 12:02 abenhamdine