On 2017/08/04 20:28, Ashutosh Bapat wrote: > On Fri, Aug 4, 2017 at 1:08 PM, Amit Langote > <langote_amit...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers