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. It seems to me that at a minimum it needs to take a
self-exclusive lock, because otherwise you can get things like this,
which we ordinarily try to avoid:
S1: BEGIN;
S1: ALTER TABLE tab DETACH PARTITION tab2;
S2: alter table tab2 set (fillfactor = 90);
S1: COMMIT;
S2: ERROR: tuple concurrently updated
I'm not 100% certain that ShareUpdateExclusiveLock is actually a
strong enough lock level, but I think that the right answer can't be
any weaker than that. So I propose to increase the level to that
value and back-patch. This looks to be a bug in the original table
partitioning commit (f0e44751d7175fa3394da2c8f85e3ceb3cdbfe63).
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.
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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
