Re: [HACKERS] expanding inheritance in partition bound order

On 2017/08/04 20:28, Ashutosh Bapat wrote:
> On Fri, Aug 4, 2017 at 1:08 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> The current way to expand inherited tables, including partitioned tables,
>> is to use either find_all_inheritors() or find_inheritance_children()
>> depending on the context.  They return child table OIDs in the (ascending)
>> order of those OIDs, which means the callers that need to lock the child
>> tables can do so without worrying about the possibility of deadlock in
>> some concurrent execution of that piece of code.  That's good.
>>
>> For partitioned tables, there is a possibility of returning child table
>> (partition) OIDs in the partition bound order, which in addition to
>> preventing the possibility of deadlocks during concurrent locking, seems
>> potentially useful for other caller-specific optimizations.  For example,
>> tuple-routing code can utilize that fact to implement binary-search based
>> partition-searching algorithm.  For one more example, refer to the "UPDATE
>> partition key" thread where it's becoming clear that it would be nice if
>> the planner had put the partitions in bound order in the ModifyTable that
>> it creates for UPDATE of partitioned tables [1].
> 
> Thanks a lot for working on this. Partition-wise join can benefit from
> this as well. See comment about build_simple_rel's matching algorithm
> in [1]. It will become O(n) instead of O(n^2).

Nice.  It seems that we have a good demand for $subject. :)

>> So attached are two WIP patches:
>>
>> 0001 implements two interface functions:
>>
>>   List *get_all_partition_oids(Oid, LOCKMODE)
>>   List *get_partition_oids(Oid, LOCKMODE)
>>
>> that resemble find_all_inheritors() and find_inheritance_children(),
>> respectively, but expect that users call them only for partitioned tables.
>>  Needless to mention, OIDs are returned with canonical order determined by
>> that of the partition bounds and they way partition tree structure is
>> traversed (top-down, breadth-first-left-to-right).  Patch replaces all the
>> calls of the old interface functions with the respective new ones for
>> partitioned table parents.  That means expand_inherited_rtentry (among
>> others) now calls get_all_partition_oids() if the RTE is for a partitioned
>> table and find_all_inheritors() otherwise.
>>
>> In its implementation, get_all_partition_oids() calls
>> RelationGetPartitionDispatchInfo(), which is useful to generate the result
>> list in the desired partition bound order.  But the current interface and
>> implementation of RelationGetPartitionDispatchInfo() needs some rework,
>> because it's too closely coupled with the executor's tuple routing code.
> 
> May be we want to implement get_all_partition_oids() calling
> get_partition_oids() on each new entry that gets added, similar to
> find_all_inheritors(). That might avoid changes to DispatchInfo() and
> also dependency on that structure.
> 
> Also almost every place which called find_all_inheritors() or
> find_inheritance_children() is changed to if () else case calling
> those functions or the new function as required. May be we should
> create macros/functions to do that routing to keep the code readable.
> May be find_all_inheritors() and find_inheritance_children()
> themselves become the routing function and their original code moves
> into new functions get_all_inheritors() and
> get_inheritance_children(). We may choose other names for functions.
> The idea is to create routing functions/macros instead of sprinkling
> code with if () else conditions.

Given the Robert's comments [1], it seems that we might have to abandon
the idea to separate the interface for partitioned and non-partitioned
inheritance cases.  I'm thinking about the issues and alternatives he
mentioned in his email.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/CA%2BTgmobwbh12OJerqAGyPEjb_%2B2y7T0nqRKTcjed6L4NTET6Fg%40mail.gmail.com