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

From Tom Lane
Subject Re: [PATCHES] libpq events patch (with sgml docs)
Date
Msg-id 10839.1221658586@sss.pgh.pa.us
Whole thread Raw
In response to Re: [PATCHES] libpq events patch (with sgml docs)  (Andrew Chernow <ac@esilo.com>)
Responses Re: [PATCHES] libpq events patch (with sgml docs)
List pgsql-hackers
Andrew Chernow <ac@esilo.com> writes:
> 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 still think it would be a good idea to expend a couple more lines in
PQcopyResult to cover the case --- the likelihood of there being code
paths that make an empty result and never invoke RESULTCREATE just seems
too high.  Defensive programming 'n all that.  In any case I'm not
convinced that we should force a RESULTCREATE cycle on external callers
of PQmakeEmptyPGResult.  If you guys didn't see a need for it in your
development then it probably doesn't exist.

> 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.

I can live with that definition, but please document it.

> The needDestroy member should be called resultInitialized.

Yeah, probably, so that you can set it FALSE in conn events and continue
to use memcpy to move things over.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: NDirectFileRead and Write
Next
From: "Kevin Grittner"
Date:
Subject: Re: Common Table Expressions (WITH RECURSIVE) patch