Re: partition tree inspection functions

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Jeevan Ladhe <jeevan(dot)ladhe(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: partition tree inspection functions
Date: 2018-06-28 05:49:22
Message-ID: 6baeb45a-6222-6b88-342d-37fc7d3cf89a@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
v3-0001-Add-assorted-partition-reporting-functions.patch text/plain 18.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-06-28 05:53:40 Re: [HACKERS] advanced partition matching algorithm for partition-wise join
Previous Message Michael Paquier 2018-06-28 05:39:15 Re: [HACKERS] Crash on promotion when recovery.conf is renamed