Allow setting client.pipeline = true|false if client is idle, otherwise throw

[justjake]

The new pipeline feature is great and preferable for typical postgres usage, however it's not compatible with some special case features like pg-cursor, pg-query-stream, pg-copy-streams, client.query({ text, rows: N }). This is a frustrating limitation for applications that already use these features alongside connection pooling: we must either use two pools, one with { pipeline: true } and one with { pipeline: false }, or use some other way to manage connections.

This PR provides an alternative: a simple and safe way to switch a client's client.pipeline property at runtime. This allows us to check out a client from a pool, disable pipeline mode if necessary, do our fancy business, and then restore pipeline before returning the client to the pool. This avoids adding complicated mode-switch queuing by pushing the complexity into userspace:

const client = await pool.connect()

try {
  const wasPipelined = client.pipeline

  await client.waitForIdle()
  client.pipeline = false

  try {
    const { rows } = await client.query({
      text: 'SELECT id FROM users',
      rows: 100,
    })

    console.log(rows)
  } finally {
    await client.waitForIdle()
    client.pipeline = wasPipelined
  }
} finally {
  client.release()
}

Specifically:

• client.isIdle(): boolean returns true when it's safe to change client.pipeline: there's no pending queries and all responses are drained.
• client.waitForIdle(): Promise<void> returns a promise that resolves when client.isIdle() is true.
• client.pipeline is now a setter that throws if changed when the client is not idle

[brianc]

Thanks for the PR! I can see the value in this; however, I wonder if the added complexity is better than just either opening a single client for "un-pipelineable" queries or keeping a separate pool around for those use-cases? Because as you said this pushes complexity out to the client application anyways, I'm not sure it's a massive gain from a UX perspective. More importantly from where I sit introducing more API surface on the client is more API surface I have to maintain forever or break it in a breaking change in a future version which I loathe to do. I think if there was absolutely no way to work around this without an API like this I'd be more partial to it, but considering you can always open another client if you need special cased things like streaming or copy streams or cursors...it might make more sense to push that complexity (that already exists, and still will after this lands) down to the client applications for now? @charmander interested in your thoughts as well.

[charmander]

@brianc yep, agreed

[justjake]

I don’t love it either - and I don’t plan to use pipeline mode myself anymore. After I put this PR up, I decided to switch to a custom Submittable so I can fully control when Sync messages are sent, so I can wrap my entire transaction in a single error boundary and avoid awaiting BEGIN. I left it open in case the code was helpful to the project or others online.

[brianc]

a custom Submittable so I can fully control when Sync messages are sent, so I can wrap my entire transaction in a single error boundary and avoid awaiting BEGIN

oh nice - i'd love to see the code for this if its open source! 😄