From: | Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)oss(dot)ntt(dot)co(dot)jp> |
---|---|
To: | markokr(at)gmail(dot)com |
Cc: | 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-07 06:14:57 |
Message-ID: | 20120307.151457.91509043.horiguchi.kyotaro@oss.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
libpq_rowproc_20120307.patch | text/x-patch | 22.1 KB |
libpq_rowproc_doc_20120307.patch | text/x-patch | 8.1 KB |
dblink_use_rowproc_20120307.patch | text/x-patch | 13.5 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2012-03-07 07:07:37 | Re: Scaling XLog insertion (was Re: Moving more work outside WALInsertLock) |
Previous Message | Pavel Stehule | 2012-03-07 05:35:06 | Re: review: CHECK FUNCTION statement |