On Wed, Sep 10, 2014 at 4:01 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> 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.
>
> We've done it that way in a bunch of other places, like ExecutorRun().
> An advantage of this approach (I think) is that jumping to a fixed
> address is faster than jumping through a function pointer - so with
> the approach I've taken here, the common case where we're talking to
> the client incurs only the overhead of a null-test, and the larger
> overhead of the function pointer jump is incurred only when the hook
> is in use. Maybe that's not enough of a difference to matter to
> anything, but I think the contention that I've invented some novel
> kind of interface here doesn't hold up to scrutiny. We have lots of
> hooks that work just like what I did here.
Here's what the other approach looks like. I can't really see doing
this way and then only providing hooks for those two functions, so
this is with hooks for all the send-side stuff.
Original version: 9 files changed, 295 insertions(+), 3 deletions(-)
This version: 9 files changed, 428 insertions(+), 47 deletions(-)
There is admittedly a certain elegance to providing a complete set of
hooks, so maybe this is the way to go. The remaining patches in the
patch series work with either the old version or this one; the changes
here don't affect anything else.
Anyone else have an opinion on which way is better here?
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company