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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Chernow <ac(at)esilo(dot)com>
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 12:16:52
Message-ID: 8872.1221653812@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

Andrew Chernow <ac(at)esilo(dot)com> writes:
>>> Now, it's questionable how tense we need to be about that as long as
>>> event proc failure aborts calling of later event procs. That means
>>> that procs have to be robust against getting DESTROY with no CREATE
>>> calls in any case. Should we try to make that less uncertain?

> I attached a patch that adds a 'needDestroy' member to PGEvent It is
> set when resultcreate or resultcopy succeeds and checked during a
> PQclear. That *should* resolve the issue of "no resultcreate but gets
> a resultdestroy".

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.

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.

Also, if we choose option B, then the same would hold for REGISTER
versus CONNDESTROY: failing REGISTER should still mean that you get
a CONNDESTROY. So maybe that's an argument for option A, because
that's how REGISTER works now.

Lastly, the idea that was in the back of my mind was to resolve this
issue by unconditionally calling all the event procs at CREATE time,
not by cutting off the later ones if an earlier one failed. Again,
I do not see a performance argument for skipping the extra steps,
and it seems to me that it might improve symmetry/robustness.
I'm not necessarily wedded to that approach but it bears thinking
about.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zdenek Kotala 2008-09-17 12:18:21 Re: New FSM patch
Previous Message Robert Haas 2008-09-17 12:12:09 Re: Common Table Expressions (WITH RECURSIVE) patch

Browse pgsql-patches by date

  From Date Subject
Next Message Andrew Chernow 2008-09-17 13:20:29 Re: [PATCHES] libpq events patch (with sgml docs)
Previous Message Andrew Chernow 2008-09-17 10:50:16 Re: [PATCHES] libpq events patch (with sgml docs)