Re: [PATCHES] libpq events patch (with sgml docs)

From: Andrew Chernow <ac(at)esilo(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Merlin Moncure <mmoncure(at)gmail(dot)com>
Subject: Re: [PATCHES] libpq events patch (with sgml docs)
Date: 2008-09-17 13:20:29
Message-ID: 48D1041D.5070808@esilo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

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/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2008-09-17 13:21:33 Re: NDirectFileRead and Write
Previous Message KaiGai Kohei 2008-09-17 13:18:03 Re: Proposal of SE-PostgreSQL patches (for CommitFest:Sep)

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2008-09-17 13:36:26 Re: [PATCHES] libpq events patch (with sgml docs)
Previous Message Tom Lane 2008-09-17 12:16:52 Re: [PATCHES] libpq events patch (with sgml docs)