On Fri, Jul 31, 2015 at 11:41 AM, Fabrízio de Royes Mello wrote:
>> We usually don't compare lock values that way, i.e. there's not
>> guaranteed to be a strict monotonicity between lock levels. I don't
>> really agree with that policy, but it's nonetheless there.
>
> And how is the better way to compare lock values to get the highest lock
> level? Perhaps creating a function to compare lock levels?
I guess that this is exactly what Andres has in mind, aka something
like LockModeCompare(lockmode, lockmode) that returns {-1,0,1}
depending on which lock is higher on the hierarchy. This would do
exactly what your patch is doing though, except that this will
localize the comparison operators in lock.c. Though I am seeing at
quick glance a couple of places already do such comparisons:
backend/commands/tablecmds.c: if (cmd_lockmode > lockmode)
backend/storage/lmgr/lock.c: lockmode > RowExclusiveLock)
backend/storage/lmgr/lock.c: if (lockmode >= AccessExclusiveLock &&
backend/access/heap/heapam.c: Assert(lockmode >= NoLock && lockmode
< MAX_LOCKMODES);
backend/access/heap/heapam.c: Assert(lockmode >= NoLock && lockmode
< MAX_LOCKMODES);
backend/access/heap/heapam.c: Assert(lockmode >= NoLock && lockmode
< MAX_LOCKMODES);
backend/access/index/indexam.c: Assert(lockmode >= NoLock &&
lockmode < MAX_LOCKMODES);
All of them are just sanity checks, except the one in tablecmds.c is
not (2dbbda0). Hence I am thinking that this is not really a problem
this patch should tackle by itself...
Regards,
--
Michael