Re: Bypassing cursors in postgres_fdw to enable parallel plans

On Tue, 12 May 2026 at 14:56, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, May 11, 2026 at 7:03 AM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
> wrote:
> > Thanks Robert for the detailed review and some great suggestions for
> improving this patch. I went through the comments and the patch and here is
> the revised version. To summarize the changes done in this patch,
> > - as suggested, split it into two patches now, the first patch changes
> the cursor_exists flag to scan_in_progress, the second patch is the one
> with the rest of the changes to implement this fetch mechanism
> > - changed the GUC to server/tables level option called streaming_fetch.
> Still a boolean type. Open to better naming suggestions.
> > - refactored postrgesReScanForeignScan for a more readable code
> > Following the suggestions above, now, it handles the case for the
> backward cursor separately and exits the function straight from there. In
> case of close cursor, it checks if cursor or cursor-free mode is used, and
> executes different actions as per the mode. In cursor-free mode we end_scan
> and tuplestore. In the third case when we start scanning the tuples already
> fetched, things remain the same in both modes.
> > - added the changes to show the option in explain verbose output
> > - Added a lot more test cases to check the option with other options and
> also to cover more code-paths, particularly rescans and ending the query in
> between, error cases etc.
> >
> > There is one problem that remains here, the use of ExprContext. We need
> this when we are fetching the tuples for the active scan, but since this is
> only available in node, we can not have the one which was there at the time
> when active_scan was the fsstate, so at the moment it is using the econtext
> from the node. This doesn't seem correct to me, but to have the correct
> ExprContext we need the node, but it also doesn't seem totally right to
> have the pointer to node in ScanState for this scenario. Please let me know
> what could be a good way to handle this.
>
> The problem here is that save_to_tuplestore() has gotten itself into
> the business of resending the query when a node is rescanned and we
> haven't yet buffered the results anywhere. But it doesn't need to do
> that in the first place. If the query needs to be resent, it's not
> currently active on the connection, and then there's no need to do
> anything to free up the connection, so save_to_tuplestore() doesn't
> need to be called in the first place. The reason why you're having
> this problem is that postgresReScanForeignScan calls end_scan but
> end_scan does not clear the active_scan pointer. So then
> save_to_tuplestore() gets called if something else tries to use the
> connection, and to make that work, you did this.
>
> But the right solution is to make sure that the active_scan pointer is
> only non-NULL when there are actually results already on the wire that
> need to be drained. If you do that, then save_to_tuplestore() won't
> get called if rescan has already read all of the pending results off
> the wire, and then you can delete the code that resends the query, and
> then you won't need an econtext here in the first place.
>
> Thanks Robert for the quick follow up on this. I found this to be true and
modified the patch accordingly.
Please find the updated patches attached. In the first patch jnothing is
changed, just adding it here for the sake of completeness.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH