On 09/09/14 22:48, Robert Haas wrote:
On Tue, Sep 9, 2014 at 1:18 PM, Petr Jelinek <p...@2ndquadrant.com> wrote:
I think that's completely wrong. As the patch series demonstrates,
it's not limited to propagating ErrorResponse and NoticeResponse. It
can also propagate NotifyResponse and RowDescription and DataRow and
anything else that comes along. We are not just propagating errors;
we are propagating all protocol messages of whatever type. So tying
it to elog specifically is not right.
Oh in that case, I think what Andres proposed is actually quite good. I know
the hook works fine it just seems like using somewhat hackish solution to
save 20 lines of code.
If it's 20 lines of code, I'm probably fine to go that way. Let's see
if we can figure out what those 20 lines look like.
libpq.h exports 29 functions that do a variety of different things.
Of those, 20 are in pqcomm.c and the others are in be-secure.c. I
presume that pluggability for the latter group, if needed at all, is a
separate project. The remaining ones break down like this:
Ugh
- The remaining functions are pq_init(), pq_comm_reset(), pq_flush(),
pq_flush_if_writable(), pq_is_send_pending(), pq_putmessage(),
pq_putmessage_noblock(), pq_startcopyout(), and pq_endcopyout().
These are the ones that I think are potentially interesting.
I didn't choose to provide hooks for all of these in the submitted
patch because they're not all needed for I want to do here:
pq_startcopyout() and pq_endcopyout() are only needed for V2 protocol
support, which did not interest me (nor did COPY, really);
pq_putmessage_noblock(), pq_flush_if_writable(), and
pq_is_send_pending() are only used for the walsender protocol, which
doesn't seem useful to redirect to a non-socket; and I just didn't
happen to have any use for pq_init() or pq_comm_reset(). Hence what I
ended up with.
But, I could revisit that. Suppose I provide a structure with 10
function pointers for all ten of those functions, or maybe 9 since
pq_init() is called so early that it's not likely we'll have control
to put the hooks in place before that point, and anyway whatever code
installs the hooks can do its own initialization then. Then make a
global variable like pqSendMethods and #define pq_comm_reset() to be
pqSendMethods->comm_reset(), pflush() to be pqSendMethods->flush(),
and so on for all 9 or 10 methods. Then the pqmq code could just
change pqSendMethods to point to its own method structure instead of
the default one. Would that address the concern this concern? It's
more than 20 lines of code, but it's not TOO bad.
Yes, although my issue with the hooks was not that you only provided
them for 2 functions, but the fact that it had no structure and the
implementation was "if hook set do this, else do that" which I don't see
like a good way of doing it.
All I personally want is structure and extensibility so struct with just
the needed functions is good enough for me and I would be ok to leave
the other 8 functions just as a option for whomever needs to make them
overridable in the future.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
