Re: postgres_fdw: transaction mode inheritance corner cases

On Sat, Oct 3, 2026 at 6:38 PM Matheus Alcantara
<matheusssilv97(at)gmail(dot)com> wrote:
> 1: the check in 0001 doesn't cover sibling savepoints
>
> Since created_at only keeps the nesting depth, a cursor declared in one
> savepoint and first fetched in a sibling savepoint at the same depth
> passes the check:
>
> begin;
> savepoint s1;
> declare c cursor for select * from ft;
> release s1;
> savepoint s2;
> fetch 1 from c;
> rollback to s2;
> fetch all from c;
> commit;
> ERROR: 34000: cursor "c1" does not exist
> CONTEXT: remote SQL command: CLOSE c1

As I said upthread, I noticed this after sending the patch.

> 2: it rejects cases that work on master
>
> A PL/pgSQL refcursor that is opened outside an exception block and first
> fetched inside it works on master, but fails with 0001 with the new
> error. It also fails if the exception handler swallows errors: the new
> error is hidden inside the block, the portal is left failed, and the
> later fetch outside the block fails with a confusing 'portal "<unnamed
> portal 2>" cannot be run'. On master this works because no remote
> savepoint exists yet, so the remote cursor is created at remote level 1.

This isn't great; I think we should support this as much as possible.

> I'm attaching a prototype 0003 on top of 0001 and 0002 that fixes both
> issues that I've mention. The idea is that the real problem is not the
> local nesting depth, but whether the remote savepoint depth at the time
> of the DECLARE is deeper than the level where the local cursor lives,
> since rolling back a remote savepoint at or below that depth destroys
> the remote cursor. So 0003 declares the remote cursor before
> synchronizing the remote savepoint level, and raises the error only if
> the current remote depth is deeper than the level of the local cursor.
> To know that level it records the subtransaction ID and the nesting
> level when the scan is created; if that subtransaction was already
> released, it conservatively assumes the cursor lives at the top level.

As demonstrated by Nikolay, the 0003 patch doesn't guarantee the
local/remote cursor-level synchronization, still causing an error:
"cursor "cN" does not exist", which isn't good. In addition to that,
I think the proposed handling using the subtransaction ID makes the
fetchability of remote cursors restrictive, reducing the compatibility
with the previous behavior than necessary. So sorry, I don't think
0003 is the right way to go.

I updated the 0001 patch to suppress the "cursor "cN" does not exist"
error while keeping the previous behavior as much as possible,
allowing cursors opened before a savepoint to be first fetched after
that savepoint. Attached is an updated patch. I'm wondering if the
patch should be master-only, though, because 1) it is rather
large/invasive and 2) there is no field report about this open-cursor
issue IIRC. Anyway I'll add more test cases next.

Best regards,
Etsuro Fujita