On Mon, Apr 11, 2016 at 9:45 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Mon, Apr 11, 2016 at 5:16 PM, Etsuro Fujita > <fujita.ets...@lab.ntt.co.jp> wrote: >> On 2016/04/11 12:30, Michael Paquier wrote: >>> >>> + 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? >> >> Seems like a good idea. Attached is an updated version of the patch. > > Thanks for the new version. The patch looks good to me.
I'm wondering why we are fixing this specific case and not any of the other calls to PQexec() or PQexecParams() in postgres_fdw.c. I mean, many of those instances are cases where the query isn't likely to run for very long, but certainly "FETCH %d FROM c%u" is in theory just as bad as the new code introduced in 9.6. In practice, it probably isn't, because we're probably only fetching 50 rows at a time rather than potentially a lot more, but if we're fixing this code up to be interrupt-safe, maybe we should fix it all at the same time. Even for the short-running queries like CLOSE and DEALLOCATE, it seems possible that there could be a network-related hang which you might want to interrupt. How about we encapsulate the while (PQisBusy(...)) loop into a new function pgfdw_get_result(), which can be called after first calling PQsendQueryParams()? So then this code will say dmstate->result = pgfdw_get_result(dmstate->conn). And we can do something similar for the other call to PQexecParams() in create_cursor(). Then let's also add something like pgfdw_exec_query() which calls PQsendQuery() and then pgfdw_get_result, and use that to replace all of the existing calls to PQexec(). Then all the SQL postgres_fdw executes would be interruptible, not just the new stuff. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers