On Fri, 21 Dec 2018 at 09:43, Robert Haas <robertmh...@gmail.com> wrote: > - I refactored expand_inherited_rtentry() to drive partition expansion > entirely off of PartitionDescs. The reason why this is necessary is > that it clearly will not work to have find_all_inheritors() use a > current snapshot to decide what children we have and lock them, and > then consult a different source of truth to decide which relations to > open with NoLock. There's nothing to keep the lists of partitions > from being different in the two cases, and that demonstrably causes > assertion failures if you SELECT with an ATTACH/DETACH loop running in > the background. However, it also changes the order in which tables get > locked. Possibly that could be fixed by teaching > expand_partitioned_rtentry() to qsort() the OIDs the way > find_inheritance_children() does. It also loses the infinite-loop > protection which find_all_inheritors() has. Not sure what to do about > that.
I don't think you need to qsort() the Oids before locking. What the qsort() does today is ensure we get a consistent locking order. Any other order would surely do, providing we stick to it consistently. I think PartitionDesc order is fine, as it's consistent. Having it locked in PartitionDesc order I think is what's needed for [1] anyway. [2] proposes to relax the locking order taken during execution. [1] https://commitfest.postgresql.org/21/1778/ [2] https://commitfest.postgresql.org/21/1887/ -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
