Re: lock_timeout GUC patch

Boszormenyi Zoltan <zb@cybertec.at> writes:
> Tom Lane �rta:
>> If this patch is touching those parts of relcache.c, it probably needs
>> rethinking.

> What I did there is to check the return value of LockRelationOid()
> and also elog(PANIC) if the lock wasn't available.
> Does it need rethinking?

Yes.  What you have done is to change all the LockSomething primitives
from return void to return bool and thereby require all call sites to
check their results.  This is a bad idea.  There is no way that you can
ensure that all third-party modules will make the same change, meaning
that accepting this patch will certainly introduce nasty, hard to
reproduce bugs.  And what's the advantage?  The callers are all going
to throw errors anyway, so you might as well do that within the Lock
function and avoid the system-wide API change.

I think this is a big patch with a small patch struggling to get out.
        regards, tom lane