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

On Fri, Mar 30, 2012 at 05:18:42PM -0400, Tom Lane wrote:
> Marko Kreen <markokr@gmail.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