Skip to content

Equivalent of Pool.query for Client #1938

Closed

Description

@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.

Activity

  1. sehrope commented on Jul 29, 2019

    @sehrope
    Contributor

    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))
    }
  2. rightaway commented on Jul 31, 2019

    @rightaway
    Author

    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?

  3. sehrope commented on Jul 31, 2019

    @sehrope
    Contributor

    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
        }
    }
  4. rightaway commented on Aug 3, 2019

    @rightaway
    Author

    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?

  5. charmander commented on Aug 3, 2019

    @charmander
    Collaborator

    @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.

  6. rightaway commented on Aug 3, 2019

    @rightaway
    Author

    @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.

  7. sehrope commented on Aug 3, 2019

    @sehrope
    Contributor

    @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.

  8. rightaway commented on Mar 25, 2020

    @rightaway
    Author

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

  9. rightaway commented on Jul 20, 2021

    @rightaway
    Author

    @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.

  10. sehrope commented on Jul 21, 2021

    @sehrope
    Contributor

    @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?

  11. charmander commented on Jul 23, 2021

    @charmander
    Collaborator

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

  12. rightaway commented on Jul 24, 2021

    @rightaway
    Author

    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?

  13. rightaway commented on Sep 8, 2021

    @rightaway
    Author

    @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?

  14. charmander commented on Sep 8, 2021

    @charmander
    Collaborator

    @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.)

  15. brianc commented on Sep 8, 2021

    @brianc
    Owner

    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.

  16. rightaway commented on Sep 9, 2021

    @rightaway
    Author

    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?

  17. charmander commented on Sep 9, 2021

    @charmander
    Collaborator

    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();.)

  18. rightaway commented on Sep 10, 2021

    @rightaway
    Author

    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()
      }
    }
  19. charmander commented on Sep 10, 2021

    @charmander
    Collaborator

    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.

  20. rightaway commented on Sep 11, 2021

    @rightaway
    Author

    @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?

  21. charmander commented on Sep 11, 2021

    @charmander
    Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions