Re: Speed dblink using alternate libpq tuple storage

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

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-02-24T10:53:14Z

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 →

Fix COPY FROM for null marker strings that correspond to invalid encoding.
- e8476f46fc84 9.2.0 cited
Improve labeling of pg_test_fsync open_sync test output.
- 2bbd88f8f841 9.2.0 cited

Attachments

libpq_rowproc_20120224.patch (text/x-patch) patch
libpq_rowproc_doc_20120224.patch (text/x-patch) patch
dblink_use_rowproc_20120224.patch (text/x-patch) patch
early_exit_20120224.diff (text/x-patch) patch

Hello, this is new version of the patch.

> > By the way, I would like to ask you one question. What is the
> > reason why void* should be head or tail of the parameter list?
> 
> Aesthetical reasons:

 I got it. Thank you.

> Last comment - if we drop the plan to make PQsetRowProcessorErrMsg()
> usable outside of handler, we can simplify internal usage as well:
> the PGresult->rowProcessorErrMsg can be dropped and let's use
> ->errMsg to transport the error message.
> 
> The PGresult is long-lived structure and adding fields for such
> temporary usage feels wrong.  There is no other libpq code between
> row processor and getAnotherTuple, so the use is safe.

I almost agree with it. Plus, I think it is no reason to consider
out of memory as particular because now row processor becomes
generic. But the previous patch does different process for OOM
and others, but I couldn't see obvious reason to do so.

- PGresult.rowProcessorErrMes is removed and use PGconn.errMsg
  instead with the new interface function PQresultSetErrMes().

- Now row processors should set message for any error status
  occurred within. pqAddRow and dblink.c is modified to do so.

- getAnotherTuple() sets the error message `unknown error' for
  the condition rp == 0 && ->errMsg == NULL.

- Always forward input cursor and do pqClearAsyncResult() and
  pqSaveErrorResult() when rp == 0 in getAnotherTuple()
  regardless whether ->errMsg is NULL or not in fe-protocol3.c.

- conn->inStart is already moved to the start point of the next
  message when row processor is called. So forwarding inStart in
  outOfMemory1 seems redundant. I removed it.

- printfPQExpBuffer() compains for variable message. So use
  resetPQExpBuffer() and appendPQExpBufferStr() instead.
  
=====
- dblink does not use conn->errorMessage before, and also now...
  all errors are displayed as `Error occurred on dblink connection...'.

- TODO: No NLS messages for error messages.

- Somehow make check yields error for base revision. So I have
  not done that.

- I have no idea how to do test for protocol 2...

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center