On 8 November 2018 at 05:05, Robert Haas <robertmh...@gmail.com> wrote: > That seems OK at present, because no new partitions can have appeared > since ExecSetupPartitionTupleRouting() acquired locks. But if we > allow new partitions to be added with only ShareUpdateExclusiveLock, > then I think there would be a problem. If a new partition OID creeps > into the partition descriptor after find_all_inheritors() and before > we fetch its partition descriptor, then we wouldn't have previously > taken a lock on it and would still be attempting to open it without a > lock, which is bad (cf. b04aeb0a053e7cf7faad89f7d47844d8ba0dc839). > > Admittedly, it might be a bit hard to provoke a failure here because > I'm not exactly sure how you could trigger a relcache reload in the > critical window, but I don't think we should rely on that. > > More generally, it seems a bit strange that we take the approach of > locking the entire partitioning hierarchy here regardless of which > relations the query actually knows about. If some relations have been > pruned, presumably we don't need to lock them; if/when we permit > concurrent partition, we don't need to lock any new ones that have > materialized. We're just going to end up ignoring them anyway because > there's nothing to do with the information that they are or are not > excluded from the query when they don't appear in the query plan in > the first place.
While the find_all_inheritors() call is something I'd like to see gone, I assume it was done that way since an UPDATE might route a tuple to a partition that there is no subplan for and due to INSERT with VALUES not having any RangeTblEntry for any of the partitions. Simply, any partition which is a descendant of the target partition table could receive the tuple regardless of what might have been pruned. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
