Re: Speed dblink using alternate libpq tuple storage

From: Marko Kreen <markokr(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp>, greg(at)2ndquadrant(dot)com, pgsql-hackers(at)postgresql(dot)org, mmoncure(at)gmail(dot)com, shigeru(dot)hanada(at)gmail(dot)com
Subject: Re: Speed dblink using alternate libpq tuple storage
Date: 2012-03-30 22:26:47
Message-ID: CACMqXC+FiqoUzHopXWczJUJUPDFS5C1RCx+jtpe1GNFrUD-uyg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Mar 31, 2012 at 1:13 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Marko Kreen <markokr(at)gmail(dot)com> writes:
>> On Fri, Mar 30, 2012 at 05:18:42PM -0400, Tom Lane wrote:
>>> 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.
>
>> But such API seems to require specifying allocator, which seems ugly.
>
> Not if the message is a constant string, which seems like the typical
> situation (think "out of memory").  If the row processor does need a
> buffer for a constructed string, it could make use of some space in its
> "void *param" area, for instance.

If it's specified as string that libpq does not own, then I'm fine with it.

>> I think it would be better to just use Kyotaro's original idea
>> of PQsetRowProcessorError() which nicer to use.
>
> I don't particularly care for that idea because it opens up all sorts of
> potential issues when such a function is called at the wrong time.
> Moreover, you have to remember that the typical situation here is that
> we're going to be out of memory or otherwise in trouble, which means
> you've got to be really circumspect about what you assume will work.
> Row processors that think they can do a lot of fancy message
> construction should be discouraged, and an API that requires
> construction of a new PGresult in order to return an error is right out.
> (This is why getAnotherTuple is careful to clear the failed result
> before it tries to build a new one.  But that trick isn't going to be
> available to an external row processor.)

Kyotaro's original idea was to assume out-of-memory if error
string was not set, thus the callback needed to set the string
only when it really had something to say.

--
marko

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2012-03-30 22:35:30 Re: Speed dblink using alternate libpq tuple storage
Previous Message Tom Lane 2012-03-30 22:13:50 Re: Speed dblink using alternate libpq tuple storage