Message-ID: <482AD896.4000504@dunslane.net>
Date: Wed, 14 May 2008 08:18:30 -0400
From: Andrew Dunstan <andrew@dunslane.net>
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US;
	rv:1.8.0.12) Gecko/20071019 Fedora/1.0.9-3.fc6 pango-text
	SeaMonkey/1.0.9
MIME-Version: 1.0
To: Alvaro Herrera <alvherre@commandprompt.com>
CC: Merlin Moncure <mmoncure@gmail.com>,
	"Patches (PostgreSQL)" <pgsql-patches@postgresql.org>,
	Andrew Chernow <ac@esilo.com>
Subject: Re: libpq object hooks
References: <b42b73150804301323i18b00019i62e765708abb65aa@mail.gmail.com>
	<482A58D8.2030205@dunslane.net>
	<20080514043313.GS6966@alvh.no-ip.org>
In-Reply-To: <20080514043313.GS6966@alvh.no-ip.org>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit


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