Re: Optimization for updating foreign tables in Postgres FDW

On Mon, Apr 11, 2016 at 11:30 AM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> I agree with Rushabh.  Thanks for updating the patch!

Yes, not using a PG_TRY/CATCH block is better. We are not doing
anything that need to clean up PGresult in case of an unplanned
failure.

> Another thing I'd like to propose to revise the patch is to call
> pgfdw_report_error in the newly added hunk, with the clear argument set to
> *false*.  The PGresult argument is NULL there, so no need to release the
> PGresult.

Sure, this saves a couple of cycles. PQclear is anyway smart enough to
handle NULL results correctly, but this way is better.

> Attached is an updated patch.

+       if (wc & WL_SOCKET_READABLE)
+       {
+           if (!PQconsumeInput(dmstate->conn))
+               pgfdw_report_error(ERROR, NULL, dmstate->conn, false,
+                                  dmstate->query);
+       }
OK, so here we would fail with ERRCODE_CONNECTION_FAILURE in the case
where the socket is readable but no data can be consumed. I guess it
makes sense.

+                       if ((cancel = PQgetCancel(entry->conn)))
+                       {
+                           PQcancel(cancel, errbuf, sizeof(errbuf));
+                           PQfreeCancel(cancel);
+                       }
Wouldn't it be better to issue a WARNING here if PQcancel does not succeed?
-- 
Michael