| From: | Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com> |
|---|---|
| To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
| Cc: | 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-01-26 11:44:23 |
| Message-ID: | CA+FpmFd7TDGAsA0rAGGch=MmWNHvvK0YS4zs-4KDPXRCLVxgNQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, 10 Dec 2025 at 18:44, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>
>
> Overall, I think the direction of the patch set has some promise, but
> I think it needs a lot of cleanup: removal of unnecessary code, proper
> formatting, moving variables to inner scopes, explanatory comments,
> good names for variables and functions and structure members, removal
> of unnecessary files from the patch, cleanup of the regression test
> coverage so that it doesn't add more bloat than necessary, proper
> choice of data structures, and so on. Right now, the good things that
> you've done here are being hidden by these sorts of mechanical issues.
> That's not just an issue for me as a reviewer: I suspect it's also
> blocking you, as the patch author, from finding places where the code
> could be made better. Being able to find such opportunities for
> improvement and act on them is what will get this patch from
> "interesting proof of concept" to "potentially committable patch".
>
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
>
Thanks Robert for your time and attention to this patch.
Based on these review comments and an off list discussion about the design
of the patch, I have reworked the patch significantly.
In this version, a tuplestore is added to the PgFdwScanState along with
a flag. Now, in case of a cursor switch, this tuplestore is filled with the
remaining tuples of the query. The corresponding flag is set to indicate
that the tuplestore is ready to be fetched. To remember the last query that
was executing, a pointer to its PgFdwScanState is maintained in the
conn_state. Note that we only need to remember just the very last query and
once tuples for it are fetched we can forget that pointer, because now its
fsstate has the remaining tuples in the tuplestore.
So, this version of the patch looks simpler than before.
A few test cases are also added in the attached patch to cover the
different scenarios in case of non-cursor mode.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH
| Attachment | Content-Type | Size |
|---|---|---|
| v5-0001-Fetch-without-cursors.patch | application/octet-stream | 55.2 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-01-26 11:49:01 | Re: DOCS - "\d mytable" also shows any publications that publish mytable |
| Previous Message | Jelte Fennema-Nio | 2026-01-26 10:50:34 | Re: Safer hash table initialization macro |