Re: Bypassing cursors in postgres_fdw to enable parallel plans

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.

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