Thanks again for quick review.
On 2018/06/28 12:43, Michael Paquier wrote:
> On Thu, Jun 28, 2018 at 11:50:13AM +0900, Amit Langote wrote:
>> For now, I've added them to create_table.sql, but maybe that's not where
>> they really belong. Attached updated patch with tests.
>
> It would have imagined that just creating a new file, say
> partition_desc.sql or similar is nicer.
How about partition_info.sql because they're testing partitioning
information functions? partition_desc reminded me of PartitionDesc, an
internal structure used in the partitioning codem which made me a bit
uncomfortable.
> + ancestors = get_partition_ancestors(relid);
> + result = llast_oid(ancestors);
> + list_free(ancestors);
>
> Relying on the fact that the top-most parent should be the last one in
> the list is brittle in my opinion.
get_partition_ancestor stops adding OIDs to the list once it reaches a
table in the ancestor chain that doesn't itself have parent (the root), so
the last OID in the returned list *must* be the root parent.
Do you think adding a check that the OID in result is indeed NOT a
partition would make it look less brittle? I added an Assert below that
llast_oid statement.
> What this patch proposes is:
> - pg_partition_root_parent to get the top-most parent within a partition
> tree for a partition.
> - pg_partition_parent to get the direct parent for a partition.
> - pg_partition_tree_tables to get a full list of all the children
> underneath.
>
> As the goal is to facilitate the life of users so as they don't have to
> craft any WITH RECURSIVE, I think that we could live with that.
>
> + <para>
> + If the table passed to <function>pg_partition_root_parent</function> is not
> + a partition, the same table is returned as the result. Result of
> + <function>pg_partition_tree_tables</function> also contains the table
> + that's passed to it as the first row.
> + </para>
> Okay for that part as well.
>
> I haven't yet looked at the code in details, but what you are proposing
> here looks sound. Could you think about adding an example in the docs
> about how to use them? Say for a measurement table here is a query to
> get the full size a partition tree takes.. That's one idea.
OK, I've added an example below the table of functions added by the patch.
Attached updated patch.
Thanks,
Amit