Re: Proposal to allow setting cursor options on Portals

> It's looking much more finished

Thanks for the review, and sorry for the long delay in getting back to
it. Attached is v5, rebased onto master and split into three patches:

v5-0001 the Bind side, as before but with your review comments
applied
v5-0002 the Execute side: a fetch direction and count
v5-0003 libpq support for the new Execute fields

Co-Authored by Sami Imseih.

> This made me realize that, adding the Bind side of cursors is only
> half of the equation. The "Execute" message should also gain new
> behaviour to support all the same functionality as FETCH and MOVE.

Agreed, and 0002 and 0003 do that, so the extension keeps the name
_pq_.cursor rather than being narrowed to _pq_.cursor_bind. A portal
created with SCROLL can now be read backwards over the protocol, which
is what makes the Bind side worth having in the first place.

> I think we can do that fairly easily by adding similar flags to
> Execute. I think we'd need three flags:
> 1. MOVE
> 2. BACKWARD
> 3. ABSOLUTE

I went with a slightly different encoding: the low three bits of the
new Int32 are a direction field rather than independent flags, with
MOVE as a separate bit on top:

0x0000 no fetch direction
0x0001 FORWARD
0x0002 BACKWARD
0x0003 ABSOLUTE
0x0004 RELATIVE
0x0008 MOVE, i.e. discard the rows instead of returning them

Three independent flags can't express FETCH RELATIVE, which is the one
direction a client needs in order to skip n rows without knowing where
in the result it currently is. It also admits combinations that have
no meaning, such as BACKWARD|ABSOLUTE, which the server would then have
to either define or reject; a direction field has no invalid states
inside the mask.

The count is a new Int64 rather than a reuse of the existing maximum
row count, because the two disagree about zero: a maximum row count of
0 means "no limit", while FETCH FORWARD 0 means "re-fetch the current
row". ABSOLUTE also wants more than 32 bits of range. A count of
INT64_MAX means all remaining rows; the server-internal FETCH_ALL is
LONG_MAX, which is not portable enough to put on the wire.

One thing I would like your opinion on. Both new fields are mandatory
once the extension is negotiated, rather than an optional trailer that
the server detects from the message length. Length sniffing does not
survive a second extension that also wants to append to Execute, so it
seemed like a bad foundation to build on. To keep that from changing
any behaviour, an all-zero direction (0x0000) means "no fetch behaviour
requested": the existing maximum row count governs, exactly as it does
today, PortalSuspended is still possible, and PORTAL_MULTI_QUERY still
works. So an all-zero trailer is byte-for-byte equivalent to today's
Execute, which matters for drivers such as the JDBC driver that rely on
the maximum row count. A direction combined with a nonzero maximum row
count is contradictory, so the server rejects it instead of picking a
winner.

MOVE is implemented as well, using None_Receiver and reporting a MOVE
command tag; a fetch that does return rows reports the query's own tag
with the number of rows fetched, so "SELECT 2" rather than a synthetic
one.

> I think the "protocol_" part in _pq_.protocol_cursor is duplicative.
> The _pq_ part already indicates that it's a protocol option, so I'd
> leave that out.

Done, it's _pq_.cursor now. I left the libpq connection parameter as
protocol_cursor, since there the "protocol" part is what tells you it
is a protocol-level thing and nothing else in the name does. Happy to
rename it to cursor if you disagree.

> I think the Message Formats page should list the actual flag values
> that are valid. The protocol docs should not require you to look at
> the postgres source code.

The values are all in the protocol documentation now, in the
_pq_.cursor entry of the Supported Protocol Extensions table, and the
Bind and Execute entries under Message Formats link to it. I kept them
in one place rather than repeating them in three, but say the word if
you would rather see them spelled out inline under Message Formats.

> > /*
> > * Only override the default cursorOptions when the client has
> > * explicitly set flags. A value of 0 means no cursor options were
> > * requested, so keep the CreatePortal defaults.
> > */
>
> What is the difference between setting cursorOptions = 0 and the
> CreatePortal defaults?

Good question, and the answer is that they are not the same:
CreatePortal sets cursorOptions to CURSOR_OPT_NO_SCROLL, not to 0. The
old code was working around that with the guard you quoted, which also
had the effect of clearing NO_SCROLL as soon as any flag was set, so
sending WITH HOLD on its own produced a portal with neither scroll bit
and left it to the planner whether backward fetches would work.

v5-0001 drops the guard and simply adds to what CreatePortal already
set, with SCROLL displacing NO_SCROLL:

if (bind_ext_flags & PQ_BIND_CURSOR_SCROLL)
portal->cursorOptions = (portal->cursorOptions &
~CURSOR_OPT_NO_SCROLL) | CURSOR_OPT_SCROLL;
if (bind_ext_flags & PQ_BIND_CURSOR_NO_SCROLL)
portal->cursorOptions |= CURSOR_OPT_NO_SCROLL;
if (bind_ext_flags & PQ_BIND_CURSOR_HOLD)
portal->cursorOptions |= CURSOR_OPT_HOLD;

A portal is now scrollable when, and only when, the client asks for
SCROLL, so no client can come to depend on scrollability that the
planner happened to hand out. NO_SCROLL is accepted but never
necessary, and all bits set to 0 creates exactly the portal that a Bind
message without the extension creates.

> In my GoAway patchset I linked enabling the protocol extension to the
> user requesting protocol v3.2 (or higher).

I have kept it as an explicit opt-in for now, because of the mandatory
fields above: enabling it for every v3.2 connection would put 4 extra
bytes on every Bind and 12 on every Execute for clients that never
touch a cursor, and would oblige every driver that asks for v3.2 to
start sending them. That seems like the wrong default, but I don't
feel strongly about it if you think the extra bytes are not worth
worrying about.

> Let's use PQ_BIND_CURSOR_VALID_FLAGS here too instead of this magic
> number.

Done.

> I think it'd be better to put these tests in the libpq_pipeline test
> file. Then we can keep all the libpq tests together so they can share
> the helper logic.

Done, the separate libpq_protocol_cursor module is gone and all of the
tests are in libpq_pipeline now, sharing its helpers.

On the libpq side, 0003 adds two functions:

PQsendExecutePortal(conn, portalName, fetchFlags, count)
PQsendBindAndExecutePortal(conn, stmtName, ..., portalName,
cursorOptions, fetchFlags, count)

The second one creates the portal and fetches from it in a single
command, which saves a round of results when the first batch of rows is
wanted immediately. Each Execute is preceded by a Describe Portal,
because the data rows need a row description in the same command and
the description from the command that created the portal is long gone
by then.

Two notes on the patches themselves. 0002 on its own makes the new
Execute fields mandatory before libpq knows to send them in 0003, so it
does not pass the tests until 0003 is applied; I can move the small
libpq change that writes the all-zero fields down into 0002 if people
would rather each patch stood alone. With all three applied the
regression, isolation, libpq and libpq_pipeline suites are clean.

Cheers,

Dave Cramer

On Mon, 6 Apr 2026 at 05:37, Jelte Fennema-Nio <postgres(at)jeltef(dot)nl> wrote:

> On Wed, 25 Mar 2026 at 15:34, Dave Cramer <davecramer(at)gmail(dot)com> wrote:
> > Attached is v4 of the patch
> > Co-Authored by Sami Imseih
> >
> > Adds docs and test module
>
> It's looking much more finished
>
> > The portal can
> > later be operated on with cursor commands such as FETCH, MOVE,
> > and CLOSE.
>
> This made me realize that, adding the Bind side of cursors is only
> half of the equation. The "Execute" message should also gain new
> behaviour to support all the same functionality as FETCH and MOVE. I
> think we can do that fairly easily by adding similar flags to Execute.
> I think we'd need three flags:
> 1. MOVE
> 2. BACKWARD
> 3. ABSOLUTE
>
> I do realize the scope creep of this, but it feels that without
> addressing Execute we have a half-finished feature. That could be
> fine, but then I don't think we should call the option
> _pq_.protocol_cursor. Because that sounds like it solves the whole
> half-baked protocol-level cursor implemention that we currently have.
> Maybe _pq_.cursor_bind instead.
>
> I think the "protocol_" part in _pq_.protocol_cursor is duplicative.
> The _pq_ part already indicates that it's a protocol option, so I'd
> leave that out.
>
> > <symbol>PQ_BIND_CURSOR_SCROLL</symbol> (scroll),
> > <symbol>PQ_BIND_CURSOR_NO_SCROLL</symbol> (no scroll), and
> > <symbol>PQ_BIND_CURSOR_HOLD</symbol> (hold).
> > These are defined in <filename>libpq-fe.h</filename>.
>
> and
>
> > <para>
> > Bitmap set by protocol extensions.
> > </para>
>
> I think the Message Formats page should list the actual flag values
> that are valid. The protocol docs should not require you to look at
> the postgres source code.
>
> /*
> * Only override the default cursorOptions when the client has
> * explicitly set flags. A value of 0 means no cursor options were
> * requested, so keep the CreatePortal defaults.
> */
>
> What is the difference between setting cursorOptions = 0 and the
> CreatePortal defaults?
>
> > {"protocol_cursor", NULL, "0", NULL,
> > "Protocol-Cursor", "", 1,
> > offsetof(struct pg_conn, protocol_cursor)},
>
> In my GoAway patchset I linked enabling the protocol extension to the
> user requesting protocol v3.2 (or higher).
>
> > /* Reject any bits we don't recognize */
> > if (bind_ext_flags & ~0x0007)
>
> Let's use PQ_BIND_CURSOR_VALID_FLAGS here too instead of this magic number.
>
> > src/test/modules/libpq_protocol_cursor/libpq_protocol_cursor.c
>
> I think it'd be better to put these tests in the libpq_pipeline test
> file. Then we can keep all the libpq tests together so they can share
> the helper logic.
>