Message-ID: <48BE0424.4080905@esilo.com>
Date: Tue, 02 Sep 2008 23:27:32 -0400
From: Andrew Chernow <ac@esilo.com>
User-Agent: Thunderbird 2.0.0.14 (Windows/20080421)
MIME-Version: 1.0
To: Alvaro Herrera <alvherre@commandprompt.com>
CC: Merlin Moncure <mmoncure@gmail.com>,
	Pg Hackers <pgsql-hackers@postgresql.org>
Subject: Re: libpq object hooks (libpq events)
References: <19342.1210776271@sss.pgh.pa.us>
	<b42b73150805140921v30d24b75m6c230932c61ffdd2@mail.gmail.com>
	<482B1480.1070705@esilo.com> <482B2AC5.7000503@esilo.com>
	<482ECF62.7010901@esilo.com> <12200.1211242959@sss.pgh.pa.us>
	<48323593.4060302@esilo.com> <13642.1211251078@sss.pgh.pa.us>
	<48323F94.6010201@esilo.com> <48325386.2010506@esilo.com>
	<20080903001920.GT12610@alvh.no-ip.org>
In-Reply-To: <20080903001920.GT12610@alvh.no-ip.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

Alvaro Herrera wrote:
> Andrew Chernow escribió:
>> Attached is the latest patch.  It has addressed the requested changes 
>> found here: 
>> http://archives.postgresql.org/pgsql-patches/2008-05/msg00389.php
>>
>> Its a tarball because there are two new files, libpq-events.c and  
>> libpq-events.h.  The patch is in the tarball as well as attached to the 
>> email.
> 
> I modified this patch slightly.  I was about to try libpqtypes on it,
> but then I noticed that libpqtypes as published on pgfoundry is based on
> a very old version of this patch, so I punted.  So, for now, the only
> guarantee is that it compiles with no warnings.
> 

Being that it is now on pgfoundry, we didn't want to make drastic changes.  We 
wanted to wait to see how this patch unfolds and make a major change then.

"event" version of libpqtypes (not on foundry yet)
http://libpqtypes.esilo.com/libpqtypes-events.tar.gz

> (I also removed PQresultAlloc.)
> 

Nooooooo ... removing PQresultAlloc breaks libpqtypes!  It also removes some of 
the use cases provided by PQsetvalue, which allows one to add to a result (in 
our case from scratch).

> There's one thing that seems a bit baroque, which is the
> PG_COPYRES_USE_ATTRS stuff in PQcopyResult.  I think that flag
> introduces different enough behavior that it should be a routine of its
> own, say PQcopyResultAttrs.  That way you would leave out the two extra
> params in PQcopyResult.
> 

Okay.  We can move attr copying to its own function.

> Oh -- one last thing.  I am not really sure about the flags to
> PQcopyResult.  Should there really be flags to _remove_ behavior,
> instead of flags that add?  
> 

Agreed, that is kinda weird.  Probably ended up that because the flags were an 
after thought.

 > However, the only change of any significance that I introduced was that
 > a "name" is attached to every event proc, so that it can be reported in
 > error messages, as reporting only %p seems very useless.  (I also
 > removed PQresultAlloc.)

I don't mind re-introducing the name, but Tom seemed very against this due to 
conflicts.  If 2 different libraries register the same name, debugging would be 
painful.

 > The main missing thing from this patch is SGML docs for the
 > new libpq functions.

We have no issue here.  We are waiting for the patch to gain acceptance so we 
only document this once.

-- 
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/