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
