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 20120216081721.GA4511@gmail.com
Whole thread Raw
In response to Re: Speed dblink using alternate libpq tuple storage  (Kyotaro HORIGUCHI <horiguchi.kyotaro@oss.ntt.co.jp>)
Responses Re: Speed dblink using alternate libpq tuple storage  (Kyotaro HORIGUCHI <horiguchi.kyotaro@oss.ntt.co.jp>)
List pgsql-hackers
On Thu, Feb 16, 2012 at 02:24:19PM +0900, Kyotaro HORIGUCHI wrote:
> As far as I see, on an out-of-memory in getAnotherTuple() makes
> conn->result->resultStatus = PGRES_FATAL_ERROR and
> qpParseInputp[23]() skips succeeding 'D' messages consequently.
> 
> When exception raised within row processor, pg_conn->inCursor
> always positioned in consistent and result->resultStatus ==
> PGRES_TUPLES_OK.
> 
> The choices of the libpq user on that point are,
> 
> - Continue to read succeeding tuples.
> 
>   Call PQgetResult() to read 'D' messages and hand it to row
>   processor succeedingly.
> 
> - Throw away the remaining results.
> 
>   Call pqClearAsyncResult() and pqSaveErrorResult(), then call
>   PQgetResult() to skip over the succeeding 'D' messages. (Of
>   course the user can't do that on current implement.)

There is also third choice, which may be even more popular than
those ones - PQfinish().
> To make the users able to select the second choice (I think this
> is rather major), we should only provide and export the new PQ*
> function to do that, I think.

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,
I suspect the need is not that big.

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.

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

-- 
marko



pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Incorrect behaviour when using a GiST index on points
Next
From: Kyotaro HORIGUCHI
Date:
Subject: Re: Speed dblink using alternate libpq tuple storage