Re: lock level for DETACH PARTITION looks sketchy

On 2018-Dec-19, Robert Haas wrote:

> While working on the problem of lowering lock levels for ATTACH
> PARTITION and DETACH PARTITION, I discovered that DETACH PARTITION
> takes only AccessShareLock on the table being detached, which I think
> is not good.

Oh, I remember eyeing that suspiciously, but was too distracted on
making the other thing work to realize it was actually wrong :-(
I agree that it's wrong.

> I noticed another thing that looks odd as well. ATExecDetachPartition
> does this:
>
> idx = index_open(idxid, AccessExclusiveLock);
> IndexSetParentIndex(idx, InvalidOid);
> update_relispartition(classRel, idxid, false);
> relation_close(idx, AccessExclusiveLock);
>
> That last line is unlike what we do nearly everywhere else in that it
> releases a lock on a user relation before commit time. I don't know
> whether that has any bad consequences, but it means that some other
> backend could obtain a lock on that relation before we've queued any
> invalidation messages associated with the change, which seems possibly
> dangerous.

Oh yeah, my bug -- at some point I had an XXX comment or several about
fixing lock levels at release time everywhere, but evidently this one
slipped through. It definitely should be NoLock. Will fix.

> A minor nitpick is that the last like there should probably use
> index_close() to match the index_open() a few lines above. That code
> happens to be identical to the code for relation_open(), but it's
> probably better to use the matching function.

No objection ... will change too.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services