Re: Bypassing cursors in postgres_fdw to enable parallel plans

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
Cc: Jelte Fennema-Nio <postgres(at)jeltef(dot)nl>, Yilin Zhang <jiezhilove(at)126(dot)com>, KENAN YILMAZ <kenan(dot)yilmaz(at)localus(dot)com(dot)tr>, Andy Fan <zhihuifan1213(at)163(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Bypassing cursors in postgres_fdw to enable parallel plans
Date: 2026-10-09 20:57:34
Message-ID: CA+TgmobcAQ6XZA3yjipTFxepVi-xfMLFz6c8BjqYO6kMN6Byhw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Sep 15, 2026 at 9:30 AM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com> wrote:
> The patches required a minor rebase.
> The latest patches attached.

If you enter drain_active_scan() with tuplestore_lost_rows = true, the
function will set it to false, which is wrong. I think that
drain_active_scan() should store the previous value of
tuplestore_lost_rows into a local variable just before setting
tuplestore_lost_rows = true, and restore that value in the places
where it previously stores false.

http://postgr.es/m/CA+Tgmoa-FXH9jZaxm039eWf8=nhnoBfDS=oeKfm50sbGEAZ58g@mail.gmail.com
pointed out that pgfdw_subxact_callback() needs to drain active scans
before calling RELEASE SAVEPOINT. It still doesn't. You should
probably add some transaction-control tests to the regression test
suite for postgres_fdw, so that these kinds of issues can't come back
once fixed.

drain_active_scan() switches to active_fsstate->batch_cxt while it's
executing, which causes allocations to be performed in the correct
memory context. But that only handles memory allocation, and
tuplestores also use files, which are controlled by the resource owner
mechanism, not the memory context mechanism. Any files created here
should end up owned by whatever the correct resource owner is for the
scan we're draining, but instead they end up using the resource owner
that's in effect at the time we need to perform the drain. You'll need
to save the resource owner and temporarily switch to it here as you do
for the memory context.

I attach Claude-written test cases for all three issues.

--
Robert Haas
Databricks

Attachment Content-Type Size
streaming_fetch_lost_rows.sql application/octet-stream 1.9 KB
streaming_fetch_release.sql application/octet-stream 1.2 KB
streaming_fetch_resowner.sql application/octet-stream 2.0 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2026-10-09 21:22:38 Re: POC: Unlocked path for GetSnapshotDataReuse
Previous Message Heikki Linnakangas 2026-10-09 20:52:32 Re: [PATCH] Discard aborted updaters when expanding a multixact