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-02-24 10:53:14 |
Message-ID: | 20120224.195314.251581347.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.
> > 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
Attachment | Content-Type | Size |
---|---|---|
libpq_rowproc_20120224.patch | text/x-patch | 23.1 KB |
libpq_rowproc_doc_20120224.patch | text/x-patch | 9.4 KB |
dblink_use_rowproc_20120224.patch | text/x-patch | 13.0 KB |
early_exit_20120224.diff | text/x-patch | 2.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Daniel Farina | 2012-02-24 11:14:50 | Re: pg_stat_statements normalization: re-review |
Previous Message | Simon Riggs | 2012-02-24 10:35:44 | Re: Initial 9.2 pgbench write results |