Clarify if queries are guaranteed to be serial in transactions

[phiresky]

I'm trying to find out whether it is necessary to await every single subquery in a transaction.

The transaction documentation has the following example:

  const client = await pool.connect()
  await client.query('BEGIN')
  await client.query(insertPhotoText, insertPhotoValues)
  await client.query('COMMIT')

which uses await for the inner query, even though the return value of that query is ignored. It is not clear if the await is required at that point:

  const client = await pool.connect()
  await client.query('BEGIN')
  client.query("update tbl set a = b");
  client.query("update tbl2 set c  = d");
  await client.query('COMMIT')

The documentation for client.query() also does not specify whether it is necessary to wait for the callback / promise to return before adding another query to the client. My reason for trying to do this this is avoiding a round trip to the server and clearing the JS stack for every single query.

Looking at the code, it seems like it should work fine because everything is synchronously pushed to a queue for the client, but it is not clear whether it is even possible to ignore the result value of a query.

[sehrope]

The commands are serialized per client but you should await each step along the way as otherwise you could end up with an unhandled promise rejection. If the UPDATE tbl ... fails due an error then your application would crash.

[boromisp]

@sehrope is right, you shouldn't have possible unhandled rejections, but there is a way to do what you want.

The proper incantations look something like this:

const client = await pool.connect();
try {
  await Promise.all([
    client.query('BEGIN'),
    client.query("update tbl set a = b"),
    client.query("update tbl2 set c  = d")
  ]);
  await client.query('COMMIT');
} catch (error) {
  await client.query('ROLLBACK');
  throw error;
} finally {
  client.release();
}

This way you can enqueue most of the queries ahead of time, and still catch every error.
The Promise.all all will reject as soon as any of its promises rejects, or resolve once all of its promises has been resolved.

You could even work around the fail-fast behavior of Promise.all — and sacrifice readability — to enqueue the COMMIT too:

const client = await pool.connect();
try {
  let error = null;
  const handleError = e => error = error || e;

  await Promise.all([
    client.query('BEGIN').catch(handleError),
    client.query("update tbl set a = b").catch(handleError),
    client.query("update tbl2 set c  = d").catch(handleError),
    client.query('COMMIT').catch(handleError)
  ]);
  if (error) throw error;
} finally {
  client.release();
}

[phiresky]

Wouldn't COMMIT also fail if one of the previous queries failed? At least thats what happens when running a sql file via piping to the psql client:

"DatabaseError: current transaction is aborted, commands ignored until end of transaction block"

This is what postgres does when a query produces an error and you try to run another query without first rolling back the transaction.

( https://stackoverflow.com/questions/2979369/databaseerror-current-transaction-is-aborted-commands-ignored-until-end-of-tra)

Which would mean it is enough to do

  const client = await pool.connect()
  await client.query('BEGIN');
  client.query("update tbl set a = b").catch(e => {});
  client.query("update tbl2 set c  = d").catch(e => {});
  await client.query('COMMIT')

(still without await, just preventing any unhandled promise rejections from going up all of the call stack)

[sehrope]

Yes and you would need to issue a ROLLBACK to reset it prior to reusing the connection. Otherwise a different piece of your app may get the invalid connection from the pool.

I think it's generally easier to have multiple await clauses vs wrapping things in Promise.all(...). Makes things look more synchronous.

To handle the out of sync connection issue the code should also attempt to issue a rollback if an error occurs at any point in the transaction including the final commit. If the rollback fails then the connection should be dropped. If it succeeds, then the connection can be returned to the pool. Alternatively you can discard any connection that has an error though this has a slight DOS issue if your app is a web app and a user can initiate a failed transaction through a bad request as it'll thrash your pool.

So something like this:

let txErr = null;
try {
    // Do transaction stuff
    await client.query('UPDATE ...');
    await client.query('UPDATE ...');
    await client.query('UPDATE ...');
    // Try to commit the transaction
    await client.query('COMMIT');
} catch(e) {
    // If we get here then *something* failed so try to rollback
    try {
        await client.query('ROLLBACK');
    } catch (e2) {
        // If we get here then the rollback failed as well so save the error so the conneciton is evicted from the pool
        txErr = e2;
    }
}
client.release(txErr);

[boromisp]

@phiresky: That should work. But the client.query('COMMIT') will resolve successfully event during a failed transaction, so if you need to know that happened, you have to save the error in the catch callbacks.

@sehrope: Could you give an example, when ROLLBACK would fail? As far as I know, Even if you are not in a transaction, it should only raise a warning (WARNING: there is no transaction in progress). Closed / aborted connections should already be handled by pg.Pool.

[phiresky]

Ok, so my first question (Is it allowed to call client.query multiple times, synchronously?) is answered.
Can we depend on this behaviour? The docs don't show any example where the next command is not in the callback or after awaiting the promise from what I see.

@boromisp Thank you. I just tried it, and sending COMMIT to a transaction with failing commands succeeds, but causes a rollback, and (await client.query('COMMIT')).command === 'ROLLBACK' even allows to check whether this is the case. But this behaviour is not mentioned in the PostgreSQL docs.

My goal was actually to not receive any response from the intermediary queries at all but this doesn't even seem possible in the postgresql protocol. But I can solve my problem by using update from values() instead of individual queries anyways so it's ok.

[sehrope]

@boromisp If the backend process on the server is killed or there is a network issue causing the client's TCP socket to close then the rollback would fail. The pool handles those situations when they happen for a connection that is in use but it's up to the user to handle it during use and instruct the pool to discard the client by passing in a non-null error object as a parameter to client.release().

[boromisp]

@phiresky: I have no idea weather it is in the SQL standard (or a documented extension of PostgreSQL), but it seems that COMMIT is replaced with ROLLBACK in aborted transactions on all versions I could test (8.3, 9.1, 9.6).

Back to your original question: I did not find any documentation for it, but all queries are pushed into a queue synchronously, and are processed in order. If the connection is "ready for query" when you call client.query(...), then the query is processed synchronously.

@sehrope: You are right. If I call client.release() on a failed connection after this event handler has been executed, the failed connection will be returned to the pool.
Depending on timing (and the Promise library used)

client.query('... aborted query ...')
  .catch(() => client.release())
  .then(() => client.release())

could either drop the failed client, or return it to the _idle queue. This I think is a bug of pg-pool, and might be related to #1454.