Re: A new C function `get_partition_root`.

Hi Shveta.

Thanks for your review!

On Tue, Jul 28, 2026 at 3:41 PM shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
>
...
>
> I agree that the patch you mentioned will benefit from such a
> function. So before we commit that patch ( FOR TABLES IN SCHEMA
> EXCEPT), this function addition can be considered. A few comments:
>
> 1)
> + * Note: This should only be called when it is known that the relation is a
> + * partition (see function get_partition_ancestors).
>
> The comment does not match the actual flow through
> pg_partition_root(). This function can also be reached when the input
> relation is not a partition but a partitioned table (the root itself).
> We should update the comment to reflect the behavior enforced by
> check_rel_can_be_partition() in pg_partition_root().

OK. I've changed the function comment to say the specified relid may
be a partitioned table.

>
> 2)
> Looking only at get_partition_root(), it doesn't seem to be
> self-contained in validating that the caller has supplied a valid
> input.
>
> Since this internal function is expected to be used more widely (in
> the thread you referenced), I would expect it to handle all input
> cases consistently:
>
> --Return NULL if the input relation is neither a partition nor a
> partitioned table. Or may be Assert will be better?
> --Return the same relation if the input is already the partitioned
> table (the root).
> --Return the root partitioned table if the input is a partition.
>
> The second and third cases currently work as expected. But the first
> and second cases are not distinguishable. Both a regular table and a
> partitioned table results in the input relation's OID being returned
> by get_partition_root (verified by pg_partition_root() by bypassing
> check_rel_can_be_partition()). I think get_partition_root needs some
> improvement for validaitng and distinguishing above cases.
>

You are right, there was some quirky behaviour for regular tables.

The original code fragment of pg_get_partition_root (below)
------
if (ancestors == NIL)
PG_RETURN_OID(relid);
------
only made sense because the relid had already been verified up-front
to be a partition or partitioned table.

I wanted my new function to be more like `get_partition_ancestors`,
which has expectations on the caller, rather than adding a lot of
up-front parameter validation like `pg_get_partition_root` has. OTOH,
I agree with your point that a regular table should not be returned
from this function, so I have added the necessary Assert to fix that.

~~~

PSA patch v2.

This still passes make check-world, and I verified different inputs
manually using the same technique that you described.

======
Kind Regards,
Peter Smith.
Fujitsu Australia