Re: Speed dblink using alternate libpq tuple storage

On Fri, Mar 30, 2012 at 05:18:42PM -0400, Tom Lane wrote:
> Marko Kreen <markokr(at)gmail(dot)com> writes:
> > On Wed, Mar 07, 2012 at 03:14:57PM +0900, Kyotaro HORIGUCHI wrote:
> >>> My suggestion - check in getAnotherTuple whether resultStatus is
> >>> already error and do nothing then. This allows internal pqAddRow
> >>> to set regular "out of memory" error. Otherwise give generic
> >>> "row processor error".
>
> >> Current implement seems already doing this in
> >> parseInput3(). Could you give me further explanation?
>
> > The suggestion was about getAnotherTuple() - currently it sets
> > always "error in row processor". With such check, the callback
> > can set the error result itself. Currently only callbacks that
> > live inside libpq can set errors, but if we happen to expose
> > error-setting function in outside API, then the getAnotherTuple()
> > would already be ready for it.
>
> I'm pretty dissatisfied with the error reporting situation for row
> processors. You can't just decide not to solve it, which seems to be
> the current state of affairs. What I'm inclined to do is to add a
> "char **" parameter to the row processor, and say that when the
> processor returns -1 it can store an error message string there.
> If it does so, that's what we report. If it doesn't (which we'd detect
> by presetting the value to NULL), then use a generic "error in row
> processor" message. This is cheap and doesn't prevent the row processor
> from using some application-specific error reporting method if it wants;
> but it does allow the processor to make use of libpq's error mechanism
> when that's preferable.

Yeah.

But such API seems to require specifying allocator, which seems ugly.
I think it would be better to just use Kyotaro's original idea
of PQsetRowProcessorError() which nicer to use.

Few thoughts on the issue:

----------

As libpq already provides quite good coverage of PGresult
manipulation APIs, then how about:

void PQsetResultError(PGresult *res, const char *msg);

that does:

res->errMsg = pqResultStrdup(msg);
res->resultStatus = PGRES_FATAL_ERROR;

that would also cause minimal fuss in getAnotherTuple().

------------

I would actually like even more:

void PQsetConnectionError(PGconn *conn, const char *msg);

that does full-blown libpq error logic. Thus it would be
useful everywherewhere in libpq. But it seems bit too disruptive,
so I would like a ACK from a somebody who knows libpq better.
(well, from you...)

-----------

Another thought - if we have API to set error from *outside*
of row-processor callback, that would immediately solve the
"how to skip incoming resultset without buffering it" problem.

And it would be usable for PQgetRow()/PQrecvRow() too.

--
marko