Re: expanding inheritance in partition bound order

Thanks Amit for the patch. I am still reviewing it, but meanwhile
below are a few comments so far ...

On 8 September 2017 at 15:53, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> [PATCH 2/2] Make RelationGetPartitionDispatch expansion order
>  depth-first
>
> This is so as it matches what the planner is doing with partitioning
> inheritance expansion.  Matching with planner order helps because
> it helps ease matching the executor's per-partition objects with
> planner-created per-partition nodes.
>
>

+   next_parted_idx += (list_length(*pds) - next_parted_idx - 1);

I think this can be replaced just by :
+   next_parted_idx = list_length(*pds) - 1;
Or, how about removing this variable next_parted_idx altogether ?
Instead, we can just do this :
pd->indexes[i] = -(1 + list_length(*pds));
If that is not possible, I may be missing something.

-----------

+ next_leaf_idx += (list_length(*leaf_part_oids) - next_leaf_idx);

Didn't understand why next_leaf_idx needs to be updated in case when
the current partrelid is partitioned. I think it should be incremented
only for leaf partitions, no ? Or for that matter, again, how about
removing the variable 'next_leaf_idx' and doing this :
*leaf_part_oids = lappend_oid(*leaf_part_oids, partrelid);
pd->indexes[i] = list_length(*leaf_part_oids) - 1;

-----------

* For every partitioned table in the tree, starting with the root
* partitioned table, add its relcache entry to parted_rels, while also
* queuing its partitions (in the order in which they appear in the
* partition descriptor) to be looked at later in the same loop.  This is
* a bit tricky but works because the foreach() macro doesn't fetch the
* next list element until the bottom of the loop.

I think the above comment needs to be modified with something
explaining the relevant changed code. For e.g. there is no
parted_rels, and the "tricky" part was there earlier because of the
list being iterated and at the same time being appended.

------------

I couldn't see the existing comments like "Indexes corresponding to
the internal partitions are multiplied by" anywhere in the patch. I
think those comments are still valid, and important.


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company