Re: [PATCHES] libpq events patch (with sgml docs) - Mailing list pgsql-hackers

From Andrew Chernow
Subject Re: [PATCHES] libpq events patch (with sgml docs)
Date
Msg-id 48D1041D.5070808@esilo.com
Whole thread Raw
In response to Re: [PATCHES] libpq events patch (with sgml docs)  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [PATCHES] libpq events patch (with sgml docs)  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Tom Lane wrote:
> 
> Some thought would need to be given to how that interacts with
> RESULTCOPY.  Presumably on the destination side, RESULTCOPY is the
> equivalent of RESULTCREATE, ie, don't DESTROY if you didn't get COPY.
> But what of the source side?  Arguably you shouldn't invoke COPY on
> events that were never initialized in this object.
> 

You are correct.  The resultcopy function needs to check if the src 
result eventproc was initialized.  BUT, that should not be possible 
unless someone is not checking return values.  Meaning, the only way to 
have an uninitialized eventproc in this instance is if something failed 
during a resultcreate.  Actually, if you call PQmakeEmptyPGResult then 
you can also run into this problem.  That can be solved by adding an 
argument to makeResult as I previously suggested.

> I also think that a little bit of thought should go into whether or
> not to call DESTROY on an event that *did* get a CREATE and failed it.
> You could argue that one either way: should a failing CREATE operation
> be expected to fully clean up after itself, or should it be expected
> to leave things in a state where DESTROY can clean up properly?
> I'm not entirely sure, but option A might force duplication of code
> between CREATE's failure path and DESTROY.  Whichever semantics we
> choose needs to be documented.
> 

If a resultcreate fails, than I think that resultdestroy should not be 
delivered to that eventproc (same for resultcopy).  That is how register 
works and how I saw this patch working (eventhough it appears I have a 
few logical errors).  I don't think it makes sense to do it otherwise, 
it would be like calling free after a malloc failure.

The needDestroy member should be called resultInitialized.  Although the 
conn doesn't reference the 'resultInitialized' member, I should 
initialize it to FALSE.  I did not do this in the last patch ... 
register function.

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


pgsql-hackers by date:

Previous
From: KaiGai Kohei
Date:
Subject: Re: Proposal of SE-PostgreSQL patches (for CommitFest:Sep)
Next
From: Tom Lane
Date:
Subject: Re: NDirectFileRead and Write