On Mon, Apr 11, 2016 at 11:30 AM, Etsuro Fujita
<fujita.ets...@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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
