Re: ATTACH/DETACH PARTITION CONCURRENTLY

On 8 November 2018 at 05:05, Robert Haas <robertmhaas(at)gmail(dot)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