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-19 05:25:49
Message-ID: 5715C15D.4020505@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016/04/19 13:59, Michael Paquier wrote:
> On Tue, Apr 19, 2016 at 1:14 PM, Etsuro Fujita
> <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> What do you think about that?

> + /* Wait for the result */
> + res = pgfdw_get_result(conn, query);
> + if (res == NULL)
> + pgfdw_report_error(ERROR, NULL, conn, false, query);
> + last_res = res;
> +
> + /*
> + * Verify that there are no more results
> + *
> + * We don't use a PG_TRY block here, so be careful not to throw error
> + * without releasing the PGresult.
> + */
> + res = pgfdw_get_result(conn, query);
> + if (res != NULL)
> + {
> + PQclear(last_res);
> + pgfdw_report_error(ERROR, res, conn, true, query);
> + }
>
> 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...

Thanks for the comment!

Best regards,
Etsuro Fujita

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-04-19 05:42:24 Re: VS 2015 support in src/tools/msvc
Previous Message Michael Paquier 2016-04-19 05:23:39 Re: snapshot too old, configured by time