Hi Chao, Thanks for v3. I applied both patches to master as of fded90eec3e, they apply cleanly and build without warnings. With v3, partition-attach-lock-order, partition-concurrent-attach and partition-drop-index-locking all pass. On an unpatched build of the same commit, partition-attach-lock-order fails with "deadlock detected" in the ATTACH step (s2attach) and the other two pass, so the test still catches the bug. The v3 changes address my comments.
Regarding the approach, I think pre-locking in the RangeVar callback is the right place, and probably the only one that works. By the time generate_partition_qual() runs, the target is already locked, so taking the ancestor locks top-down there would still leave the target locked before its ancestors. RangeVarCallbackForDropRelation() does a similar thing for DROP (it locks the parent before the partition), so it is fine this way too. One behavior change worth mentioning in the commit message. With a warm partition-constraint cache, generate_partition_qual() returns the cached copy without locking, so on master an ATTACH doesn't always lock every ancestor. I checked pg_locks after ALTER TABLE mid2 ATTACH PARTITION in a 3-level tree (in a single-user backend), on master with mid2's cache warmed first, the root isn't locked (mid1 still is), with v3 the root and mid1 are always locked in AccessShareLock, the same as master does with a cold cache. So with v3, ATTACH on a mid-level table always takes a lock on the root, which on master it doesn't hold when the cache is warm. I think that's fine, since it only makes the lock set independent of cache state, but a sentence saying so would help a committer. Otherwise v3 looks good to me. Regards, Surya Poondla
