Re: Speed dblink using alternate libpq tuple storage

On Thu, Feb 16, 2012 at 02:24:19PM +0900, Kyotaro HORIGUCHI wrote:
> As far as I see, on an out-of-memory in getAnotherTuple() makes
> conn->result->resultStatus = PGRES_FATAL_ERROR and
> qpParseInputp[23]() skips succeeding 'D' messages consequently.
>
> When exception raised within row processor, pg_conn->inCursor
> always positioned in consistent and result->resultStatus ==
> PGRES_TUPLES_OK.
>
> The choices of the libpq user on that point are,
>
> - Continue to read succeeding tuples.
>
> Call PQgetResult() to read 'D' messages and hand it to row
> processor succeedingly.
>
> - Throw away the remaining results.
>
> Call pqClearAsyncResult() and pqSaveErrorResult(), then call
> PQgetResult() to skip over the succeeding 'D' messages. (Of
> course the user can't do that on current implement.)

There is also third choice, which may be even more popular than
those ones - PQfinish().

> To make the users able to select the second choice (I think this
> is rather major), we should only provide and export the new PQ*
> function to do that, I think.

I think we already have such function - PQsetRowProcessor().
Considering the user can use that to install skipping callback
or simply set some flag in it's own per-connection state,
I suspect the need is not that big.

But if you want to set error state for skipping, I would instead
generalize PQsetRowProcessorErrMsg() to support setting error state
outside of callback. That would also help the external processing with
'return 2'. But I would keep the requirement that row processing must
be ongoing, standalone error setting does not make sense. Thus the name
can stay.

There seems to be 2 ways to do it:

1) Replace the PGresult under PGconn. This seems ot require that
PQsetRowProcessorErrMsg takes PGconn as argument instead of
PGresult. This also means callback should get PGconn as
argument. Kind of makes sense even.

2) Convert current partial PGresult to error state. That also
makes sense, current use ->rowProcessorErrMsg to transport
the message to later set the error in caller feels weird.

I guess I slightly prefer 2) to 1).

--
marko