Re: postgres_fdw: transaction mode inheritance corner cases

On Fri Oct 16, 2026 at 9:10 AM -03, Etsuro Fujita wrote:
>> 2: stale ForeignScanHash entries after a failed portal
>>
>> An entry is removed only by postgresEndForeignScan or when the
>> subtransaction that created the scan aborts (or at transaction end). But
>> if an error happens while the scan of an outer-level cursor is fetched
>> inside a savepoint, the portal is marked failed and the executor doesn't
>> call EndForeignScan, so the entry is left pointing to memory that is
>> freed later. See it:
>
> I couldn't reproduce this in my environment. Could you provide the
> reproducer in more detail including the foreign table definition, the
> postgres_fdw options, ...?
>
create extension postgres_fdw;
create server loopback foreign data wrapper postgres_fdw options (dbname 'postgres', port '5432');
create user mapping for current_user server loopback;
create table t (a int);
insert into t values (1), (2), (3), (4);
create foreign table ft (a int) server loopback options (table_name 't');

begin;
declare c cursor for select 1 / (a - 3) from ft order by a;
fetch 1 from c;
savepoint s2;
fetch 5 from c;
rollback to s2;
commit;

After the COMMIT command it produces:

WARNING: 01000: ForeignScan node 0x106019368 still existing
LOCATION: AtEOXact_ForeignScan, postgres_fdw.c:4381

The details that matter are that the first "fetch 1" happens before the
savepoint, so IIUC the scan and its cursor belong to the outer level,
and that the error happens while fetching inside the savepoint. The
expression 1 / (a - 3) is not pushed down, so the division by zero is
raised locally while the scan returns rows, and the portal is left in
the failed state. If I add this after "rollback to s2" and before
"commit":

close c;
savepoint s3;
select 1;
release s3;

the backend crashes in the subtransaction callback.

> Thanks for working on the fix! It would be great if you share it.
>

I'm attaching the v4 patch. It is your v3-0001 plus the fixes from my
previous email that you agree. Since you said you'd remove the
restrictions of 3 and 4, and add the comments of 6, I went ahead and did
them in this version. A few notes:

- for 1, the error no longer uses the relation name when fsstate->rel is
NULL.
- for 2, every scan registers a memory context reset callback on its
executor memory context, which removes its hash entry when that memory
is freed, even if EndForeignScan is never called. I also removed the
WARNING about leftover scans at commit, since a failed portal can
legitimately leave one behind at that point. Please let me know if you
disagree with that.
- for 3 and 4, scan_allowed is replaced by a cursor_lost flag: when a
savepoint rollback removes the remote cursor, the scan can still
return the rows that are already in its buffer, and the error is only
raised when a new remote fetch is needed. A rescan of such a scan
either reuses what it has in memory or forgets the cursor so that the
next fetch creates a new one.
- for 5, I kept ERRCODE_FEATURE_NOT_SUPPORTED as you noted, and only
improved the message.
- for 6, I added the comments, and while doing it I dropped fs_level,
since with the reset callback it wasn't needed anymore: the hash only
holds the scan pointer and all the level tracking is in cursor_level.
- I added tests for the pushed-down join, the failed portal case, a
small fetch_size where more rows can't be fetched, the rescan of a
scan with a lost cursor, sibling and nested savepoints, Nikolay's
initplan case, a PL/pgSQL refcursor and an async scan.

--
Matheus Alcantara
EDB: https://www.enterprisedb.com