Docs: document client.release()

[abenhamdine]

Hi Brian,

I just upgraded from pg@5.1 to pg@6.2.3, and noticed something :

• in pg versions <6, pg.client has not attached release() method.
• since 6.0.0 (or perhaps later ?), client.release() exists.

Therefore I presume that it's no longer necessary to use the following pattern (from the current version of wiki) :

pool.connect(function(err, client, release) {

  client.query('SELECT $1::text as name', ['foo'], function(err, result) {
    release();
    console.log(result.rows[0].name); // output: foo
  });
});

and now with pg >=6, we can safely use this shorter way ?

pool.connect(function(err, client) {

  client.query('SELECT $1::text as name', ['foo'], function(err, result) {
    // note we use directly the release() method of client object
    client.release();
    console.log(result.rows[0].name); // output: foo
  });
});

It seems to work fine with my code, but I would want to be sure that is the proper way to release the client and that client.release() is part of the public API.

[sehrope]

No that's incorrect. The automatic borrow/return from the pool only happens if you use the pool.query(...) functions. If you explicitly borrow a client via pool.connect(...) then you need to return it to the pool by calling the release(err?) (i..e "done") function. Otherwise it'll be a connection leak.

The simplest fix is to use pool.query(...) everywhere. Assuming you're not using transactions or require multiple commands in sequence within the same connection, then that will work everywhere.

If you do require transactions or more esoteric use cases that require maintaining state within a given connection (ex: modifying connection properties), then you need to understand the borrow/return and manual connection management.

tl;dr: Use pool.query(...)

[abenhamdine]

Thx @sehrope for your quick answer !

As you have presumed correctly, I can't use pool.query() everywhere because I sometimes use transactions

I use manual connection/release for quite a long time now in a big application, and I think I understand the principle, but I always found weird to have to handle two separate arguments, ie client & release() to manage properly the client lifecycle.
So I thought it would be nice to use only the client object to do it.

In pg<6, to do it, I used to use a custom connect method :

// override pg.connect to have a convenient method to connect
pg.customConnect = function (cb) {
		pg.connect(function (err, client, release) {
			if (err) {
				return callback(err);
			}
			client.release = release;
			return cb(null, client);
		});
};

so I was able to do :

pg.customConnect(function(err, client) {
  client.query('SELECT $1::text as name', ['foo'], function(err, result) {
    client.release();
    console.log(result.rows[0].name); // output: foo
  });
});

With pg>=6, client.release() is already there ! That's why I wonder if it is a drop-in replacement for the release() method.

You said it isn't, but why there is a client.release() method if it doesn't return client to the pool ? So what does this method do ?

Thx you a lot by advance.

[sehrope]

The new function is to allow it to be used via the Promise interface.

With callbacks you have the option of receiving additional parameters to your callback (ex: the release()) function.

With Promises, you don't have that option so any additional functions need to be on the returned object. You can see an example of it here: https://github.com/brianc/node-pg-pool#acquire-clients-with-a-promise

I'd suggest sticking to one or the other. Using the callback interface to connect then switching over to the Promise interface to release the connection will probably work as under the covers they're calling the same code (see https://github.com/brianc/node-pg-pool/blob/master/index.js#L125). It just seems ugly to mix the two APIs.

[brianc]

yeah sorry I kinda fangled up the promise & callback api - I should have made a separate API surface area for promises. It's something I plan on re-working (in an almost completely drop-in way) in 7.0. There will be a little bit of API breakage...but I also plan on really beefing up the docs to make things more clear. And, yeah 'release' could be modified to work with the callback style as well. I'll add this to 7.0 milestone so I can track it. Sorry for the confusion. 😬

[abenhamdine]

Ok, thx to both of you, it's clear now, at least for me, but probably confusing for newcomers...
Especially when you use typescript definitions, so your IDE kindly suggests .release() on autocompletion when you type client 😬

I think that's something that definitely deserves to be in the docs for version 7.0

@brianc : I don't know if the Promise & Callback API need to be distinct concerning this specific point. As @sehrope pointed, the code is currently exactly the same : https://github.com/brianc/node-pg-pool/blob/master/index.js#L125 therefore it's fine to call client.release() while using the callback API.

[charmander]

Yep, client.release() is going to have to remain fine when using callbacks (and it seems perfectly suitable to me!).

Anyway, this is now documented at https://node-postgres.com/api/pool/#pool.connect.