On 2026-Aug-03, shveta malik wrote: > On Mon, Aug 3, 2026 at 2:34 PM Álvaro Herrera <[email protected]> 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. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ Thou shalt check the array bounds of all strings (indeed, all arrays), for surely where thou typest "foo" someone someday shall type "supercalifragilisticexpialidocious" (5th Commandment for C programmers)
