Thread: Re: [BUGS] ALTER TABLE

Re: [BUGS] ALTER TABLE

From
Tom Lane
Date:
Stephan Szabo <sszabo@megazone23.bigpanda.com> writes:
> I believe these are all the cases I saw of heap_openr with no lock
> and no check in the actual trigger functions (patch to
> hopefully elog(ERROR) instead of crashing attached).
> [ patch snipped ]

We had a discussion about that on 11-July and the consensus seemed to be
that the real problem is heap_open's definition; it's too easy to forget
when you need to check for failure return.  I have just committed
changes that split heap_open into two routines: heap_open() now ALWAYS
elogs on failure, regardless of lock mode, while heap_open_nofail() is
what to call if you really want a NULL return on failure.  (Likewise for
heap_openr() of course.)  I found only about three places in the whole
backend that really wanted the _nofail() case.

Accordingly, this patch is not needed anymore in current sources, though
it'd still be the most convenient fix for 7.0.* series if anyone is
concerned enough to apply it.

A possibly more important issue: why are the RI triggers opening the
referenced rel with NoLock anyway?  Doesn't that leave you open to
someone deleting the referenced rel out from under you while you are
working with it?  Seems like at minimum you should grab AccessShareLock.
        regards, tom lane


Re: [BUGS] ALTER TABLE

From
Stephan Szabo
Date:
On Thu, 3 Aug 2000, Tom Lane wrote:

> Accordingly, this patch is not needed anymore in current sources, though
> it'd still be the most convenient fix for 7.0.* series if anyone is
> concerned enough to apply it.
Yeah, actually, a friend of mine ran into this recently with incorrect 
create constraint trigger statements so I already was going to send a
patch to him, then it got mentioned on -bugs.

> A possibly more important issue: why are the RI triggers opening the
> referenced rel with NoLock anyway?  Doesn't that leave you open to
> someone deleting the referenced rel out from under you while you are
> working with it?  Seems like at minimum you should grab AccessShareLock.
That's a good point.  To be honest, I don't really know why it's not
grabbing a lock (Jan?).  As a general newbie question for such things,
what happens to your relation pointer if it were to be deleted out
from under?  I figure that if it gets to the actual query, it will fail
(unless someone were to create a table with that name in the meantime -
ouch...)