Scrollable cursors at the protocol level

Greetings,
I've been carrying the pq.protocol_cursor patch
https://www.postgresql.org/message-id/flat/CADK3HHKe1PA1U6aB5-7tWBQ0yZGgNvY7H=ECDD9955Pas_zx_Q(at)mail(dot)gmail(dot)com
for a while (most recent rev is the v4 I posted), which lets Bind carry
CURSOR_OPT_SCROLL/NO_SCROLL/HOLD so the resulting portal is scrollable.
That solves half the problem. The other half is that even with a scrollable
portal, Execute can only drive it forward. There's no way at the protocol
level to say "give me the previous 100 rows" or "rewind to absolute
position 200." Drivers that want scrollable behavior today have to fall
back to DECLARE...SCROLL CURSOR plus FETCH BACKWARD strings, which means
leaving the extended-query protocol and giving up parameter binding, plan
caching, and most of what makes Bind/Execute useful in the first place.

That doesn't feel like a good place to stop. The backend already does all
the work. PortalRunFetch in src/backend/tcop/pquery.c handles every
direction the SQL FETCH grammar supports, but no protocol message ever
reaches it. The Execute path in exec_execute_message (postgres.c around
line 2122) only ever calls PortalRun, which is forward-only. What I'd like
to do, and what I'd like to hear objections to before I write it up, is
extend the Execute message to carry a direction and use PortalRunFetch when
it's anything other than forward.

The wire format I have in mind would stay inside the pq.protocol_cursor
extension that the Bind patch already negotiates. When a client has the
extension on, an Execute message could carry two trailing fields after the
existing portal name and max_rows: an Int32 direction matching the
FetchDirection enum (0 forward, 1 backward, 2 absolute, 3 relative), and an
Int64 count. Count is signed because SQL FETCH count is signed, and I want
the protocol to be a thin pass-through to the same machinery rather than
its own thing with its own quirks. Detection is by message length, the same
way the Bind extension currently does it. Without the extension negotiated,
trailing bytes on Execute are a protocol violation. With the extension on
but no trailing bytes, the message keeps its existing meaning — forward
fetch up to max_rows.

The interaction between the legacy max_rows field and the new count field
is the first thing I'm not certain about. The cleanest answer is to ignore
max_rows whenever the new fields are present and document it, because the
new field exists specifically to express richer semantics than max_rows
can. The icker alternative is to error when both are non-zero. I'd rather
not do that — having the extension turned on and trailing bytes present is
enough signal — but I can see the argument the other way.

Backward fetch on a non-scrollable portal already throws "cursor can only
scan forward" from PortalRunFetch's existing path, so that error surfaces
unchanged. Backward fetch on a portal whose strategy isn't ONE_SELECT,
ONE_RETURNING, ONE_MOD_WITH, or UTIL_SELECT (the four strategies
PortalRunFetch's switch handles) needs a similar check in the wrapper. In
practice I don't think clients will Bind queries that produce other
strategies and then ask for scrollable execution, but the guard should be
there.

The atStart/atEnd flags on Portal need to keep working across reverse
fetches, and PortalRunFetch already maintains them, so the execute_is_fetch
detection at postgres.c:2239 (which is just checking !portal->atStart)
stays correct. Holdable portals are the more interesting case:
CURSOR_OPT_HOLD materializes the result at commit, after which the portal's
strategy effectively becomes UTIL_SELECT for fetch purposes, and
PortalRunFetch handles that path too. I want tests for both, but especially
for the holdable case across a commit, because that's the scenario where
today's Execute-only model is most obviously insufficient and where a
protocol-level scrollable cursor offers the most over the DECLARE/FETCH
workaround.

For libpq, two new entry points fall out fairly naturally. A
PQexecutePortalEx that takes a direction and a count, and a
PQsendQueryScrollable convenience that does the Bind+Execute+Sync sequence
with cursor options. Direction constants in libpq-fe.h. None of this is
exotic; the Bind side of the v4 patch already adds parallel API surface for
the cursor options, and this is the same shape on the Execute side.

The pre-emptive pushback I'd like to address: yes, you can do all of this
with DECLARE...SCROLL CURSOR FOR ... and FETCH BACKWARD. JDBC drivers know
how. But that path requires a textual SQL round-trip for each fetch,
doesn't compose with Bind parameters, doesn't share plan caching with
extended-protocol queries, and forces drivers into two parallel
implementation strategies, one for forward-only result sets and one for
scrollable ones. pgjdbc's TYPE_SCROLL_INSENSITIVE today buffers the entire
result on the client because going to the server via DECLARE/FETCH is too
painful at the API level. A protocol-level scrollable cursor lets drivers
stop buffering and lets the server's existing materialization do its job.

A few things I haven't fully thought through. First, mid-fetch error
reporting: if the client asks for 100 rows and the cursor only has 30 left
in the requested direction, we send 30 DataRows and a CommandComplete with
the actual count, the way SQL FETCH does. I think the existing
PortalRunFetch return value gives us enough to form the right tag, but I
want to verify on a real run.

Second, PgBouncer in transaction-pool mode. I don't think it's a problem —
a portal lives within a transaction unless WITH HOLD is set, and within a
transaction the same backend handles all the messages — but anyone with
PgBouncer scars from this area is welcome to weigh in.

Third, and this is the question I'd most like an opinion on: should this be
a separately-negotiated extension from the Bind cursor-options work, or
live inside the same pq.protocol_cursor namespace? I lean toward keeping
them together. Making a portal scrollable at Bind matters only if you can
drive it scrollably at Execute, and splitting the negotiation creates a
state where one half of the feature is enabled and the other half isn't,
which doesn't seem useful. But putting two features under one extension
flag is also slightly unusual, so I want to surface the question rather
than just decide it.

If folks want to look at code rather than prose, I can push the Bind half
(the v4 patch, rebased) and a sketch of the Execute side to a branch. I'd
rather argue about the wire format now than have to redo it later.

Dave Cramer