Re: Speed dblink using alternate libpq tuple storage - Mailing list pgsql-hackers

Marko Kreen <markokr@gmail.com> writes:
> The fact remains that upper-level code must cooperate with callback.
> Why is it useful to hijack PQgetResult() to do so?

Because that's the defined communications channel.  We're not
"hijacking" it.  If we're going to start using pejorative words here,
I will assert that your proposal hijacks PQisBusy to make it do
something substantially different than the traditional understanding
of it (more about that below).

> Seems our concept of "async mode" is different.  I associate
> PQisnonblocking() with it.

Well, there are really four levels to the API design:
* Plain old PQexec.* Break down PQexec into PQsendQuery and PQgetResult.* Avoid waiting in PQgetResult by testing
PQisBusy.*Avoid waiting in PQsendQuery (ie, avoid the risk of blocking  on socket writes) by using PQisnonblocking.
 

Any given app might choose to run at any one of those four levels,
although the first one probably isn't interesting for an app that would
care about using a suspending row processor.  But I see no reason that
we should design the suspension feature such that it doesn't work at the
second level.  PQisBusy is, and should be, an optional state-testing
function, not something that changes the set of things you can do with
the connection.

> IMHO it's much better to remove the concept of early-exit from public
> API completely and instead give "get" style API that does the early-exit
> internally.

If the row processor has an early-exit option, it hardly seems
reasonable to me to claim that that's not part of the public API.

In particular, I flat out will not accept a design in which that option
doesn't work unless the current call came via PQisBusy, much less some
entirely new call like PQhasRowOrResult.  It's unusably fragile (ie,
timing sensitive) if that has to be true.

There's another way in which I think your proposal breaks the existing
API, which is that without an internal PQASYNC_SUSPENDED state that is
cleared by PQgetResult, it's unsafe to probe PQisBusy multiple times
before doing something useful.  That shouldn't be the case.  PQisBusy
is supposed to test whether data is ready for the application to do
something with --- it should not throw away data until a separate call
has been made to consume the data.  Changing its behavior like that
would risk creating subtle bugs in existing event-loop logic.  A
possibly useful analogy is that select() and poll() don't clear a
socket's read-ready state, you have to do a separate read() to do that.
There are good reasons for that separation of actions.

> Again, note that if we would provide PQrecvRow()-style API, then we *don't*
> need early-exit in callback API.  Nor exceptions...

AFAICS, we *do* need exceptions for dblink's usage.  So most of what's
at stake here is not avoidable, unless you're proposing we put this
whole set of patches off till 9.3 so we can think it over some more.
        regards, tom lane