Re: [HACKERS] Vacuum analyze bug CAUGHT

Bruce Momjian <maillist(at)candle(dot)pha(dot)pa(dot)us> writes:
>> Tom Lane wrote:
>>>> Or maybe better: change heap_open/heap_openr/heap_close to take an
>>>> additional parameter specifying the kind of lock to grab.

>> This way is better.

> ... there is no reason not to add additional parameters to function
> calls like I did for heap_fetch() if it makes sense.

OK. Another thing that's been on my to-do list is that a lot of places
fail to check for a failure return from heap_open(r), which means you
get a null pointer dereference coredump instead of a useful message if
anything goes wrong. (But, of course, when opening a system table
nothing can ever go wrong ... go wrogn ...)

Here's what I propose:

Add another parameter to heap_open and heap_openr, which can be any of
the lock types currently mentioned in storage/lmgr.h, or "NoLock".
With "NoLock" you get the current behavior: no lock is acquired, and
NULL is returned if the open fails; it's up to the caller to check that
and do something appropriate. Otherwise, the routines will check for
open failure and raise a standard elog(ERROR) if they do not succeed;
furthermore, they will acquire the specified type of lock on the
relation before returning. (And, thanks to the already-in-place call
in LockRelation, any pending SI-inval messages will be read.)

heap_close will also take an additional parameter, which is a lock type
to release the specified lock, or NoLock to release no lock. (Note
that it is often correct not to release the lock acquired at heap_open
during heap_close; in this case, the lock is held till end of
transaction. So, we don't want heap_close to automagically release
whatever lock was acquired by the corresponding heap_open, even if it
were easy to do so which it isn't...)

If I don't hear any objections, I'll get on with implementing that.

regards, tom lane