Re: BUG #17889: Invalid cursor direction for a foreign scan that reached the fetch_size (MOVE BACKWARD ALL IN cX)

On Sun, Apr 30, 2023 at 7:50 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> writes:
> >> I think that the root cause is in commit d844cd75a, which disallowed
> >> rewinding and then re-fetching forwards in a NO SCROLL cursor. I am
> >> not sure what to do about this issue, but I am wondering whether that
> >> commit is too restrictive, because 1) these examples would work just
> >> fine without that commit, and 2) we still allow
> >> rewind-and-fetch-forwards in a SCROLL cursor even when the query
> >> includes volatile functions.
>
> Well, the short answer here is that postgres_fdw is depending on
> something that has squishy semantics, for the reasons enumerated
> in d844cd75a (see also [1]).

Understood.

> Maybe we can carve out an exception
> that's narrow enough that we can convince ourselves it's not squishy,
> but I'm not very sure what the rules should be.

I think it would be good if we could relax the restriction on NO
SCROLL cursors so that postgres_fdw can rewind them as before, but I
am not sure we can. I think problematic cases arise when a cursor
query contains FOR UPDATE: as discussed in [1], the cursor would be
idempotent if later commands did not update tuples locked by the
query; but if they did, it would not be idempotent anymore, leading to
reintroducing an anomaly removed by that commit.

To fix, how about instead modifying postgres_fdw so that if the remote
server is v15 or later, it just recreates a cursor when rewinding it
is needed, like the attached? If we had a way to check the
scrollability of a remote cursor created without the SCROLL option, we
could avoid doing so for scrollable cursors, but we do not have it, so
I modified it to recreate it in any case.

Best regards,
Etsuro Fujita