Re: Speed dblink using alternate libpq tuple storage

On Tue, Feb 21, 2012 at 11:42 AM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp> wrote:
>> Also, as row handler is on connection, then changing it's
>> behavior is connection context, not result.
>
> OK, current implement copying the pointer to the row processor
> from PGconn to PGresult and getAnotherTuple() using it on
> PGresult to avoid unintended replacement of row processor by
> PQsetRowProcessor(), and I understand that row processor setting
> should be in PGconn context and the change by PGsetRowProcessor()
> should immediately become effective. That's right?

Note I dropped the row processor from under PGresult.
Please don't put it back there.

>> Ok, lets see how it looks.  But please do it like this:
>>
>> - PQgetRowProcessor() that returns existing row processor.
>
> This seems also can be done by the return value of
> PQsetRowProcessor() (currently void). Anyway, I provide this
> function and also change the return value of PQsetRowProcessor().

Note you need processorParam as well.
I think it's simpler to rely on PQgetProcessor()

>> - PQskipResult() that just replaces row processor, then calls
>>   PQgetResult() to eat the result.  It's behaviour fully
>>   matches PQgetResult() then.
>
> There seems to be two choices, one is that PQskipResult()
> replaces the row processor with NULL pointer and
> getAnotherTuple() calls row processor if not NULL. And the
> another is PQskipResult() sets the empty function as row
> processor. I do the latter for the present.

Yes, let's avoid NULLs.

> This approach does needless call of getAnotherTuple(). Seeing if
> the pointer to row processor is NULL in pqParseInput[23]() could
> avoid this extra calls and may reduce the time for skip, but I
> think it is no need to do so for such rare cases.

We should optimize for common case, which is non-skipping
row processor.

>> Only question here is what should happen if there are
>> several incoming resultsets (several queries in PQexec).
>> Should we leave to user to loop over them?
>>
>> Seems there is 2 approaches here:
>>
>> 1) int PQskipResult(PGconn)
>> 2) int PQskipResult(PGconn, int skipAll)
>>
>> Both cases returning:
>>     1 - got resultset, there might be more
>>     0 - PQgetResult() returned NULL, connection is empty
>>    -1 - error
>>
>> Although 1) mirrors regular PGgetResult() better, most users
>> might not know that function as they are using sync API.
>> They have simpler time with 2).  So 2) then?
>
> Let me confirm the effects of this function. Is the below
> description right?
>
> - PQskipResult(conn, false) makes just following PQgetResult() to
>  skip current bunch of rows and the consequent PQgetResult()'s
>  gathers rows as usual.

Yes.

> - PQskipResult(conn, true) makes all consequent PQgetResult()'s
>  to skip all the rows.
>
> If this is right, row processor should stay also in PGresult
> context. PQskipResult() replaces the row processor in PGconn when
> the second parameter is true, and in PGresult for false.

No, let's keep row processor only under PGconn.

--
marko