| From: | Álvaro Herrera <alvherre(at)kurilemu(dot)de> |
|---|---|
| To: | shveta malik <shveta(dot)malik(at)gmail(dot)com> |
| Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: A new C function `get_partition_root`. |
| Date: | 2026-08-03 10:25:26 |
| Message-ID: | anBr9xLwGprfT_5k@alvherre.pgsql |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
--
Á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)
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Amit Kapila | 2026-08-03 10:36:48 | Re: CREATE SUBSCRIPTION ... SERVER vs. pg_dump, etc. |
| Previous Message | shveta malik | 2026-08-03 10:15:32 | Re: A new C function `get_partition_root`. |