Re: Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

On Fri, Jul 31, 2015 at 10:01 AM, Michael Paquier <michael.paquier@gmail.com> wrote:
>
> 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...
>

I did it in the attached version of the patch... But I don't know if the names are good so fell free to suggest others if you dislike of my choice.

In this patch I didn't change all lockmode comparison places previous pointed by you, but I can change it maybe adding other method called LockModeIsValid(lockmode) to do the comparison "lockmode >= NoLock && lockmode < MAX_LOCKMODES" used in many places.

Regards,

--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog: http://fabriziomello.github.io
>> Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
>> Github: http://github.com/fabriziomello