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

--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches

Reply via email to