Re: ALTER TABLE

Stephan Szabo <sszabo(at)megazone23(dot)bigpanda(dot)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