Re: Speed dblink using alternate libpq tuple storage - Mailing list pgsql-hackers
From | Marko Kreen |
---|---|
Subject | Re: Speed dblink using alternate libpq tuple storage |
Date | |
Msg-id | 20120330215940.GA13857@gmail.com Whole thread Raw |
In response to | Re: Speed dblink using alternate libpq tuple storage (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Speed dblink using alternate libpq tuple storage
|
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
pgsql-hackers by date: