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
