Re: Speed dblink using alternate libpq tuple storage

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp>
To: markokr(at)gmail(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org, greg(at)2ndquadrant(dot)com, shigeru(dot)hanada(at)gmail(dot)com, mmoncure(at)gmail(dot)com
Subject: Re: Speed dblink using alternate libpq tuple storage
Date: 2012-02-21 05:11:35
Message-ID: 20120221.141135.47394784.horiguchi.kyotaro@oss.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

I don't have any attachment to PQskipRemainingResults(), but I
think that some formal method to skip until Command-Complete('C')
without breaking session is necessary because current libpq does
so.

====
> On Thu, Feb 16, 2012 at 02:24:19PM +0900, Kyotaro HORIGUCHI wrote:
> > The choices of the libpq user on that point are,
> >
> > - Continue to read succeeding tuples.
> > - Throw away the remaining results.
>
> There is also third choice, which may be even more popular than
> those ones - PQfinish().

That's it. I've implicitly assumed not to tear off the current
session.

> 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,

PQsetRowProcessor() sets row processor not to PGresult but
PGconn. So using PGsetRowProcessor() makes no sense for the
PGresult on which the user currently working. Another interface
function is needed to do that on PGresult.

But of course the users can do that by signalling using flags
within their code without PQsetRowProcessor or
PQskipRemainingResults.

Or returning to the beginning implement using PG_TRY() to inhibit
longjmp out of the row processor itself makes that possible too.

Altough it is possible in that ways, it needs (currently)
undocumented (in human-readable langs :-) knowledge about the
client protocol and the handling manner of that in libpq which
might be changed with no description in the release notes.

> I suspect the need is not that big.

I think so, too. But current implement of libpq does so for the
out-of-memory on receiving result rows. So I think some formal
(documented, in other words) way to do that is indispensable.

> 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.

mmm.. I consider that the cause of the problem proposed here is
the exceptions raised by certain server-side functions called in
row processor especially in C/C++ code. And we shouldn't use
PG_TRY() to catch it there where is too frequently executed. I
think 'return 2' is not applicable for the case. Some aid for
non-local exit from row processors (PQexec and the link from
users's sight) is needed. And I think it should be formal.

> 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).

The former might be inappropreate from the point of view of the
`undocumented knowledge' above. The latter seems missing the
point about exceptions.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Maxim Boguk 2012-02-21 05:40:30 Re: Unfamous 'could not read block ... in file "...": read only 0 of 8192 bytes' again
Previous Message Tom Lane 2012-02-21 05:03:11 Re: Unfamous 'could not read block ... in file "...": read only 0 of 8192 bytes' again