Re: Optimization for updating foreign tables in Postgres FDW

From: Etsuro Fujita <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Cc: Noah Misch <noah(at)leadboat(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Rushabh Lathia <rushabh(dot)lathia(at)gmail(dot)com>, Thom Brown <thom(at)linux(dot)com>, Stephen Frost <sfrost(at)snowman(dot)net>, Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Optimization for updating foreign tables in Postgres FDW
Date: 2016-04-21 08:22:03
Message-ID: 57188DAB.2080905@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016/04/19 14:25, Etsuro Fujita wrote:
> On 2016/04/19 13:59, Michael Paquier wrote:
>> But huge objection to that because this fragilizes the current logic
>> postgres_fdw is based on: PQexec returns the last result to caller,
>> I'd rather not break that logic for 9.6 stability's sake.

> IIUC, I think each query submitted by PQexec in postgres_fdw.c contains
> just a single command. Maybe I'm missing something, though.

>> A even better proof of that is the following, which just emulates what
>> your version of pgfdw_get_result is doing when consuming the results.
>> + /* Verify that there are no more results */
>> + res = pgfdw_get_result(fmstate->conn, fmstate->query);
>> + if (res != NULL)
>> + pgfdw_report_error(ERROR, res, fmstate->conn, true,
>> fmstate->query);
>> This could even lead to incorrect errors in the future if multiple
>> queries are combined with those DMLs for a reason or another.

> I'd like to leave such enhancements for future work...

On reflection, I'd like to agree with Michael on that point. As
mentioned by him, we should not lose the future extendability. And I
don't think his version of pgfdw_get_result would damage the robustness
of in-postgres_fdw.c functions using that, which was my concern. Sorry
for the noise.

Attached is an updated version of the patch, which modified Michael's
version of the patch, as I proposed in [1] (see "Other changes:"). I
modified comments for pgfdw_get_result/pgfdw_exec_query also, mainly
because words like "non-blocking mode" there seems confusing (note that
we have PQsetnonbloking).

Best regards,
Etsuro Fujita

[1] http://www.postgresql.org/message-id/5715B08A.2030404@lab.ntt.co.jp

Attachment Content-Type Size
pgfdw-interrupt-v5.patch text/x-patch 13.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message tushar 2016-04-21 08:31:50 Re: "parallel= " information is not coming in pg_dumpall for create aggregate
Previous Message Shay Rojansky 2016-04-21 06:19:21 Wire protocol compression