Equivalent of Pool.query for Client

[rightaway]

People who are using external connection pooling like pgbouncer use the Client api rather than Pool. But Client has no equivalent to the convenient Pool.query. Documentation says

If you don't need a transaction or you just need to run a single query, the pool has a convenience method to run a query on any available client in the pool. This is the preferred way to query with node-postgres if you can as it removes the risk of leaking a client.

Instead we have to do Client.connect then Client.query then Client.end. There's a risk of leaking a client with Client that doesn't exist when using Pool.query. If an equivalent were provided it would be much better than everyone writing their own helper function that does this.

Since the name Client.query is already taken, it could have a different name like Client.single.

[sehrope]

Something like this should be on the top level import, not on the Client.

Maybe something like withClient(config, task) would be useful as well if you want to perform multiple things with the same client (ex: a transaction) and then make sure it's closed.

const pg = require('pg');
const { withClient } = pg;

await withClient(opts, async (client) => {
    await client.query('....');
    await client.query('....');
    await client.query('....');
    return 'some result';
})

Then the single query version could use that generic one to run one query and return it's result:

const runOneQuery = (opts, sql, params) => {
    return pg.withClient(opts, (client) => client.query(sql, params))
}

[rightaway]

Why shouldn't it be on Client?

Client would let you set all the connection parameters, then anywhere you need to call it you just run something like client.single(query, args). Rather than needing to pass the opts in each time. Why not the simplest possible api, like Pool offers?

[sehrope]

Ah, you're referring to having it on a client instance (lowercase "c"). Having it on the instance would cause issues due to how connection state is tracked internally.

I read the original comment to imply it was a static function on the class, Client. Seemed to me it'd be better to keep the Client class focused on just being a client and having something that aids in managing it's state (like Pool is now) separate.

Rather than adding top level functions, how about adding a new top level sibling to Pool that does not do any pooling but maintains a similar API. Could be a drop-in replacement for user's that do not want any app level pooling.

Something like this (NOTE: not tested!):

class ZeroPool {
    constructor(opts) {
        // Save connection settings
        this.opts = opts;
    }

    connect = async () => {
        const client = new Client(this.opts);
        await client.connect();
        // Mimic pool's release() but actually end connection:
        client.release = () => {
            return client.end();
        }
        return client;
    }

    query = async (sql, params) => {
        // Outside the try block as we don't need to end a failed connect
        const client = await this.connect();
        try {
            // Must await result so that finally block executes after this
            const result = await client.query(sql, params);
            // If we get here then query was successful
            // ... otherwise the query error have been thrown
            return result;
        } finally {
            try {
                await client.end();
            } catch (ignore) {
            }
        }
    }

    end = () => {
        // no-op
    }
}

[rightaway]

That looks great! Could it be added to an upcoming version?

I'm assuming connect in that example is meant to behave like it's private and nobody should use it?

[charmander]

@rightaway Its connect can be public and used just like a Pool’s, where acquiring a client from the ZeroPool always means connecting a new one and releasing a client to the ZeroPool always means disconnecting it.

[rightaway]

@charmander If you needed to use connect to acquire a new client wouldn't you use the existing Client class instead of ZeroPool? I'm unsure on when you'd want to use connect in ZeroPool and acquire a client.

[sehrope]

@rightaway It's mean to be a drop-in replacement for environments where you do not want to using app level pooling. If you have existing code that uses pool.connect(...) to take control of a connection, say to manage a transaction, then you could use this in its place with no code change besides the pool switch.

[rightaway]

Will .on('error') callback be provided for people who will use this new feature, or is that just for pg.Pool?

[rightaway]

@sehrope Could this feature be added? Will be very useful for people who are using pgbouncer and have to write their own, potentially buggy implementations of managing transactions. Pool.query is great for this, but there should be a way to people not using pooling at the application level to get the same benefits that Pool.query gives.

[sehrope]

@rightaway I guess it's common enough in Lambda style settings that it'd be useful.

@charmander What do you think? Should I open a PR for it?

[charmander]

Well, I’d make a new package – but it’s worth asking @brianc!

[rightaway]

When I import pg I get pg-pool automatically and don't need to import it. If this becomes a new package will it also not need to be explicitly imported, since it's an alternative to pg-pool?

[rightaway]

@charmander When you make a single Pool.query call, does node-postgres have to make at least 3 round trip calls behind the scenes for the BEGIN, the query and the COMMIT/ROLLBACK like it has to for Client.query? https://node-postgres.com/features/transactions#a-pooled-client-with-asyncawait

Or does it batch them #1190 (comment) to avoid round trips that aren't needed?

[charmander]

@charmander When you make a single Pool.query call, does node-postgres have to make at least 3 round trip calls behind the scenes for the BEGIN, the query and the COMMIT/ROLLBACK like it has to for Client.query? https://node-postgres.com/features/transactions#a-pooled-client-with-asyncawait

Or does it batch them #1190 (comment) to avoid round trips that aren't needed?

Neither. There's no need to send BEGIN or COMMIT explicitly to make a single query in the PostgreSQL protocol. (There's also no need to do this explicitly if you're using Client#query without wanting a transaction.)

[brianc]

If this becomes a new package will it also not need to be explicitly imported, since it's an alternative to pg-pool?

yep. And that's how I'd prefer it to be. It's too custom to your use case to be something we roll into the core of pg. As far as I understand it you want something like:

export async function query(connectionArgs, queryText, queryParams) {
  await client.connect(connectionArgs)
  try {
    return client.query(queryText, queryParams)
  }
  finally {
    client.end()
  }
}

This is p small & you should really create a layer between your application and the database anyway and not sprinkle direct calls to postgres everywhere as it makes it harder to add error handling, logging, other custom decorator stuff. You can add something like this in that layer.

Ultimately I think this would be better served in a separate module as it solves a particular use case & niche and could have tests written against pg-bouncer for that module's CI and better documentation and explanation and all that.

[rightaway]

Neither. There's no need to send BEGIN or COMMIT explicitly to make a single query in the PostgreSQL protocol. (There's also no need to do this explicitly if you're using Client#query without wanting a transaction.)

@charmander So if I do client.query("select * from function1('value1', 'value2'); select * from function2('value1', 'value2');", is there only 1 round trip to the database in total for both function calls?

Are function1 and function2 run in 2 separate transactions which have an implicit BEGIN and COMMIT around each one?

[charmander]

So if I do client.query("select * from function1('value1', 'value2'); select * from function2('value1', 'value2');", is there only 1 round trip to the database in total for both function calls?

Yes.

Are function1 and function2 run in 2 separate transactions which have an implicit BEGIN and COMMIT around each one?

They're run in two separate transactions, yes. (Or I'm pretty sure, at least. You can double-check with SELECT txid_current(); SELECT txid_current();.)

[rightaway]

They're run in two separate transactions, yes. (Or I'm pretty sure, at least. You can double-check with SELECT txid_current(); SELECT txid_current();.)

@charmander Actually I tried it with Pool and plain Client and all statements passed in 1 string are run in the same transaction. Will be good if it can be added to documentation to prevent confusion.

Do client.connect() and client.end() below make round trips to the database? Can I remove client.connect() from here and do it just once on application startup so that I don't need to make that round trip on every query? Just like how Pool.query doesn't need that extra round trip on every query because Pool.connect is only run once on application startup.

export async function query(connectionArgs, queryText, queryParams) {
  await client.connect(connectionArgs)
  try {
    return client.query(queryText, queryParams)
  }
  finally {
    client.end()
  }
}

[charmander]

Do client.connect() and client.end() below make round trips to the database?

Yes.

Can I remove client.connect() from here and do it just once on application startup so that I don't need to make that round trip on every query?

Yes. You can't connect the same Client instance more than once, note. I recommend a (max: 1) pool, which will also manage reconnecting if your client gets disconnected.

[rightaway]

@charmander If I stick with Pool instead of Client is there a way to use LISTEN/NOTIFY? How can you create one listener when the application starts and keep it running the whole time the application is running? The docs only show how to do it with Client. Can you give an example of how it should look if you're using Pool?

[charmander]

@rightaway Ah, no, if you're using LISTEN you’ll need to manage reconnection on your own.