Horton

Re: Speed dblink using alternate libpq tuple storage

Kyotaro Horiguchi <horiguchi.kyotaro@oss.ntt.co.jp>

View thread
From: Kyotaro HORIGUCHI <horiguchi.kyotaro@oss.ntt.co.jp>
To: markokr@gmail.com
Cc: greg@2ndquadrant.com, pgsql-hackers@postgresql.org, mmoncure@gmail.com, shigeru.hanada@gmail.com
Date: 2012-03-07T06:14:57Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix COPY FROM for null marker strings that correspond to invalid encoding.

  2. Improve labeling of pg_test_fsync open_sync test output.

Attachments

Hello, this is new version of the patch.

# early_exit.diff is not included for this time and maybe also
# later.  The set of the return values of PQrowProcessor looks
# unnatural if the 0 is removed.

> * Convert old EOFs to protocol errors in V3 getAnotherTuple()

done.

> * V2 getAnotherTuple() can leak PGresult when handling custom
>   error from row processor.

Custom error message is removed from both V2 and V3.

> * remove pqIsnonblocking(conn) check when row processor returned 2.
>   I missed that it's valid to call PQisBusy/PQconsumeInput/PQgetResult
>   on sync connection.

done. This affects PQisBusy, but PQgetResult won't be affected as
far as I see. I have no idea for PQconsumeInput()..

> * It seems the return codes from callback should be remapped,
>   (0, 1, 2) is unusual pattern.  Better would be:
> 
>    -1 - error
>     0 - stop parsing / early exit ("I'm not done yet")
>     1 - OK ("I'm done with the row")

done.

This might be described more precisely as follows,

|    -1 - error - erase result and change result status to
|               - FATAL_ERROR All the rest rows in current result
|               - will skipped(consumed).
|     0 - stop parsing / early exit ("I'm not done yet")
|                - getAnotherTuple returns EOF without dropping PGresult.
#                - We expect PQisBusy(), PQconsumeInput()(?) and 
#                - PQgetResult() to exit immediately and we can
#                - call PQgetResult(), PQskipResult() or
#                - PQisBusy() after.
|     1 - OK ("I'm done with the row")
|                - save result and getAnotherTuple returns 0.

The lines prefixed with '#' is the desirable behavior I have
understood from the discussion so far. And I doubt that it works
as we expected for PQgetResult().

> * Please drop PQsetRowProcessorErrMsg() / PQresultSetErrMsg().

done.

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

regards, 

-- 
Kyotaro Horiguchi
NTT Open Source Software Center