Alvaro Herrera wrote:
> Andrew Dunstan escribió:
>
>
>> 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.
>>
>
> I'm not sure I understand this point. Remember that this is here to
> support the libpqtypes library. There doesn't seem to be a way for an
> API such as you describe to work.
>
That might well be true. The issue then becomes "Do we want to add
something with this flavor to libpq?" I take it Bruce's answer is "No",
at least until he has seen more evidence of general usefulness. I think
we need to make a decision on this before anyone wastes any more time.
It should be noted that while this feels slightly foreign, it isn't
hugely invasive, unlike the previous effort - it's only a few hundred
lines of new code.
If we reject this, presumably the authors will have no alternative than
to offer libpqtypes as a patch to libpq. ISTM that we're then asking
them to climb over a fairly high hurdle. On the one hand we want them to
demonstrate that there's demand for their tool and on the other we make
it difficult to distribute and deploy.
>
>> 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.
>>
>
> Keep in mind that the original patch supported a single hook being
> registered. Perhaps we could dream about having a couple of hooks
> registered, but turning into hashed search would seem to be overkill, at
> least for now. (If hooking into libpq is truly successful we can always
> improve it later -- it's not an exported detail of the API after all.)
>
>
Right, it was more the case insensitive part that bothered me.
cheers
andrew
