On Fri, 21 Dec 2018 at 09:43, Robert Haas <robertmhaas@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