Greg Sabino Mullane <[email protected]> writes: > Thanks for looking into this. I mostly agree with your analysis. > > In my opinion, at the very least, pg_db_cancel should be changed to conform >> to the documented PQgetResult protocol so that it not only >> blocks until the running query (if any) has terminated but also gets rid >> of the null pointer indicating this. > > That seems sensible. Would be good to see how other clients do this. I know > there was a reason for us not doing multiple calls here but I cannot recall > what it was now (it's been a decade plus). > > IMHO, a better idea would be to get rid of the PQgetResult call in >> pg_db_cancel altogether and document that >> a client must still determine the result of the running command (which may >> be "running command was cancelled") in a suitable way after invoking >> pg_cancel. Problem: Might break existing code which assumes that pg_cancel >> is a synchronous operation. > > I'm not a fan of this solution. It seems to push a little too much onto the > client. Presumably, if they call pg_cancel they really want to cancel and > thus give up on anything else in progress. At which point, we might as well > clean up behind them, right? (which, per the earlier point, we are not > doing well)
There might be 'elses' in progress which have absolutely nothing to with Postgres. The reason for using the async API will usually be that this is part of a program structured around a select/ poll/ epoll/ younameit event loop wich can have outstanding request on any number of file descriptors connecting to any number of different external and internal entites which doesn't want to be blocked waiting for input to become available on any particular file descriptor. That's also the use case the Postgres async API was designed to support. The existing pg_db_cancel implementation makes this impossible. And this essentially renders the whole async API moot. If the client is happy with being blocked until the server has replied (if ever happens), the synchronous interface could have been used to begin with. As an added benefit, the complete code handling 'async query cancelled state' could be removed. Clients using the async API need to handle query results arriving asynchronously, anyway. NB: This is more-or-less for theoretical consideration. As such a change might well break existing code, it would IMHO be very ill advised to do this in a stable driver which has exhibited this behaviour for a long time. I'm in the process of implementing this for a forked version using a different name (PgAsync), although I cannot yet tell for how long I can continue with putting as much work into this as I'm presently doing.
