Re: Add pg_partition_root to get top-most parent of a partition tree

Hi Michael,

Sorry about the long delay in replying.  Looking at the latest patch (v4)
but replying to an earlier email of yours.

On 2018/12/15 10:16, Michael Paquier wrote:
> On Fri, Dec 14, 2018 at 02:20:27PM +0900, Amit Langote wrote:
>> +static bool
>> +check_rel_for_partition_info(Oid relid)
>> +{
>> +    char        relkind;
>> +
>> +    /* Check if relation exists */
>> +    if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(relid)))
>> +        return false;
>>
>> This should be checked in the caller imho.
> 
> On this one I disagree, both pg_partition_root and pg_partition_tree
> share the same semantics on the matter.  If the set of functions gets
> expanded again later on, I got the feeling that we could forget about it
> again, and at least placing the check here has the merit to make out
> future selves not forget about that pattern..

OK, no problem.

Some minor comments on v4:

+/*
+ * Perform several checks on a relation on which is extracted some
+ * information related to its partition tree.

This is a bit unclear to me.  How about:

Checks if a given relation can be part of a partition tree

+/*
+ * pg_partition_root
+ *
+ * For the given relation part of a partition tree, return its top-most
+ * root parent.
+ */

How about writing:

Returns the top-most parent of the partition tree to which a given
relation belongs, or NULL if it's not (or cannot be) part of any partition
tree


Given that a couple (?) of other patches depend on this, maybe it'd be a
good idea to proceed with this.  Sorry that I kept this hanging too long
by not sending these comments sooner.

Thanks,
Amit