Re: Adding support for Default partition in partitioning

On Sat, Jul 29, 2017 at 2:55 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 28, 2017 at 9:30 AM, Ashutosh Bapat
> <ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:
>> 0004 patch
>> The patch adds another column partdefid to catalog pg_partitioned_table. The
>> column gives OID of the default partition for a given partitioned table. This
>> means that the default partition's OID is stored at two places 1. in the
>> default partition table's pg_class entry and in pg_partitioned_table. There is
>> no way to detect when these two go out of sync. Keeping those two in sync is
>> also a maintenance burdern. Given that default partition's OID is required only
>> while adding/dropping a partition, which is a less frequent operation, it won't
>> hurt to join a few catalogs (pg_inherits and pg_class in this case) to find out
>> the default partition's OID. That will be occasional performance hit
>> worth the otherwise maintenance burden.
>
> Performance isn't the only consideration here. We also need to think
> about locking and concurrency. I think that most operations that
> involve locking the parent will also involve locking the default
> partition. However, we can't safely build a relcache entry for a
> relation before we've got some kind of lock on it. We can't assume
> that there is no concurrent DDL going on before we take some lock. We
> can't assume invalidation messages are processed before we have taken
> some lock. If we read multiple catalog tuples, they may be from
> different points in time. If we can figure out everything we need to
> know from one or two syscache lookups, it may be easier to verify that
> the code is bug-free vs. having to do something more complicated.
>

The code takes a lock on the parent relation. While that function
holds that lock nobody else would change partitions of that relation
and hence nobody changes the default partition.
heap_drop_with_catalog() has code to lock the parent. Looking up
pg_inherits catalog for its partitions followed by identifying the
partition which has default partition bounds specification while
holding the lock on the parent should be safe. Any changes to
partition bounds, or partitions would require lock on the parent. In
order to prevent any buggy code changing the default partition without
sufficient locks, we should lock the default partition after it's
found and check the default partition bound specification again. Will
that work?

> Now that having been said, I'm not taking the position that Jeevan's
> patch (based on Amit Langote's idea) has definitely got the right
> idea, just that you should think twice before shooting down the
> approach.
>

If we can avoid the problems specified by Amit Langote, I am fine with
the approach of reading the default partition OID from the Relcache as
well. But I am not able to device a solution to those problems.

--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company