Re: Bypassing cursors in postgres_fdw to enable parallel plans

From: Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: 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-07-28 09:51:14
Message-ID: CA+FpmFeZ3aqcFBorEswBV-n4bYWhbRk-aE4K7j-L_V=h2eyYaw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, 28 Jul 2026 at 14:30, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Thu, Jul 23, 2026 at 6:03 AM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
> wrote:
> >> I think you can add a parameter to pgfdw_cancel_query and
> >> pgfdw_cancel_query_end, a PgFdwConnState. The callers in connection.c
> >> have a ConnCacheEntry and can pass &entry->state, and the callers in
> >> postgres_fdw.c have a PgFdwScanState and can pass fsstate->conn_state.
> >>
> > Yes, you are right we can do that. However, looking at it more closely,
> zeroing the conn_state which includes pendingAreq doesn't look right.
> Because in pgfdw_abort_cleanup, when the check is done for pendingAreq, it
> would always be false, since we have cleared it already. So, wouldn't
> doing active_scan = NULL in pgfdw_cancel_query_end make more sense here
> rather than zeroing...?
>
> I'm not entirely sure I understand the question. I'd generally
> recommend clearing individual fields of a structure over using memset
> to zeroing the entire thing. On that note, I wonder if the memset that
> the current code uses in pgfdw_abort_cleanup() is buggy -- how can it
> justify zeroing the *entire* state entry? I guess maybe that works
> today because the only member of PgFdwConnState is pendingAreq, but it
> doesn't seem like the right thing in general.
>
> The question was that we should just clear active_scan and not the struct
memset the way it is doing in pgfdw_abort_cleanup, which you clarified in
your answer.

Regarding your other point, that's my understanding too that this memset is
working because it is the only one in struct.
Should we also change that to clearing only pendingAreq instead of the
complete struct memset that it is doing now? Should this be done as a
separate patch?

> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Richard Guo 2026-07-28 09:53:00 Re: bug: query returns different result with and without memoization.
Previous Message Yugo Nagata 2026-07-28 09:43:13 Re: enhance wraparound warnings