On Fri, Dec 21, 2018 at 6:04 PM David Rowley <david.row...@2ndquadrant.com> wrote: > 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/
Based on this feedback, I went ahead and committed the part of the previously-posted patch set that makes this change. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
