Cursor in transaction deadlock

[k0nserv]

Hey,

Thanks for making this library, it's very nice!

I believe I've found a deadlock when a cursor is used in a transaction and queries are issued on the same transaction as the cursor is being iterated.

Reproduction

Postgres Versions: 15 and 17 postgres version: 3.4.5 (latest)

const postgres = require('postgres');

const pg = postgres('postgres://...');
(async () => {
    try {
        await pg.begin(async (sql) => {
            const cursor = sql`select * from generate_series(0, 1000)`.cursor(1);
            for await (const _ of cursor) {
                // If `sql` is replaced with `pg` here it all works.
                await sql`select 1`;
                console.log('Loop iterated');
            }
        });
    } catch (e) {
        console.error('Caught error:', e.message);
    }
})()
    .then(() => {
        console.log('Done');
        process.exit(0);
    })
    .catch((e) => {
        console.error('Caught error:', e.message);
    });

This will stall forever and never log Loop iterated or Done. The resulting Postgres connection state will be wait_event: ClientRead.

$ SELECT wait_event, wait_event_type, query, backend_type, state FROM pg_stat_activity;
ClientRead	Client	select * from generate_series(0, 1)	client backend	active

Cause

As I understand this the cause is that the cursor query is conceptually still running and thus the second query ends up waiting for the cursor query to complete. Even if the query was allowed to run I don't think the Postgres backend would accept the query at this time. The unnamed portal is suspended and bound to the cursor query, a new query would clobber the unnamed portal and break the cursor in the next iteration.

I don't fully understand the code, but adding this line console.dir({ full, queries, isFull: c.queue === full, fullQ: full._xs[0] }); after src/index.js:318 on b231b688 prints isFull: true. Presumably this means the query will be queued until the cursor is exhausted.

Further proof of this is the following

const postgres = require('postgres');

const pg = postgres('postgres://...');

(async () => {
    try {
        await pg.begin(async (sql) => {
            const cursor = sql`select * from generate_series(0, 1)`.cursor(1);
            for await (const _ of cursor) {
                // NB: no await
                sql`select 1`
                    .then((v) => console.log('Select result:', v))
                    .catch((e) => console.error('Select error:', e.message));
                console.log('Loop iterated');
            }
            console.log('After loop');
        });
    } catch (e) {
        console.error('Caught error:', e.message);
    }
})()
    .then(() => {
        console.log('Done');
        process.exit(0);
    })
    .catch((e) => {
        console.error('Caught error:', e.message);
    });

Which prints

Loop iterated
Loop iterated
After loop
Select result: Result(1) [ { '?column?': 1 } ]
Select result: Result(1) [ { '?column?': 1 } ]
Done

Solution ideation

Feel free to ignore this section, but here are my thoughts on solves.

Throw on query while iterating over a cursor

It would be great if an exception is thrown if you attempt to issue a query like this while in the process of iterating over a cursor. This seems better than the current silent stall.

Migrate to true postgres Cursors

Instead of using the unnamed portal and suspending it, it would be nice if actual Postgres cursors were used under the hood i.e. DECLARE CURSOR et.al. I played around a little with creating cursors like this and wrapping it in an AsyncIterator, but it's currently unergonomic for consumers because all the usual awesomeness of the library isn't available (have to rely on sql.unsafe, no argument interpolation etc).

Thanks for reading!