Re: [HACKERS] expanding inheritance in partition bound order

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).

>
> 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.

[1] https://www.postgresql.org/message-id/CA+TgmobeRUTu4osXA_UA4AORho83WxAvFG8n1NQcoFuujbeh7A@mail.gmail.com
-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company