Re: [HACKERS] Vacuum analyze bug CAUGHT

Bruce Momjian <maillist@candle.pha.pa.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