Re: A new C function `get_partition_root`.

On Mon, Aug 3, 2026 at 8:25 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> On 2026-Aug-03, shveta malik wrote:
>
> > On Mon, Aug 3, 2026 at 2:34 PM Álvaro Herrera <alvherre(at)kurilemu(dot)de> wrote:
>
> > > I'd say this looks okay, but why do you need get_partition_root_guts()
> > > exposed in partition.h? In fact, it's not clear to me why you need a
> > > second routine at all. Why isn't enough to have just get_partition_root()?
> >
> > I think to avoid performing the validation twice in
> > pg_partition_root(): first via check_rel_can_be_partition(), and then
> > again in get_partition_root (see [1]), get_partition_root_guts() is
> > introduced and exposed in partition.h.
> >
> > [1]:
> > + /* Validate relid is member of a partition tree */
> > + Assert(get_rel_relispartition(relid) ||
> > + RELKIND_HAS_PARTITIONS(get_rel_relkind(relid)));
>
> This seems wrong actually (having this as an assert rather than
> if/elog), because it means no validation at all occur on normal builds.
> Surely that's the wrong thing?
>
> Redundant asserts are no cause for concern IMO. I would certainly not
> create two routines just to avoid an assert, which is nothing at all in
> production builds.
>

Hi Alvaro.

Thankyou for your review cvomments.

I'd like to implement this whichever way you think is best, but I
wasn't sure what had been decided for some of the questions.

e.g.

Q1. Keep Asserts for validation, or change to if/elog?
- As Shveta mentioned, the get_partition_ancestors also has no
validation for normal builds

Q2. Keep or remove the wrapper layer for get_partition_root_guts?
- As Shveta mentioned, the purpose of the wrapper was to avoid
doubling the validation when called from SQL pg_partition_root()
- If keeping Aserts, there would be zero overheads in a normal build,
so removing the wrapper is fine
- If get_partition_root uses if/elog validation, I'm not so sure

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