On 16 August 2017 at 11:06, Amit Langote <langote_amit...@lab.ntt.co.jp> wrote:
> Attached updated patches. Thanks Amit for the patches. I too agree with the overall approach taken for keeping the locking order consistent: it's best to do the locking with the existing find_all_inheritors() since it is much cheaper than to lock them in partition-bound order, the later being expensive since it requires opening partitioned tables. > I haven't yet done anything about changing the timing of opening and > locking leaf partitions, because it will require some more thinking about > the required planner changes. But the above set of patches will get us > far enough to get leaf partition sub-plans appear in the partition bound > order (same order as what partition tuple-routing uses in the executor). So, I believe none of the changes done in pg_inherits.c are essential for expanding the inheritence tree in bound order, right ? I am not sure whether we are planning to commit these two things together or incrementally : 1. Expand in bound order 2. Allow for locking only the partitioned tables first. For #1, the changes in pg_inherits.c are not required (viz, keeping partitioned tables at the head of the list, adding inhchildparted column, etc). If we are going to do #2 together with #1, then in the patch set there is no one using the capability introduced by #2. That is, there are no callers of find_all_inheritors() that make use of the new num_partitioned_children parameter. Also, there is no boolean parameter for find_all_inheritors() to be used to lock only the partitioned tables. I feel we should think about 0002-Teach-pg_inherits.c-a-bit-about-partitioning.patch later, and first get the review done for the other patches. ------- I see that RelationGetPartitionDispatchInfo() now returns quite a small subset of what it used to return, which is good. But I feel for expand_inherited_rtentry(), RelationGetPartitionDispatchInfo() is still a bit heavy. We only require the oids, so the PartitionedTableInfo data structure (including the pd->indexes array) gets discarded. Also, RelationGetPartitionDispatchInfo() has to call get_rel_relkind() for each child. In expand_inherited_rtentry(), we anyway have to open all the child tables, so we get the partition descriptors for each of the children for free. So how about, in expand_inherited_rtentry(), we traverse the partition tree using these descriptors similar to how it is traversed in RelationGetPartitionDispatchInfo() ? May be to avoid code duplication for traversing, we can have a common API. Still looking at RelationGetPartitionDispatchInfo() changes ... -- Thanks, -Amit Khandekar EnterpriseDB Corporation The Postgres Database Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
