Merlin Moncure wrote:
> Attached is an updated version of 'libpq object hooks'. The primary
> purpose for libpq object hooks is to support our libpqtypes system
> (not attached), but could possibly some nice sideband features to
> libpq. We are hoping to sneak this into the May commit fest.
>
>
I've had a preliminary look at this.
The first thing it needs is lots and lots of documentation. I think it
probably needs a Section in the libpq chapter all on its own, preferably
with some examples. I think that lack alone is enough to keep it from
being committed for now.
Second, the hook names are compared case insensitively and by linear
search. I don't see any justification for using case insensitive names
for hooks in a C program, so I think that part should go. And if we
expect to keep anything other than trivial numbers of hooks we should
look at some sort of binary or hashed search.
The thing that is a bit disturbing is that the whole style of this
scheme is very different from the fairly simple APIs that the rest of
libpq presents. It's going to make libpq look rather odd, I think. I
would have felt happier if the authors had been able to come up with a
simple scheme to add API calls to export whatever information they
needed, rather than using this callback scheme.
That said, this patch doesn't look that bad to me otherwise, at least on
first inspection. One might say the the ability to add tuples to a
resultset arbitrarily, or to change an attribute arbitrarily, might be
footguns (and if you can add one, why can't you delete one?), but then
this is data in the hands of the client anyway, so they can do what they
like with it after they get it out of the resultset, so I guess there's
no real danger.
cheers
andrew
